Skip to content

Commit

Permalink
SONAR-11125 Publish all components with measures but without sources …
Browse files Browse the repository at this point in the history 
…of unchanged files
  • Loading branch information
Janos Gyerik authored and sonartech committed Sep 19, 2018
1 parent 3e85705 commit adb7939
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 48 deletions.
12 changes: 9 additions & 3 deletions sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ChangedLinesPublisher.java
View file
Expand Up @@ -72,10 +72,16 @@ public ChangedLinesPublisher(ScmConfiguration scmConfiguration, InputModuleHiera
} }


private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer) { private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer) {
String targetBranchName = branchConfiguration.branchTarget();
if (targetBranchName == null) {
return 0;
}

Path rootBaseDir = inputModuleHierarchy.root().getBaseDir(); Path rootBaseDir = inputModuleHierarchy.root().getBaseDir();
Map<Path, DefaultInputFile> allPublishedFiles = StreamSupport.stream(inputComponentStore.allFilesToPublish().spliterator(), false) Map<Path, DefaultInputFile> changedFiles = StreamSupport.stream(inputComponentStore.allChangedFilesToPublish().spliterator(), false)
.collect(Collectors.toMap(DefaultInputFile::path, f -> f)); .collect(Collectors.toMap(DefaultInputFile::path, f -> f));
Map<Path, Set<Integer>> pathSetMap = provider.branchChangedLines(branchConfiguration.branchTarget(), rootBaseDir, allPublishedFiles.keySet());
Map<Path, Set<Integer>> pathSetMap = provider.branchChangedLines(targetBranchName, rootBaseDir, changedFiles.keySet());
int count = 0; int count = 0;


if (pathSetMap == null) { if (pathSetMap == null) {
Expand All @@ -84,7 +90,7 @@ private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer)
return count; return count;
} }


