From 946a448c08ccc10ad1c0af778ae5d9f179655ef8 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Tue, 29 Mar 2016 18:13:12 +0200 Subject: [PATCH] SONAR-7497 Restore behavior for line coverage after move to proto3 --- .../src/test/java/it/test/CoverageTest.java | 31 +++++--- .../CoverageTest/it_coverage-expected.json | 4 +- .../source/CoverageLineReader.java | 23 ++++-- .../source/CoverageLineReaderTest.java | 66 +++++++++++++++- .../sonar/batch/report/CoveragePublisher.java | 20 +++-- .../batch/report/CoveragePublisherTest.java | 75 +++++++++++++++++++ .../src/main/protobuf/scanner_report.proto | 20 +++-- 7 files changed, 210 insertions(+), 29 deletions(-) diff --git a/it/it-tests/src/test/java/it/test/CoverageTest.java b/it/it-tests/src/test/java/it/test/CoverageTest.java index 9c717b6cb52..a76ba175497 100644 --- a/it/it-tests/src/test/java/it/test/CoverageTest.java +++ b/it/it-tests/src/test/java/it/test/CoverageTest.java @@ -22,18 +22,18 @@ package it.test; import com.sonar.orchestrator.Orchestrator; import com.sonar.orchestrator.build.SonarRunner; import it.Category2Suite; - import java.io.File; - import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.io.filefilter.TrueFileFilter; +import org.apache.commons.lang.StringUtils; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.skyscreamer.jsonassert.JSONAssert; import org.sonar.wsclient.services.Resource; import org.sonar.wsclient.services.ResourceQuery; + import static org.assertj.core.api.Assertions.assertThat; import static util.ItUtils.projectDir; @@ -71,12 +71,20 @@ public class CoverageTest { assertThat(project.getMeasureValue("overall_coverage")).isNull(); - String coverage = orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-ut-coverage:src/main/xoo/sample/Sample.xoo"); - JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/unit_test_coverage-expected.json"), "UTF-8"), coverage, false); + String coverage = cleanupScmAndDuplication(orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-ut-coverage:src/main/xoo/sample/Sample.xoo")); + // Use strict checking to be sure IT coverage is not present + JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/unit_test_coverage-expected.json"), "UTF-8"), coverage, true); verifyComputeEngineTempDirIsEmpty(); } + private String cleanupScmAndDuplication(String coverage) { + coverage = StringUtils.remove(coverage, ",\"scmAuthor\":\"\""); + coverage = StringUtils.remove(coverage, ",\"scmRevision\":\"\""); + coverage = StringUtils.remove(coverage, ",\"duplicated\":false"); + return coverage; + } + @Test public void unit_test_coverage_no_condition() throws Exception { orchestrator.executeBuilds(SonarRunner.create(projectDir("testing/xoo-sample-ut-coverage-no-condition"))); @@ -94,9 +102,10 @@ public class CoverageTest { assertThat(project.getMeasureValue("overall_coverage")).isNull(); - String coverage = orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-ut-coverage:src/main/xoo/sample/Sample.xoo"); + String coverage = cleanupScmAndDuplication(orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-ut-coverage:src/main/xoo/sample/Sample.xoo")); + // Use strict checking to be sure IT coverage is not present JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/unit_test_coverage_no_condition-expected.json"), "UTF-8"), coverage, - false); + true); verifyComputeEngineTempDirIsEmpty(); } @@ -118,8 +127,9 @@ public class CoverageTest { assertThat(project.getMeasureValue("overall_coverage")).isNull(); - String coverage = orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-it-coverage:src/main/xoo/sample/Sample.xoo"); - JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/it_coverage-expected.json"), "UTF-8"), coverage, false); + String coverage = cleanupScmAndDuplication(orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-it-coverage:src/main/xoo/sample/Sample.xoo")); + // Use strict checking to be sure UT coverage is not present + JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/it_coverage-expected.json"), "UTF-8"), coverage, true); verifyComputeEngineTempDirIsEmpty(); } @@ -153,8 +163,9 @@ public class CoverageTest { assertThat(project.getMeasureValue("overall_uncovered_conditions")).isEqualTo(2); assertThat(project.getMeasureValue("overall_coverage")).isEqualTo(62.5); - String coverage = orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-overall-coverage:src/main/xoo/sample/Sample.xoo"); - JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/ut_and_it_coverage-expected.json"), "UTF-8"), coverage, false); + String coverage = cleanupScmAndDuplication(orchestrator.getServer().adminWsClient().get("api/sources/lines", "key", "sample-overall-coverage:src/main/xoo/sample/Sample.xoo")); + // Use strict checking to be sure no extra coverage is present + JSONAssert.assertEquals(IOUtils.toString(this.getClass().getResourceAsStream("/test/CoverageTest/ut_and_it_coverage-expected.json"), "UTF-8"), coverage, true); verifyComputeEngineTempDirIsEmpty(); } diff --git a/it/it-tests/src/test/resources/test/CoverageTest/it_coverage-expected.json b/it/it-tests/src/test/resources/test/CoverageTest/it_coverage-expected.json index 443362d0d6b..e18e570ff1e 100644 --- a/it/it-tests/src/test/resources/test/CoverageTest/it_coverage-expected.json +++ b/it/it-tests/src/test/resources/test/CoverageTest/it_coverage-expected.json @@ -40,7 +40,9 @@ { "line": 10, "code": "\t\tif (foo == bar) {", - "itLineHits": 0 + "itLineHits": 0, + "itConditions": 2, + "itCoveredConditions": 1 }, { "line": 11, diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/CoverageLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/CoverageLineReader.java index 19f8a80d611..9f30c7adfbe 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/CoverageLineReader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/CoverageLineReader.java @@ -23,6 +23,11 @@ import java.util.Iterator; import javax.annotation.CheckForNull; import org.sonar.db.protobuf.DbFileSources; import org.sonar.scanner.protocol.output.ScannerReport; +import org.sonar.scanner.protocol.output.ScannerReport.LineCoverage.HasItCoveredConditionsCase; +import org.sonar.scanner.protocol.output.ScannerReport.LineCoverage.HasItHitsCase; +import org.sonar.scanner.protocol.output.ScannerReport.LineCoverage.HasOverallCoveredConditionsCase; +import org.sonar.scanner.protocol.output.ScannerReport.LineCoverage.HasUtCoveredConditionsCase; +import org.sonar.scanner.protocol.output.ScannerReport.LineCoverage.HasUtHitsCase; public class CoverageLineReader implements LineReader { @@ -45,24 +50,30 @@ public class CoverageLineReader implements LineReader { } private static void processUnitTest(DbFileSources.Line.Builder lineBuilder, ScannerReport.LineCoverage reportCoverage) { - lineBuilder.setUtLineHits(reportCoverage.getUtHits() ? 1 : 0); - if (reportCoverage.getConditions() > 0) { + if (reportCoverage.getHasUtHitsCase() == HasUtHitsCase.UT_HITS) { + lineBuilder.setUtLineHits(reportCoverage.getUtHits() ? 1 : 0); + } + if (reportCoverage.getHasUtCoveredConditionsCase() == HasUtCoveredConditionsCase.UT_COVERED_CONDITIONS) { lineBuilder.setUtConditions(reportCoverage.getConditions()); lineBuilder.setUtCoveredConditions(reportCoverage.getUtCoveredConditions()); } } private static void processIntegrationTest(DbFileSources.Line.Builder lineBuilder, ScannerReport.LineCoverage reportCoverage) { - lineBuilder.setItLineHits(reportCoverage.getItHits() ? 1 : 0); - if (reportCoverage.getConditions() > 0) { + if (reportCoverage.getHasItHitsCase() == HasItHitsCase.IT_HITS) { + lineBuilder.setItLineHits(reportCoverage.getItHits() ? 1 : 0); + } + if (reportCoverage.getHasItCoveredConditionsCase() == HasItCoveredConditionsCase.IT_COVERED_CONDITIONS) { lineBuilder.setItConditions(reportCoverage.getConditions()); lineBuilder.setItCoveredConditions(reportCoverage.getItCoveredConditions()); } } private static void processOverallTest(DbFileSources.Line.Builder lineBuilder, ScannerReport.LineCoverage reportCoverage) { - lineBuilder.setOverallLineHits((reportCoverage.getUtHits() || reportCoverage.getItHits()) ? 1 : 0); - if (reportCoverage.getConditions() > 0) { + if (reportCoverage.getHasUtHitsCase() == HasUtHitsCase.UT_HITS || reportCoverage.getHasItHitsCase() == HasItHitsCase.IT_HITS) { + lineBuilder.setOverallLineHits((reportCoverage.getUtHits() || reportCoverage.getItHits()) ? 1 : 0); + } + if (reportCoverage.getHasOverallCoveredConditionsCase() == HasOverallCoveredConditionsCase.OVERALL_COVERED_CONDITIONS) { lineBuilder.setOverallConditions(reportCoverage.getConditions()); lineBuilder.setOverallCoveredConditions(reportCoverage.getOverallCoveredConditions()); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/CoverageLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/CoverageLineReaderTest.java index 1f211b59363..881e9a4bed7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/CoverageLineReaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/CoverageLineReaderTest.java @@ -53,6 +53,70 @@ public class CoverageLineReaderTest { assertThat(lineBuilder.getOverallConditions()).isEqualTo(10); } + // Some tools are only able to report condition coverage + @Test + public void set_coverage_only_conditions() { + CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() + .setLine(1) + .setConditions(10) + .setUtCoveredConditions(2) + .setItCoveredConditions(3) + .build()).iterator()); + + DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); + computeCoverageLine.read(lineBuilder); + + assertThat(lineBuilder.hasUtLineHits()).isFalse(); + assertThat(lineBuilder.getUtConditions()).isEqualTo(10); + assertThat(lineBuilder.hasItLineHits()).isFalse(); + assertThat(lineBuilder.getItConditions()).isEqualTo(10); + assertThat(lineBuilder.getItCoveredConditions()).isEqualTo(3); + assertThat(lineBuilder.hasOverallLineHits()).isFalse(); + assertThat(lineBuilder.hasOverallConditions()).isFalse(); + } + + @Test + public void set_coverage_only_ut() { + CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() + .setLine(1) + .setConditions(10) + .setUtHits(true) + .setUtCoveredConditions(2) + .build()).iterator()); + + DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); + computeCoverageLine.read(lineBuilder); + + assertThat(lineBuilder.getUtLineHits()).isEqualTo(1); + assertThat(lineBuilder.getUtConditions()).isEqualTo(10); + assertThat(lineBuilder.hasItLineHits()).isFalse(); + assertThat(lineBuilder.hasItConditions()).isFalse(); + assertThat(lineBuilder.hasItCoveredConditions()).isFalse(); + assertThat(lineBuilder.getOverallLineHits()).isEqualTo(1); + assertThat(lineBuilder.hasOverallCoveredConditions()).isFalse(); + } + + @Test + public void set_coverage_only_it() { + CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() + .setLine(1) + .setConditions(10) + .setItHits(true) + .setItCoveredConditions(3) + .build()).iterator()); + + DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); + computeCoverageLine.read(lineBuilder); + + assertThat(lineBuilder.hasUtLineHits()).isFalse(); + assertThat(lineBuilder.hasUtConditions()).isFalse(); + assertThat(lineBuilder.getItLineHits()).isEqualTo(1); + assertThat(lineBuilder.getItConditions()).isEqualTo(10); + assertThat(lineBuilder.getItCoveredConditions()).isEqualTo(3); + assertThat(lineBuilder.getOverallLineHits()).isEqualTo(1); + assertThat(lineBuilder.hasOverallConditions()).isFalse(); + } + @Test public void set_coverage_on_uncovered_lines() { CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() @@ -77,7 +141,6 @@ public class CoverageLineReaderTest { CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() .setLine(1) .setUtHits(true) - .setItHits(false) .build()).iterator()); DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(1); @@ -90,7 +153,6 @@ public class CoverageLineReaderTest { public void set_overall_line_hits_with_only_it() { CoverageLineReader computeCoverageLine = new CoverageLineReader(newArrayList(ScannerReport.LineCoverage.newBuilder() .setLine(1) - .setUtHits(false) .setItHits(true) .build()).iterator()); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/batch/report/CoveragePublisher.java b/sonar-scanner-engine/src/main/java/org/sonar/batch/report/CoveragePublisher.java index 1d8d5fee448..1ed96d1ceb7 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/batch/report/CoveragePublisher.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/batch/report/CoveragePublisher.java @@ -62,25 +62,35 @@ public class CoveragePublisher implements ReportPublisherStep { builder.setUtHits(Integer.parseInt(value) > 0); } }); - applyLineMeasure(resource.key(), lineCount, CoreMetrics.CONDITIONS_BY_LINE_KEY, coveragePerLine, + applyLineMeasure(resource.key(), lineCount, CoreMetrics.IT_COVERAGE_LINE_HITS_DATA_KEY, coveragePerLine, + new MeasureOperation() { + @Override + public void apply(String value, LineCoverage.Builder builder) { + builder.setItHits(Integer.parseInt(value) > 0); + } + }); + // Just in case we have only IT and no UT, we first take conditions from ITs, then from UTs (UTs will override ITs). + // Note that it would be very strange (and unsupported in scanner report) to have ITs and UTs don't report the same number of + // conditions. Not even talking about overall conditions... + applyLineMeasure(resource.key(), lineCount, CoreMetrics.IT_CONDITIONS_BY_LINE_KEY, coveragePerLine, new MeasureOperation() { @Override public void apply(String value, LineCoverage.Builder builder) { builder.setConditions(Integer.parseInt(value)); } }); - applyLineMeasure(resource.key(), lineCount, CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY, coveragePerLine, + applyLineMeasure(resource.key(), lineCount, CoreMetrics.CONDITIONS_BY_LINE_KEY, coveragePerLine, new MeasureOperation() { @Override public void apply(String value, LineCoverage.Builder builder) { - builder.setUtCoveredConditions(Integer.parseInt(value)); + builder.setConditions(Integer.parseInt(value)); } }); - applyLineMeasure(resource.key(), lineCount, CoreMetrics.IT_COVERAGE_LINE_HITS_DATA_KEY, coveragePerLine, + applyLineMeasure(resource.key(), lineCount, CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY, coveragePerLine, new MeasureOperation() { @Override public void apply(String value, LineCoverage.Builder builder) { - builder.setItHits(Integer.parseInt(value) > 0); + builder.setUtCoveredConditions(Integer.parseInt(value)); } }); applyLineMeasure(resource.key(), lineCount, CoreMetrics.IT_COVERED_CONDITIONS_BY_LINE_KEY, coveragePerLine, diff --git a/sonar-scanner-engine/src/test/java/org/sonar/batch/report/CoveragePublisherTest.java b/sonar-scanner-engine/src/test/java/org/sonar/batch/report/CoveragePublisherTest.java index 1175661f8b9..b040483a57b 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/batch/report/CoveragePublisherTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/batch/report/CoveragePublisherTest.java @@ -70,6 +70,9 @@ public class CoveragePublisherTest { Measure utLineHits = new Measure<>(CoreMetrics.COVERAGE_LINE_HITS_DATA).setData("2=1;3=1;5=0;6=3"); when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.COVERAGE_LINE_HITS_DATA_KEY)).thenReturn(utLineHits); + Measure itsConditionsByLine = new Measure<>(CoreMetrics.IT_CONDITIONS_BY_LINE).setData("3=4"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.IT_CONDITIONS_BY_LINE_KEY)).thenReturn(itsConditionsByLine); + Measure conditionsByLine = new Measure<>(CoreMetrics.CONDITIONS_BY_LINE).setData("3=4"); when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.CONDITIONS_BY_LINE_KEY)).thenReturn(conditionsByLine); @@ -113,4 +116,76 @@ public class CoveragePublisherTest { } } + + @Test + public void publishCoverageOnlyUts() throws Exception { + + Measure utLineHits = new Measure<>(CoreMetrics.COVERAGE_LINE_HITS_DATA).setData("2=1;3=1;5=0;6=3"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.COVERAGE_LINE_HITS_DATA_KEY)).thenReturn(utLineHits); + + Measure conditionsByLine = new Measure<>(CoreMetrics.CONDITIONS_BY_LINE).setData("3=4"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.CONDITIONS_BY_LINE_KEY)).thenReturn(conditionsByLine); + + Measure coveredConditionsByUts = new Measure<>(CoreMetrics.COVERED_CONDITIONS_BY_LINE).setData("3=2"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.COVERED_CONDITIONS_BY_LINE_KEY)).thenReturn(coveredConditionsByUts); + + File outputDir = temp.newFolder(); + ScannerReportWriter writer = new ScannerReportWriter(outputDir); + + publisher.publish(writer); + + try (CloseableIterator it = new ScannerReportReader(outputDir).readComponentCoverage(2)) { + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(2) + .setUtHits(true) + .build()); + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(3) + .setUtHits(true) + .setConditions(4) + .setUtCoveredConditions(2) + .build()); + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(5) + .setUtHits(false) + .build()); + } + + } + + @Test + public void publishCoverageOnlyIts() throws Exception { + + Measure itsConditionsByLine = new Measure<>(CoreMetrics.IT_CONDITIONS_BY_LINE).setData("3=4"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.IT_CONDITIONS_BY_LINE_KEY)).thenReturn(itsConditionsByLine); + + Measure itLineHits = new Measure<>(CoreMetrics.IT_COVERAGE_LINE_HITS_DATA).setData("2=0;3=0;5=1"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.IT_COVERAGE_LINE_HITS_DATA_KEY)).thenReturn(itLineHits); + + Measure coveredConditionsByIts = new Measure<>(CoreMetrics.IT_COVERED_CONDITIONS_BY_LINE).setData("3=1"); + when(measureCache.byMetric("foo:src/Foo.php", CoreMetrics.IT_COVERED_CONDITIONS_BY_LINE_KEY)).thenReturn(coveredConditionsByIts); + + File outputDir = temp.newFolder(); + ScannerReportWriter writer = new ScannerReportWriter(outputDir); + + publisher.publish(writer); + + try (CloseableIterator it = new ScannerReportReader(outputDir).readComponentCoverage(2)) { + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(2) + .setItHits(false) + .build()); + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(3) + .setItHits(false) + .setConditions(4) + .setItCoveredConditions(1) + .build()); + assertThat(it.next()).isEqualTo(LineCoverage.newBuilder() + .setLine(5) + .setItHits(true) + .build()); + } + + } } diff --git a/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto b/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto index 0d958a4eef7..f7ef467f06b 100644 --- a/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto +++ b/sonar-scanner-protocol/src/main/protobuf/scanner_report.proto @@ -216,15 +216,25 @@ message LineCoverage { // Number of conditions to cover (if set, the value must be greater than 0) int32 conditions = 2; // Is the line has been touched by a unit test ? Returning false means that no test has touched this executable line. - bool ut_hits = 3; + oneof has_ut_hits { + bool ut_hits = 3; + } // Is the line has been touched by a integration test ? Returning false means that no test has touched this executable line. - bool it_hits = 4; + oneof has_it_hits { + bool it_hits = 4; + } // Number of conditions covered by unit tests - int32 ut_covered_conditions = 5; + oneof has_ut_covered_conditions { + int32 ut_covered_conditions = 5; + } // Number of conditions covered by integration tests - int32 it_covered_conditions = 6; + oneof has_it_covered_conditions { + int32 it_covered_conditions = 6; + } // Number of conditions covered by overall tests - int32 overall_covered_conditions = 7; + oneof has_overall_covered_conditions { + int32 overall_covered_conditions = 7; + } } // Must be sorted by line and start offset -- 2.39.5