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 1413a7a6bfa3..5a60eeb52903 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 @@ -72,10 +72,16 @@ public ChangedLinesPublisher(ScmConfiguration scmConfiguration, InputModuleHiera } private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer) { + String targetBranchName = branchConfiguration.branchTarget(); + if (targetBranchName == null) { + return 0; + } + Path rootBaseDir = inputModuleHierarchy.root().getBaseDir(); - Map allPublishedFiles = StreamSupport.stream(inputComponentStore.allFilesToPublish().spliterator(), false) + Map changedFiles = StreamSupport.stream(inputComponentStore.allChangedFilesToPublish().spliterator(), false) .collect(Collectors.toMap(DefaultInputFile::path, f -> f)); - Map> pathSetMap = provider.branchChangedLines(branchConfiguration.branchTarget(), rootBaseDir, allPublishedFiles.keySet()); + + Map> pathSetMap = provider.branchChangedLines(targetBranchName, rootBaseDir, changedFiles.keySet()); int count = 0; if (pathSetMap == null) { @@ -84,7 +90,7 @@ private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer) return count; } - for (Map.Entry e : allPublishedFiles.entrySet()) { + for (Map.Entry e : changedFiles.entrySet()) { Set changedLines = pathSetMap.getOrDefault(e.getKey(), Collections.emptySet()); if (changedLines.isEmpty()) { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ComponentsPublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ComponentsPublisher.java index 8ec6f7af82aa..2ad95729c130 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ComponentsPublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ComponentsPublisher.java @@ -165,7 +165,7 @@ private boolean shouldSkipComponent(DefaultInputComponent component, Collection< } else if (component instanceof DefaultInputFile) { // skip files not marked for publishing DefaultInputFile inputFile = (DefaultInputFile) component; - return !inputFile.isPublished() || (branchConfiguration.isShortOrPullRequest() && inputFile.status() == Status.SAME); + return !inputFile.isPublished(); } return false; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/SourcePublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/SourcePublisher.java index acb37b393faa..95cfc041b1ee 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/SourcePublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/report/SourcePublisher.java @@ -44,7 +44,7 @@ public SourcePublisher(InputComponentStore componentStore) { @Override public void publish(ScannerReportWriter writer) { - for (final DefaultInputFile inputFile : componentCache.allFilesToPublish()) { + for (final DefaultInputFile inputFile : componentCache.allChangedFilesToPublish()) { File iofile = writer.getSourceFile(inputFile.batchId()); try (OutputStream output = new BufferedOutputStream(new FileOutputStream(iofile)); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/InputComponentStore.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/InputComponentStore.java index e6d96e517339..0d4388e3dd73 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/InputComponentStore.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/InputComponentStore.java @@ -31,12 +31,12 @@ import java.util.Map; import java.util.SortedSet; import java.util.TreeSet; +import java.util.stream.Stream; import javax.annotation.CheckForNull; import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputDir; import org.sonar.api.batch.fs.InputFile; -import org.sonar.api.batch.fs.InputFile.Status; import org.sonar.api.batch.fs.InputModule; import org.sonar.api.batch.fs.internal.DefaultInputDir; import org.sonar.api.batch.fs.internal.DefaultInputFile; @@ -76,11 +76,19 @@ public Collection all() { return inputComponents.values(); } - public Iterable allFilesToPublish() { + private Stream allFilesToPublishStream() { return inputFileCache.values().stream() .map(f -> (DefaultInputFile) f) - .filter(DefaultInputFile::isPublished) - .filter(f -> !branchConfiguration.isShortOrPullRequest() || f.status() != Status.SAME) + .filter(DefaultInputFile::isPublished); + } + + public Iterable allFilesToPublish() { + return allFilesToPublishStream()::iterator; + } + + public Iterable allChangedFilesToPublish() { + return allFilesToPublishStream() + .filter(f -> !branchConfiguration.isShortOrPullRequest() || f.status() != InputFile.Status.SAME) ::iterator; } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java index 1d568737d7c5..859cda8b75dd 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java @@ -231,9 +231,6 @@ public DefaultSensorStorage(MetricFinder metricFinder, ModuleIssues moduleIssues public void store(Measure newMeasure) { if (newMeasure.inputComponent() instanceof DefaultInputFile) { DefaultInputFile defaultInputFile = (DefaultInputFile) newMeasure.inputComponent(); - if (shouldSkipStorage(defaultInputFile)) { - return; - } defaultInputFile.setPublished(true); } saveMeasure(newMeasure.inputComponent(), (DefaultMeasure) newMeasure); @@ -248,9 +245,6 @@ private void logOnce(String metricKey, String msg, Object... params) { private void saveMeasure(InputComponent component, DefaultMeasure measure) { if (component.isFile()) { DefaultInputFile defaultInputFile = (DefaultInputFile) component; - if (shouldSkipStorage(defaultInputFile)) { - return; - } defaultInputFile.setPublished(true); } @@ -471,9 +465,6 @@ public void store(DefaultSymbolTable symbolTable) { @Override public void store(DefaultCoverage defaultCoverage) { DefaultInputFile inputFile = (DefaultInputFile) defaultCoverage.inputFile(); - if (shouldSkipStorage(inputFile)) { - return; - } inputFile.setPublished(true); if (defaultCoverage.linesToCover() > 0) { saveCoverageMetricInternal(inputFile, LINES_TO_COVER, new DefaultMeasure().forMetric(LINES_TO_COVER).withValue(defaultCoverage.linesToCover())); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java index 28d4b5e0d77a..efc99a743f83 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java @@ -70,7 +70,7 @@ public void prepare() throws IOException { } @Test - public void should_skip_report_for_unchanged_files_in_short_branch() { + public void should_not_skip_report_for_unchanged_files_in_short_branch() { // sanity check, normally report gets generated TaskResult result = getResult(tester); assertThat(getResult(tester).getReportComponent(result.inputFile(FILE_PATH).key())).isNotNull(); @@ -79,12 +79,12 @@ public void should_skip_report_for_unchanged_files_in_short_branch() { assertThat(result.getReportReader().hasCoverage(fileId)).isTrue(); assertThat(result.getReportReader().readFileSource(fileId)).isNotNull(); - // file is skipped for short branches (no report, no coverage, no duplications) + // file is not skipped for short branches (need coverage, duplications coming soon) TaskResult result2 = getResult(tester.setBranchType(BranchType.SHORT)); - assertThat(result2.getReportComponent(result2.inputFile(FILE_PATH).key())).isNull(); + assertThat(result2.getReportComponent(result2.inputFile(FILE_PATH).key())).isNotNull(); assertThat(result2.getReportReader().readChangesets(fileId)).isNull(); - assertThat(result2.getReportReader().hasCoverage(fileId)).isFalse(); - assertThat(result.getReportReader().readFileSource(fileId)).isNull(); + assertThat(result2.getReportReader().hasCoverage(fileId)).isTrue(); + assertThat(result2.getReportReader().readFileSource(fileId)).isNull(); } @Test 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 4f73d5894cf1..6bb328a02226 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 @@ -119,7 +119,7 @@ public void write_changed_files() { Set paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2"))); 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.allFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileWithoutChangedLines)); + when(inputComponentStore.allChangedFilesToPublish()).thenReturn(Arrays.asList(fileWithChangedLines, fileWithoutChangedLines)); publisher.publish(writer); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ComponentsPublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ComponentsPublisherTest.java index 1e84cff44347..da85993c73a3 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ComponentsPublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ComponentsPublisherTest.java @@ -309,7 +309,7 @@ public void should_skip_empty_modules_for_short_living_branches() throws IOExcep } @Test - public void skip_unchanged_components_in_short_branches() throws IOException { + public void do_not_skip_unchanged_components_in_short_branches() throws IOException { when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class); when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12")); @@ -373,13 +373,14 @@ public void skip_unchanged_components_in_short_branches() throws IOException { assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 4)).isTrue(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isFalse(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isFalse(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isFalse(); + // do not skip, needed for computing overall coverage + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isTrue(); + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isTrue(); + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isTrue(); } @Test - public void skip_unchanged_components_in_pull_requests() throws IOException { + public void do_not_skip_unchanged_components_in_pull_requests() throws IOException { when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class); when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12")); @@ -443,9 +444,10 @@ public void skip_unchanged_components_in_pull_requests() throws IOException { assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 4)).isTrue(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isFalse(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isFalse(); - assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isFalse(); + // do not skip, needed for computing overall coverage + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isTrue(); + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isTrue(); + assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isTrue(); } @Test diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java index 27b67e502208..813c6310e5be 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java @@ -86,6 +86,7 @@ public void prepare() throws Exception { MetricFinder metricFinder = mock(MetricFinder.class); when(metricFinder.findByKey(CoreMetrics.NCLOC_KEY)).thenReturn(CoreMetrics.NCLOC); when(metricFinder.findByKey(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION_KEY)).thenReturn(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION); + when(metricFinder.findByKey(CoreMetrics.LINES_TO_COVER_KEY)).thenReturn(CoreMetrics.LINES_TO_COVER); settings = new MapSettings(); moduleIssues = mock(ModuleIssues.class); @@ -191,29 +192,20 @@ public void should_save_file_measure() { } @Test - public void should_skip_file_measure_on_short_branch_when_file_status_is_SAME() { - InputFile file = new TestInputFileBuilder("foo", "src/Foo.php").setStatus(InputFile.Status.SAME).build(); - when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); - - underTest.store(new DefaultMeasure() - .on(file) - .forMetric(CoreMetrics.NCLOC) - .withValue(10)); - - verifyZeroInteractions(measureCache); - } - - @Test - public void should_skip_file_measure_on_pull_request_when_file_status_is_SAME() { + public void should_not_skip_file_measures_on_short_lived_branch_or_pull_request_when_file_status_is_SAME() { InputFile file = new TestInputFileBuilder("foo", "src/Foo.php").setStatus(InputFile.Status.SAME).build(); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(DefaultMeasure.class); + when(measureCache.put(eq(file.key()), eq(CoreMetrics.LINES_TO_COVER_KEY), argumentCaptor.capture())).thenReturn(null); underTest.store(new DefaultMeasure() .on(file) - .forMetric(CoreMetrics.NCLOC) + .forMetric(CoreMetrics.LINES_TO_COVER) .withValue(10)); - verifyZeroInteractions(measureCache); + DefaultMeasure m = argumentCaptor.getValue(); + assertThat(m.value()).isEqualTo(10); + assertThat(m.metric()).isEqualTo(CoreMetrics.LINES_TO_COVER); } @Test