for (Map.Entry<Path, DefaultInputFile> e : allPublishedFiles.entrySet()) { for (Map.Entry<Path, DefaultInputFile> e : changedFiles.entrySet()) {
Set<Integer> changedLines = pathSetMap.getOrDefault(e.getKey(), Collections.emptySet()); Set<Integer> changedLines = pathSetMap.getOrDefault(e.getKey(), Collections.emptySet());


if (changedLines.isEmpty()) { if (changedLines.isEmpty()) {
Expand Down
2 changes: 1 addition & 1 deletion sonar-scanner-engine/src/main/java/org/sonar/scanner/report/ComponentsPublisher.java
View file
Expand Up @@ -165,7 +165,7 @@ private boolean shouldSkipComponent(DefaultInputComponent component, Collection<
} else if (component instanceof DefaultInputFile) { } else if (component instanceof DefaultInputFile) {
// skip files not marked for publishing // skip files not marked for publishing
DefaultInputFile inputFile = (DefaultInputFile) component; DefaultInputFile inputFile = (DefaultInputFile) component;
return !inputFile.isPublished() || (branchConfiguration.isShortOrPullRequest() && inputFile.status() == Status.SAME); return !inputFile.isPublished();
} }
return false; return false;
} }
Expand Down
2 changes: 1 addition & 1 deletion sonar-scanner-engine/src/main/java/org/sonar/scanner/report/SourcePublisher.java
View file
Expand Up @@ -44,7 +44,7 @@ public SourcePublisher(InputComponentStore componentStore) {


@Override @Override
public void publish(ScannerReportWriter writer) { public void publish(ScannerReportWriter writer) {
for (final DefaultInputFile inputFile : componentCache.allFilesToPublish()) { for (final DefaultInputFile inputFile : componentCache.allChangedFilesToPublish()) {
File iofile = writer.getSourceFile(inputFile.batchId()); File iofile = writer.getSourceFile(inputFile.batchId());


try (OutputStream output = new BufferedOutputStream(new FileOutputStream(iofile)); try (OutputStream output = new BufferedOutputStream(new FileOutputStream(iofile));
Expand Down
16 changes: 12 additions & 4 deletions ...r-scanner-engine/src/main/java/org/sonar/scanner/scan/filesystem/InputComponentStore.java
View file
Expand Up @@ -31,12 +31,12 @@
import java.util.Map; import java.util.Map;
import java.util.SortedSet; import java.util.SortedSet;
import java.util.TreeSet; import java.util.TreeSet;
import java.util.stream.Stream;
import javax.annotation.CheckForNull; import javax.annotation.CheckForNull;
import org.sonar.api.batch.ScannerSide; import org.sonar.api.batch.ScannerSide;
import org.sonar.api.batch.fs.InputComponent; import org.sonar.api.batch.fs.InputComponent;
import org.sonar.api.batch.fs.InputDir; import org.sonar.api.batch.fs.InputDir;
import org.sonar.api.batch.fs.InputFile; 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.InputModule;
import org.sonar.api.batch.fs.internal.DefaultInputDir; import org.sonar.api.batch.fs.internal.DefaultInputDir;
import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile;
Expand Down Expand Up @@ -76,11 +76,19 @@ public Collection<InputComponent> all() {
return inputComponents.values(); return inputComponents.values();
} }


public Iterable<DefaultInputFile> allFilesToPublish() { private Stream<DefaultInputFile> allFilesToPublishStream() {
return inputFileCache.values().stream() return inputFileCache.values().stream()
.map(f -> (DefaultInputFile) f) .map(f -> (DefaultInputFile) f)
.filter(DefaultInputFile::isPublished) .filter(DefaultInputFile::isPublished);
.filter(f -> !branchConfiguration.isShortOrPullRequest() || f.status() != Status.SAME) }

public Iterable<DefaultInputFile> allFilesToPublish() {
return allFilesToPublishStream()::iterator;
}

public Iterable<DefaultInputFile> allChangedFilesToPublish() {
return allFilesToPublishStream()
.filter(f -> !branchConfiguration.isShortOrPullRequest() || f.status() != InputFile.Status.SAME)
::iterator; ::iterator;
} }


Expand Down
9 changes: 0 additions & 9 deletions sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java
View file
Expand Up @@ -231,9 +231,6 @@ public DefaultSensorStorage(MetricFinder metricFinder, ModuleIssues moduleIssues
public void store(Measure newMeasure) { public void store(Measure newMeasure) {
if (newMeasure.inputComponent() instanceof DefaultInputFile) { if (newMeasure.inputComponent() instanceof DefaultInputFile) {
DefaultInputFile defaultInputFile = (DefaultInputFile) newMeasure.inputComponent(); DefaultInputFile defaultInputFile = (DefaultInputFile) newMeasure.inputComponent();
if (shouldSkipStorage(defaultInputFile)) {
return;
}
defaultInputFile.setPublished(true); defaultInputFile.setPublished(true);
} }
saveMeasure(newMeasure.inputComponent(), (DefaultMeasure<?>) newMeasure); saveMeasure(newMeasure.inputComponent(), (DefaultMeasure<?>) newMeasure);
Expand All @@ -248,9 +245,6 @@ private void logOnce(String metricKey, String msg, Object... params) {
private void saveMeasure(InputComponent component, DefaultMeasure<?> measure) { private void saveMeasure(InputComponent component, DefaultMeasure<?> measure) {
if (component.isFile()) { if (component.isFile()) {
DefaultInputFile defaultInputFile = (DefaultInputFile) component; DefaultInputFile defaultInputFile = (DefaultInputFile) component;
if (shouldSkipStorage(defaultInputFile)) {
return;
}
defaultInputFile.setPublished(true); defaultInputFile.setPublished(true);
} }


Expand Down Expand Up @@ -471,9 +465,6 @@ public void store(DefaultSymbolTable symbolTable) {
@Override @Override
public void store(DefaultCoverage defaultCoverage) { public void store(DefaultCoverage defaultCoverage) {
DefaultInputFile inputFile = (DefaultInputFile) defaultCoverage.inputFile(); DefaultInputFile inputFile = (DefaultInputFile) defaultCoverage.inputFile();
if (shouldSkipStorage(inputFile)) {
return;
}
inputFile.setPublished(true); inputFile.setPublished(true);
if (defaultCoverage.linesToCover() > 0) { if (defaultCoverage.linesToCover() > 0) {
saveCoverageMetricInternal(inputFile, LINES_TO_COVER, new DefaultMeasure<Integer>().forMetric(LINES_TO_COVER).withValue(defaultCoverage.linesToCover())); saveCoverageMetricInternal(inputFile, LINES_TO_COVER, new DefaultMeasure<Integer>().forMetric(LINES_TO_COVER).withValue(defaultCoverage.linesToCover()));
Expand Down
10 changes: 5 additions & 5 deletions sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/branch/BranchMediumTest.java
View file
Expand Up @@ -70,7 +70,7 @@ public void prepare() throws IOException {
} }


@Test @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 // sanity check, normally report gets generated
TaskResult result = getResult(tester); TaskResult result = getResult(tester);
assertThat(getResult(tester).getReportComponent(result.inputFile(FILE_PATH).key())).isNotNull(); assertThat(getResult(tester).getReportComponent(result.inputFile(FILE_PATH).key())).isNotNull();
Expand All @@ -79,12 +79,12 @@ public void should_skip_report_for_unchanged_files_in_short_branch() {
assertThat(result.getReportReader().hasCoverage(fileId)).isTrue(); assertThat(result.getReportReader().hasCoverage(fileId)).isTrue();
assertThat(result.getReportReader().readFileSource(fileId)).isNotNull(); 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)); 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().readChangesets(fileId)).isNull();
assertThat(result2.getReportReader().hasCoverage(fileId)).isFalse(); assertThat(result2.getReportReader().hasCoverage(fileId)).isTrue();
assertThat(result.getReportReader().readFileSource(fileId)).isNull(); assertThat(result2.getReportReader().readFileSource(fileId)).isNull();
} }


@Test @Test
Expand Down
2 changes: 1 addition & 1 deletion sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ChangedLinesPublisherTest.java
View file
Expand Up @@ -119,7 +119,7 @@ public void write_changed_files() {
Set<Path> paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2"))); Set<Path> paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2")));
Set<Integer> lines = new HashSet<>(Arrays.asList(1, 10)); Set<Integer> lines = new HashSet<>(Arrays.asList(1, 10));
when(provider.branchChangedLines(TARGET_BRANCH, BASE_DIR, paths)).thenReturn(Collections.singletonMap(BASE_DIR.resolve("path1"), lines)); 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); publisher.publish(writer);


Expand Down
18 changes: 10 additions & 8 deletions sonar-scanner-engine/src/test/java/org/sonar/scanner/report/ComponentsPublisherTest.java
View file
Expand Up @@ -309,7 +309,7 @@ public void should_skip_empty_modules_for_short_living_branches() throws IOExcep
} }


@Test @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); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true);
ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class); ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class);
when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12")); when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12"));
Expand Down Expand Up @@ -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, 4)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue();


assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isFalse(); // do not skip, needed for computing overall coverage
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isFalse(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isFalse(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isTrue();
} }


@Test @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); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true);
ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class); ProjectAnalysisInfo projectAnalysisInfo = mock(ProjectAnalysisInfo.class);
when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12")); when(projectAnalysisInfo.analysisDate()).thenReturn(DateUtils.parseDate("2012-12-12"));
Expand Down Expand Up @@ -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, 4)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isTrue();


assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isFalse(); // do not skip, needed for computing overall coverage
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isFalse(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 3)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isFalse(); assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isTrue();
} }


@Test @Test
Expand Down
24 changes: 8 additions & 16 deletions sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java
View file
Expand Up @@ -86,6 +86,7 @@ public void prepare() throws Exception {
MetricFinder metricFinder = mock(MetricFinder.class); MetricFinder metricFinder = mock(MetricFinder.class);
when(metricFinder.<Integer>findByKey(CoreMetrics.NCLOC_KEY)).thenReturn(CoreMetrics.NCLOC); when(metricFinder.<Integer>findByKey(CoreMetrics.NCLOC_KEY)).thenReturn(CoreMetrics.NCLOC);
when(metricFinder.<String>findByKey(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION_KEY)).thenReturn(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION); when(metricFinder.<String>findByKey(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION_KEY)).thenReturn(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION);
when(metricFinder.<Integer>findByKey(CoreMetrics.LINES_TO_COVER_KEY)).thenReturn(CoreMetrics.LINES_TO_COVER);


settings = new MapSettings(); settings = new MapSettings();
moduleIssues = mock(ModuleIssues.class); moduleIssues = mock(ModuleIssues.class);
Expand Down Expand Up @@ -191,29 +192,20 @@ public void should_save_file_measure() {
} }


@Test @Test
public void should_skip_file_measure_on_short_branch_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);

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() {
InputFile file = new TestInputFileBuilder("foo", "src/Foo.php").setStatus(InputFile.Status.SAME).build(); InputFile file = new TestInputFileBuilder("foo", "src/Foo.php").setStatus(InputFile.Status.SAME).build();
when(branchConfiguration.isShortOrPullRequest()).thenReturn(true); when(branchConfiguration.isShortOrPullRequest()).thenReturn(true);


ArgumentCaptor<DefaultMeasure> 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() underTest.store(new DefaultMeasure()
.on(file) .on(file)
.forMetric(CoreMetrics.NCLOC) .forMetric(CoreMetrics.LINES_TO_COVER)
.withValue(10)); .withValue(10));


verifyZeroInteractions(measureCache); DefaultMeasure m = argumentCaptor.getValue();
assertThat(m.value()).isEqualTo(10);
assertThat(m.metric()).isEqualTo(CoreMetrics.LINES_TO_COVER);
} }


@Test @Test
Expand Down

0 comments on commit adb7939

Please sign in to comment.