Merge pull request #516 from jenkinsci/warnings-delta

Add method to compute all the modified lines of a file change

pom.xml

@@ -4,7 +4,7 @@
   <parent>
     <groupId>org.jvnet.hudson.plugins</groupId>
     <artifactId>analysis-pom</artifactId>
-    <version>6.17.0</version>
+    <version>7.0.0</version>
     <relativePath />
   </parent>
 

src/main/java/io/jenkins/plugins/forensics/delta/Change.java

@@ -16,7 +16,6 @@
  */
 @SuppressWarnings("PMD.DataClass")
 public class Change implements Serializable {
-
     private static final long serialVersionUID = 1543635877389921937L;
 
     private final ChangeEditType changeEditType;

src/main/java/io/jenkins/plugins/forensics/delta/Delta.java

@@ -13,18 +13,14 @@
  */
 @SuppressWarnings("PMD.DataClass")
 public class Delta implements Serializable {
+    private static final long serialVersionUID = 5641235877389921937L;
 
     static final String ERROR_MESSAGE_UNKNOWN_FILE = "No information about changes for the file with the ID '%s' stored";
 
-    private static final long serialVersionUID = 5641235877389921937L;
-
     private final String currentCommit;
-
     private final String referenceCommit;
 
-    /**
-     * Map which contains the changes for modified files, mapped by the file ID.
-     */
+    /** Contains the changes for modified files, mapped by the file ID. */
     private final Map<String, FileChanges> fileChangesMap;
 
     /**

src/main/java/io/jenkins/plugins/forensics/delta/DeltaCalculator.java

@@ -3,6 +3,8 @@
 import java.io.Serializable;
 import java.util.Optional;
 
+import org.apache.commons.lang3.StringUtils;
+
 import edu.hm.hafner.util.FilteredLog;
 
 import hudson.model.Run;
@@ -15,6 +17,23 @@
 public abstract class DeltaCalculator implements Serializable {
     private static final long serialVersionUID = 8641535877389921937L;
 
+    /**
+     * Calculates the {@link Delta} between two passed Jenkins builds.
+     *
+     * @param build
+     *         the currently processed build
+     * @param referenceBuild
+     *         The reference build
+     * @param logger
+     *         The used log
+     *
+     * @return the delta if it could be calculated
+     */
+    public Optional<Delta> calculateDelta(final Run<?, ?> build, final Run<?, ?> referenceBuild,
+            final FilteredLog logger) {
+        return calculateDelta(build, referenceBuild, StringUtils.EMPTY, logger);
+    }
+
     /**
      * Calculates the {@link Delta} between two passed Jenkins builds.
      *
@@ -28,7 +47,9 @@ public abstract class DeltaCalculator implements Serializable {
      *         The used log
      *
      * @return the delta if it could be calculated
+     * @deprecated use {@link #calculateDelta(Run, Run, FilteredLog)} instead
      */
+    @Deprecated
     public abstract Optional<Delta> calculateDelta(Run<?, ?> build, Run<?, ?> referenceBuild,
             String scmKeyFilter, FilteredLog logger);
 

src/main/java/io/jenkins/plugins/forensics/delta/FileChanges.java

@@ -1,6 +1,7 @@
 package io.jenkins.plugins.forensics.delta;
 
 import java.io.Serializable;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
@@ -16,7 +17,6 @@
  */
 @SuppressWarnings("PMD.DataClass")
 public class FileChanges implements Serializable {
-
     private static final long serialVersionUID = 6135245877389921937L;
 
     private final String fileName;
@@ -108,6 +108,30 @@ public void addChange(final Change change) {
         }
     }
 
+    /**
+     * Returns all modified lines in this changed file.
+     *
+     * @return the modified line
+     */
+    public Set<Integer> getModifiedLines() {
+        return changes.values().stream()
+                .flatMap(Collection::stream)
+                .map(this::getModifiedLinesForChange)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+    }
+
+    private Set<Integer> getModifiedLinesForChange(final Change filter) {
+        if (filter.getEditType() == ChangeEditType.INSERT || filter.getEditType() == ChangeEditType.REPLACE) {
+            var lines = new HashSet<Integer>();
+            for (int line = filter.getFromLine(); line <= filter.getToLine(); line++) {
+                lines.add(line);
+            }
+            return lines;
+        }
+        return Set.of();
+    }
+
     @Override
     public boolean equals(final Object o) {
         if (this == o) {

src/main/java/io/jenkins/plugins/forensics/miner/RelativeCountTrendChart.java

@@ -61,5 +61,4 @@ private LineSeries getSeries(final LinesDataSet dataSet,
         series.addAll(dataSet.getSeries(dataSetId));
         return series;
     }
-
 }

src/test/java/io/jenkins/plugins/forensics/delta/DeltaCalculatorFactoryITest.java

@@ -38,7 +38,7 @@ void shouldSelectNullDeltaCalculatorIfNoCalculatorIsFound() {
         DeltaCalculator nullCalculator = createDeltaCalculator("/", log);
 
         assertThat(nullCalculator).isInstanceOf(NullDeltaCalculator.class);
-        assertThat(nullCalculator.calculateDelta(mock(Run.class), mock(Run.class), "", log)).isEmpty();
+        assertThat(nullCalculator.calculateDelta(mock(Run.class), mock(Run.class), log)).isEmpty();
         assertThat(log.getInfoMessages()).containsOnly(NO_SUITABLE_DELTA_CALCULATOR_FOUND,
                 ACTUAL_FACTORY_NULL_DELTA_CALCULATOR, EMPTY_FACTORY_NULL_DELTA_CALCULATOR);
         assertThat(log.getErrorMessages()).isEmpty();

src/test/java/io/jenkins/plugins/forensics/delta/DeltaTest.java

@@ -17,7 +17,6 @@
  * @author Florian Orendi
  */
 class DeltaTest {
-
     private static final String CURRENT_COMMIT_ID = "testIdOne";
     private static final String REFERENCE_COMMIT_ID = "testIdTwo";
     private static final Map<String, FileChanges> FILE_CHANGES_MAP = Collections.emptyMap();

src/test/java/io/jenkins/plugins/forensics/delta/FileChangesTest.java

@@ -5,33 +5,32 @@
 import java.util.Set;
 
 import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
 
 import nl.jqno.equalsverifier.EqualsVerifier;
 
 import static io.jenkins.plugins.forensics.assertions.Assertions.*;
+import static org.mockito.Mockito.*;
 
 /**
  * Tests the class {@link FileChanges}.
  *
  * @author Florian Orendi
  */
 class FileChangesTest {
-
     private static final String FILE_NAME = "test";
     private static final String OLD_FILE_NAME = "testOld";
     private static final String FILE_CONTENT = "test";
     private static final FileEditType FILE_EDIT_TYPE = FileEditType.ADD;
-    private static final Map<ChangeEditType, Set<Change>> FILE_CHANGES = Collections.emptyMap();
 
     @Test
-    void testFileChangesGetter() {
+    void shouldCreateEmptyChanges() {
         FileChanges fileChanges = createFileChanges();
-        assertThat(fileChanges.getFileName()).isEqualTo(FILE_NAME);
-        assertThat(fileChanges.getOldFileName()).isEqualTo(OLD_FILE_NAME);
-        assertThat(fileChanges.getFileContent()).isEqualTo(FILE_CONTENT);
-        assertThat(fileChanges.getFileEditType()).isEqualTo(FILE_EDIT_TYPE);
-        assertThat(fileChanges.getChanges()).isEqualTo(FILE_CHANGES);
+        assertThat(fileChanges).hasFileName(FILE_NAME)
+                .hasOldFileName(OLD_FILE_NAME)
+                .hasFileContent(FILE_CONTENT)
+                .hasFileEditType(FILE_EDIT_TYPE)
+                .hasNoModifiedLines()
+                .hasChanges(Map.of());
     }
 
     @Test
@@ -48,27 +47,29 @@ void shouldAddChange() {
         assertThat(fileChanges.getChanges()).isEmpty();
         assertThat(fileChanges.getChangesByType(changeEditType)).isEmpty();
 
-        Change change = Mockito.mock(Change.class);
-        Mockito.when(change.getEditType()).thenReturn(changeEditType);
-
-        fileChanges.addChange(change);
-        fileChanges.addChange(change);
-
-        Map<ChangeEditType, Set<Change>> changesMap = fileChanges.getChanges();
-        assertThat(changesMap.size()).isEqualTo(1);
-        assertThat(changesMap).containsKey(changeEditType);
+        var first = createChange(changeEditType, 10, 14);
+        fileChanges.addChange(first);
+        var second = createChange(changeEditType, 100, 100);
+        fileChanges.addChange(second);
+        var unrelated = createChange(ChangeEditType.DELETE, 1000, 2000);
+        fileChanges.addChange(unrelated);
+
+        assertThat(fileChanges.getChanges()).containsOnly(
+                entry(changeEditType, Set.of(first, second)),
+                entry(ChangeEditType.DELETE, Set.of(unrelated)));
+        assertThat(fileChanges.getChangesByType(changeEditType)).containsExactlyInAnyOrder(first, second);
+        assertThat(fileChanges).hasModifiedLines(10, 11, 12, 13, 14, 100);
+    }
 
-        Set<Change> changes = fileChanges.getChangesByType(changeEditType);
-        assertThat(changes.size()).isEqualTo(1);
-        assertThat(changes).contains(change);
+    private Change createChange(final ChangeEditType changeEditType, final int start, final int end) {
+        Change change = mock(Change.class);
+        when(change.getEditType()).thenReturn(changeEditType);
+        when(change.getFromLine()).thenReturn(start);
+        when(change.getToLine()).thenReturn(end);
+        return change;
     }
 
-    /**
-     * Factory method which creates an instance of {@link FileChanges}.
-     *
-     * @return the created instance
-     */
     private FileChanges createFileChanges() {
-        return new FileChanges(FILE_NAME, OLD_FILE_NAME, FILE_CONTENT, FILE_EDIT_TYPE, FILE_CHANGES);
+        return new FileChanges(FILE_NAME, OLD_FILE_NAME, FILE_CONTENT, FILE_EDIT_TYPE, Collections.emptyMap());
     }
 }

src/test/java/io/jenkins/plugins/forensics/miner/AddedVersusDeletedLinesForensicsSeriesBuilderTest.java

@@ -2,40 +2,34 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
 class AddedVersusDeletedLinesForensicsSeriesBuilderTest {
-
     private static final String ADDED = "added";
     private static final String DELETED = "deleted";
 
     @Test
     void shouldCreateSeriesWithMockito() {
-        // Given
-        AddedVersusDeletedLinesForensicsSeriesBuilder builder = new AddedVersusDeletedLinesForensicsSeriesBuilder();
-        ForensicsBuildAction actionStub = mock(ForensicsBuildAction.class);
+        var actionStub = mock(ForensicsBuildAction.class);
         when(actionStub.getCommitStatistics()).thenReturn(createCommitStatistics(1, 2));
 
-        // When
-        Map<String, Integer> series = builder.computeSeries(actionStub);
-
-        // Then
-        assertThat(series)
+        assertThat(new AddedVersusDeletedLinesForensicsSeriesBuilder().computeSeries(actionStub))
                 .containsEntry(ADDED, 1)
                 .containsEntry(DELETED, 2);
-
     }
 
     private CommitStatistics createCommitStatistics(final int added, final int deleted) {
         List<CommitDiffItem> commits = new ArrayList<>();
+
         CommitDiffItem item = new CommitDiffItem("1", "author", 1);
         item.addLines(added).deleteLines(deleted);
+
         commits.add(item);
+
         return new CommitStatistics(commits);
     }
-}
+}

src/test/java/io/jenkins/plugins/forensics/miner/CodeMetricSeriesBuilderTest.java

@@ -1,29 +1,23 @@
 package io.jenkins.plugins.forensics.miner;
 
-import java.util.Map;
-
 import org.junit.jupiter.api.Test;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.data.MapEntry.entry;
+import static org.mockito.Mockito.*;
 
 class CodeMetricSeriesBuilderTest {
-
     @Test
     void shouldComputeRelativeCountStatistics() {
-        final int totalLinesOfCode = 10;
-        final int totalChurn = 20;
-        ForensicsBuildAction forensicsBuildAction = mock(ForensicsBuildAction.class);
-        when(forensicsBuildAction.getTotalLinesOfCode()).thenReturn(totalLinesOfCode);
-        when(forensicsBuildAction.getTotalChurn()).thenReturn(totalChurn);
+        int totalLinesOfCode = 10;
+        int totalChurn = 20;
 
-        CodeMetricSeriesBuilder codeMetricSeriesBuilder = new CodeMetricSeriesBuilder();
-        Map<String, Integer> computedSeries = codeMetricSeriesBuilder.computeSeries(forensicsBuildAction);
+        var action = mock(ForensicsBuildAction.class);
+        when(action.getTotalLinesOfCode()).thenReturn(totalLinesOfCode);
+        when(action.getTotalChurn()).thenReturn(totalChurn);
 
-        assertThat(computedSeries).hasSize(2);
-        assertThat(computedSeries.get(CodeMetricSeriesBuilder.LOC_KEY)).isEqualTo(totalLinesOfCode);
-        assertThat(computedSeries.get(CodeMetricSeriesBuilder.CHURN_KEY)).isEqualTo(totalChurn);
+        assertThat(new CodeMetricSeriesBuilder().computeSeries(action))
+                .containsExactly(entry(CodeMetricSeriesBuilder.LOC_KEY, totalLinesOfCode),
+                entry(CodeMetricSeriesBuilder.CHURN_KEY, totalChurn));
     }
-
-}
+}

src/test/java/io/jenkins/plugins/forensics/miner/CommitStatisticsTest.java

@@ -162,7 +162,6 @@ void shouldIgnoreCapitalizationOfEmailNames() {
 
         CommitStatistics secondCommit = new CommitStatistics(commits);
         assertThat(secondCommit).hasAuthorCount(1);
-
     }
 
     private TreeString asTreeString(final String old) {

src/test/java/io/jenkins/plugins/forensics/miner/ForensicsTableModelTest.java

@@ -5,11 +5,10 @@
 import io.jenkins.plugins.datatables.TableColumn;
 import io.jenkins.plugins.forensics.miner.ForensicsTableModel.ForensicsRow;
 
-import static io.jenkins.plugins.forensics.assertions.Assertions.assertThat;
+import static io.jenkins.plugins.forensics.assertions.Assertions.*;
 import static org.mockito.Mockito.*;
 
 class ForensicsTableModelTest {
-
     @Test
     void shouldCreateForensicsTableModel() {
         RepositoryStatistics statistics = new RepositoryStatistics();
@@ -29,7 +28,6 @@ void shouldCreateForensicsTableModel() {
                         Messages.Table_Column_LOC(),
                         Messages.Table_Column_Churn()
                 );
-
     }
 
     @Test
@@ -76,5 +74,4 @@ void checkForensicsRowGetters() {
                 .hasLinesOfCode(5)
                 .hasChurn(6);
     }
-
-}
+}

src/test/java/io/jenkins/plugins/forensics/miner/RelativeCountTrendChartTest.java

@@ -20,7 +20,6 @@
  * @author Nikolas Paripovic
  */
 class RelativeCountTrendChartTest {
-
     @Test
     void shouldCreate() {
         Iterable<BuildResult<CommitStatisticsBuildAction>> buildResult = new ArrayList<>();
@@ -74,5 +73,4 @@ private BuildResult<CommitStatisticsBuildAction> createResult(final int buildNum
         Build build = new Build(buildNumber);
         return new BuildResult<>(build, action);
     }
-
 }

src/test/java/io/jenkins/plugins/forensics/miner/SizePieChartTest.java

@@ -14,7 +14,6 @@
 import static org.mockito.Mockito.*;
 
 class SizePieChartTest {
-
     @Test
     void shouldCreateEmptyModel() {
         SizePieChart chart = new SizePieChart();
@@ -84,4 +83,4 @@ void shouldCreateTwoPieSegments() {
                 .hasSize(list.size())
                 .isEqualTo(list);
     }
-}
+}