From: Duarte Meneses Date: Thu, 5 Mar 2020 17:45:56 +0000 (-0600) Subject: SONAR-13165 Scanner warns that it wasn't able to detected changed lines on files... X-Git-Tag: 8.3.0.34182~153 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=fb03a19566ef193e4860fb276824ff315ca5047d;p=sonarqube.git SONAR-13165 Scanner warns that it wasn't able to detected changed lines on files with lines removed only --- diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java index e7bda351dd3..be44d13eb41 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/scm/ScmProvider.java @@ -68,8 +68,9 @@ public abstract class ScmProvider { /** * Return a map between paths given as argument and the corresponding line numbers which are new compared to the provided target branch. - * If null is returned or if a path is not included in the map, an imprecise fallback mechanism will be used to detect which lines - * are new (based on SCM dates). + * If nothing is returned for a file, the scanner will consider that the provider was unable to determine changes for that file and it will + * assume that nothing was changed in it. + * If null is returned, an imprecise fallback mechanism will be used to detect which lines are new (based on SCM dates). * * @param files Absolute path of files of interest * @return null if the SCM provider was not able to compute the new lines diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java index d934999ab9b..47dd8f402b5 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java @@ -89,18 +89,20 @@ public class ChangedLinesPublisher implements ReportPublisherStep { for (Map.Entry e : changedFiles.entrySet()) { DefaultInputFile inputFile = e.getValue(); - Set changedLines = pathSetMap.getOrDefault(e.getKey(), Collections.emptySet()); + Set changedLines = pathSetMap.get(e.getKey()); - // detect unchanged last empty line - if (changedLines.size() + 1 == inputFile.lines() && inputFile.lineLength(inputFile.lines()) == 0) { - changedLines.add(inputFile.lines()); - } - - if (changedLines.isEmpty()) { + if (changedLines == null) { LOG.warn("File '{}' was detected as changed but without having changed lines", e.getKey().toAbsolutePath()); + // assume that no line was changed + writeChangedLines(writer, e.getValue().scannerId(), Collections.emptySet()); + } else { + // detect unchanged last empty line + if (changedLines.size() + 1 == inputFile.lines() && inputFile.lineLength(inputFile.lines()) == 0) { + changedLines.add(inputFile.lines()); + } + count++; + writeChangedLines(writer, e.getValue().scannerId(), changedLines); } - count++; - writeChangedLines(writer, e.getValue().scannerId(), changedLines); } return count; } diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java index ee1f0b37382..bcbd48c843c 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java @@ -19,6 +19,7 @@ */ package org.sonar.scanner.report; +import com.google.common.collect.ImmutableMap; import java.io.File; import java.nio.file.Path; import java.nio.file.Paths; @@ -34,6 +35,7 @@ import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.DefaultInputProject; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.scm.ScmProvider; +import org.sonar.api.utils.log.LogTester; import org.sonar.scanner.fs.InputModuleHierarchy; import org.sonar.scanner.protocol.output.ScannerReportReader; import org.sonar.scanner.protocol.output.ScannerReportWriter; @@ -42,6 +44,7 @@ import org.sonar.scanner.scan.filesystem.InputComponentStore; import org.sonar.scanner.scm.ScmConfiguration; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; @@ -61,6 +64,9 @@ public class ChangedLinesPublisherTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); + @Rule + public LogTester logTester = new LogTester(); + private ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration); @Before @@ -114,16 +120,22 @@ public class ChangedLinesPublisherTest { @Test public void write_changed_files() { DefaultInputFile fileWithChangedLines = createInputFile("path1", "l1\nl2\nl3\n"); - DefaultInputFile fileWithoutChangedLines = createInputFile("path2", "l1\nl2\nl3\n"); - Set paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2"))); + DefaultInputFile fileNotReturned = createInputFile("path2", "l1\nl2\nl3\n"); + DefaultInputFile fileWithoutChangedLines = createInputFile("path3", "l1\nl2\nl3\n"); + + Set paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2"), BASE_DIR.resolve("path3"))); Set lines = new HashSet<>(Arrays.asList(1, 10)); - when(provider.branchChangedLines(TARGET_BRANCH, BASE_DIR, paths)).thenReturn(Collections.singletonMap(BASE_DIR.resolve("path1"), lines)); - when(inputComponentStore.allChangedFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileWithoutChangedLines)); + when(provider.branchChangedLines(TARGET_BRANCH, BASE_DIR, paths)) + .thenReturn(ImmutableMap.of(BASE_DIR.resolve("path1"), lines, BASE_DIR.resolve("path3"), Collections.emptySet())); + when(inputComponentStore.allChangedFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileNotReturned, fileWithoutChangedLines)); publisher.publish(writer); assertPublished(fileWithChangedLines, new HashSet<>(Arrays.asList(1, 10))); assertPublished(fileWithoutChangedLines, Collections.emptySet()); + assertPublished(fileNotReturned, Collections.emptySet()); + + assumeThat(logTester.logs()).contains("File '/root/path2' was detected as changed but without having changed lines"); } @Test @@ -170,10 +182,6 @@ public class ChangedLinesPublisherTest { assertThat(reader.readComponentChangedLines(file.scannerId()).getLineList()).containsExactlyElementsOf(lines); } - private void assertNotPublished(DefaultInputFile file) { - assertThat(new File(temp.getRoot(), "changed-lines-" + file.scannerId() + ".pb")).doesNotExist(); - } - private void assertNotPublished() { assertThat(temp.getRoot().list()).isEmpty(); }