From 568869df726ae8de653ac8ad41ff60a97441abc9 Mon Sep 17 00:00:00 2001 From: Andrew Knowles Date: Wed, 27 Sep 2017 16:01:37 -0400 Subject: [PATCH 1/5] Support configurable metrics for Jacoco and SonarQube... --- .../githubprcoveragestatus/Configuration.java | 10 ++ .../githubprcoveragestatus/JacocoParser.java | 32 +++-- .../SettingsRepository.java | 2 + .../SonarMasterCoverageRepository.java | 9 +- .../Configuration/global.groovy | 4 + .../help-sonarCoverageMetric.html | 3 + .../JacocoParserTest.java | 47 +++++++- .../SonarMasterCoverageRepositoryTest.java | 111 +++++++++++++++--- .../measureFound.json | 2 +- .../measureFoundBranch.json | 28 +++++ .../measureFoundLine.json | 28 +++++ 11 files changed, 244 insertions(+), 32 deletions(-) create mode 100644 src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-sonarCoverageMetric.html create mode 100644 src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundBranch.json create mode 100644 src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundLine.json diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java index 1121343..92eb46b 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java @@ -77,6 +77,10 @@ public static Boolean isUseSonarForMasterCoverage() { return DESCRIPTOR.isUseSonarForMasterCoverage(); } + public static String getSonarCoverageMetric() { + return DESCRIPTOR.getSonarCoverageMetric(); + } + public static void setMasterCoverage(final String repo, final float coverage) { DESCRIPTOR.set(repo, coverage); } @@ -104,6 +108,7 @@ public static final class ConfigurationDescriptor extends Descriptor + Coverage metric to retrieve from Sonar(Optional). It will use "coverage" if not specified. Known values are "coverage", "line_coverage", and "branch_coverage". + \ No newline at end of file diff --git a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java index 25ef44b..0813ccb 100644 --- a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java +++ b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java @@ -18,17 +18,56 @@ package com.github.terma.jenkins.githubprcoveragestatus; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; + +import static org.mockito.Mockito.mock; import java.io.IOException; public class JacocoParserTest { + private SettingsRepository settingsRepository = mock(SettingsRepository.class); + + @Before + public void initMocks() throws IOException { + ServiceRegistry.setSettingsRepository(settingsRepository); + } + + @Test + public void extractCoverageFromJacocoReportDefault() throws IOException { + String filePath = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(null); + Assert.assertEquals(0.22, new JacocoParser().get(filePath), 0.1); + } + + @Test + public void extractCoverageFromJacocoReportInstruction() throws IOException { + String filePath = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME); + Assert.assertEquals(0.22, new JacocoParser().get(filePath), 0.1); + } + + @Test + public void extractCoverageFromJacocoReportLine() throws IOException { + String filePath = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME); + Assert.assertEquals(0.22, new JacocoParser().get(filePath), 0.1); + } + @Test - public void extractCoverageFromJacocoReport() throws IOException { + public void extractCoverageFromJacocoReportBranch() throws IOException { String filePath = JacocoParserTest.class.getResource( "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME); Assert.assertEquals(0.22, new JacocoParser().get(filePath), 0.1); } @@ -59,7 +98,7 @@ public void throwExceptionWhenExtractCoverageFromJacocoAndNoLineTag() throws IOE " \"report.dtd\">\n" + "\n" + "", - messageWithoutAbsolutePath); + messageWithoutAbsolutePath.replace("\r\n", "\n")); } } @@ -83,7 +122,7 @@ public void throwExceptionWhenExtractCoverageFromJacocoAndMissedNotNumber() thro "\n" + " \n" + "", - messageWithoutAbsolutePath); + messageWithoutAbsolutePath.replace("\r\n", "\n")); } } @@ -107,7 +146,7 @@ public void throwExceptionWhenExtractCoverageFromJacocoAndCoveredNotNumber() thr "\n" + " \n" + "", - messageWithoutAbsolutePath); + messageWithoutAbsolutePath.replace("\r\n", "\n")); } } diff --git a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest.java b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest.java index 3e3b964..a0c46e8 100644 --- a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest.java +++ b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest.java @@ -1,21 +1,28 @@ package com.github.terma.jenkins.githubprcoveragestatus; -import com.github.tomakehurst.wiremock.client.MappingBuilder; -import com.github.tomakehurst.wiremock.junit.WireMockRule; -import org.apache.commons.io.IOUtils; -import org.junit.After; -import org.junit.ClassRule; -import org.junit.Test; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; +import static com.google.common.base.Charsets.UTF_8; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.mockito.Mockito.mock; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; -import static com.github.tomakehurst.wiremock.client.WireMock.*; -import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; -import static com.google.common.base.Charsets.UTF_8; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; +import org.apache.commons.io.IOUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.mockito.Mockito; + +import com.github.tomakehurst.wiremock.client.MappingBuilder; +import com.github.tomakehurst.wiremock.junit.WireMockRule; public class SonarMasterCoverageRepositoryTest { @@ -27,6 +34,13 @@ public class SonarMasterCoverageRepositoryTest { private SonarMasterCoverageRepository sonarMasterCoverageRepository; private ByteArrayOutputStream buildLogOutputStream; + private SettingsRepository settingsRepository = mock(SettingsRepository.class); + + @Before + public void initMocks() throws IOException { + ServiceRegistry.setSettingsRepository(settingsRepository); + } + @After public void afterTest() throws Exception { System.out.println(buildLogOutputStream.toString()); @@ -34,20 +48,43 @@ public void afterTest() throws Exception { } @Test - public void should_get_coverage() throws IOException { - testCoverage(null, null); - testCoverage("token", ""); - testCoverage("login", "password"); + public void should_get_coverage_default() throws IOException { + testCoverage(null, null, null, 0.953f); + testCoverage("token", "", null, 0.953f); + testCoverage("login", "password", null, 0.953f); + } + + @Test + public void should_get_coverage_branch() throws IOException { + testCoverage(null, null, SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME, 0.753f); + testCoverage("token", "", SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME, 0.753f); + testCoverage("login", "password", SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME, 0.753f); + } + + @Test + public void should_get_coverage_instruction() throws IOException { + testCoverage(null, null, SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME, 0.953f); + testCoverage("token", "", SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME, 0.953f); + testCoverage("login", "password", SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME, 0.953f); } - private void testCoverage(final String login, final String password) throws IOException { + @Test + public void should_get_coverage_line() throws IOException { + testCoverage(null, null, SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME, 0.852f); + testCoverage("token", "", SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME, 0.852f); + testCoverage("login", "password", SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME, 0.852f); + } + + private void testCoverage(final String login, final String password, final String metric, final float expected) throws IOException { givenCoverageRepository(login, password); givenProjectResponseWithSingleMatch(login, password); givenMeasureResponse(); + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(metric); + final float coverage = sonarMasterCoverageRepository.get(GIT_REPO_URL); - assertThat(coverage, is(0.953f)); + assertThat(coverage, is(expected)); } @Test @@ -57,6 +94,8 @@ public void should_get_coverage_for_multiple_projects_found() throws IOException givenProjectResponseWithMultipleMatches(); givenMeasureResponse(); + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(null); + final float coverage = sonarMasterCoverageRepository.get(GIT_REPO_URL); assertThat(coverage, is(0.953f)); } @@ -67,6 +106,8 @@ public void should_get_zero_coverage_for_not_found() { givenProjectResponseWithoutMatch(); + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn(null); + assertThat(sonarMasterCoverageRepository.get(GIT_REPO_URL), is(0f)); } @@ -77,6 +118,8 @@ public void should_get_zero_coverage_for_unknown_metric() throws IOException { givenProjectResponseWithSingleMatch(null, null); givenNotFoundMeasureResponse(); + Mockito.when(settingsRepository.getSonarCoverageMetric()).thenReturn("unknown_coverage"); + assertThat(sonarMasterCoverageRepository.get(GIT_REPO_URL), is(0f)); } @@ -122,15 +165,39 @@ private void givenProjectResponseWithoutMatch() { private void givenMeasureResponse() throws IOException { wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) .withQueryParam("componentKey", equalTo("my-project:origin/master")) - .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME)) + .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME)) .willReturn(aResponse() .withStatus(200) .withBody(getResponseBodyFromFile("measureFound.json")) ) ); + wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) + .withQueryParam("componentKey", equalTo("my-project:origin/master")) + .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME)) + .willReturn(aResponse() + .withStatus(200) + .withBody(getResponseBodyFromFile("measureFoundLine.json")) + ) + ); + wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) + .withQueryParam("componentKey", equalTo("my-project:origin/master")) + .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME)) + .willReturn(aResponse() + .withStatus(200) + .withBody(getResponseBodyFromFile("measureFoundBranch.json")) + ) + ); } private void givenNotFoundMeasureResponse() throws IOException { + wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) + .withQueryParam("componentKey", equalTo("my-project:origin/master")) + .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_INSTRUCTION_COVERAGE_METRIC_NAME)) + .willReturn(aResponse() + .withStatus(404) + .withBody(getResponseBodyFromFile("metricNotFound.json")) + ) + ); wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) .withQueryParam("componentKey", equalTo("my-project:origin/master")) .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_LINE_COVERAGE_METRIC_NAME)) @@ -139,6 +206,14 @@ private void givenNotFoundMeasureResponse() throws IOException { .withBody(getResponseBodyFromFile("metricNotFound.json")) ) ); + wireMockRule.stubFor(get(urlPathEqualTo("/api/measures/component")) + .withQueryParam("componentKey", equalTo("my-project:origin/master")) + .withQueryParam("metricKeys", equalTo(SonarMasterCoverageRepository.SONAR_OVERALL_BRANCH_COVERAGE_METRIC_NAME)) + .willReturn(aResponse() + .withStatus(404) + .withBody(getResponseBodyFromFile("metricNotFound.json")) + ) + ); } private String getResponseBodyFromFile(String fileName) throws IOException { diff --git a/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFound.json b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFound.json index 18a4e55..6924982 100644 --- a/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFound.json +++ b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFound.json @@ -4,7 +4,7 @@ "key": "my-project:origin/master", "measures": [ { - "metric": "overall_line_coverage", + "metric": "coverage", "periods": [ { "index": 1, diff --git a/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundBranch.json b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundBranch.json new file mode 100644 index 0000000..39eca67 --- /dev/null +++ b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundBranch.json @@ -0,0 +1,28 @@ +{ + "component": { + "id": "AVePr_fQwz0tAcNAWEGz", + "key": "my-project:origin/master", + "measures": [ + { + "metric": "branch_coverage", + "periods": [ + { + "index": 1, + "value": "0.0" + }, + { + "index": 2, + "value": "0.0" + }, + { + "index": 3, + "value": "0.0" + } + ], + "value": "75.3" + } + ], + "name": "my-project origin/master", + "qualifier": "TRK" + } +} \ No newline at end of file diff --git a/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundLine.json b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundLine.json new file mode 100644 index 0000000..d29859a --- /dev/null +++ b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepositoryTest/measureFoundLine.json @@ -0,0 +1,28 @@ +{ + "component": { + "id": "AVePr_fQwz0tAcNAWEGz", + "key": "my-project:origin/master", + "measures": [ + { + "metric": "line_coverage", + "periods": [ + { + "index": 1, + "value": "0.0" + }, + { + "index": 2, + "value": "0.0" + }, + { + "index": 3, + "value": "0.0" + } + ], + "value": "85.2" + } + ], + "name": "my-project origin/master", + "qualifier": "TRK" + } +} \ No newline at end of file From 96917ba05d13a03109df26aaceb53d5f05fb8470 Mon Sep 17 00:00:00 2001 From: Andrew Knowles Date: Wed, 27 Sep 2017 17:29:16 -0400 Subject: [PATCH 2/5] Fix input box for metric. Add in customizable rounding to match sonar. And some logging. --- .../CompareCoverageAction.java | 8 +++++ .../githubprcoveragestatus/Configuration.java | 14 ++++++++- .../githubprcoveragestatus/Message.java | 10 +++++-- .../githubprcoveragestatus/Percent.java | 5 ++++ .../SettingsRepository.java | 2 ++ .../Configuration/global.groovy | 6 +++- .../help-coverageRoundingDigits.html | 3 ++ .../githubprcoveragestatus/MessageTest.java | 29 ++++++++++++++++++- 8 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-coverageRoundingDigits.html diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java index 5508bf5..dfdfe9b 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java @@ -105,6 +105,14 @@ public void perform( final int prId = PrIdAndUrlUtils.getPrId(scmVars, build, listener); final String gitUrl = PrIdAndUrlUtils.getGitUrl(scmVars, build, listener); + if (settingsRepository.getSonarCoverageMetric() != null) { + buildLog.println(BUILD_LOG_PREFIX + "using coverage metrics: " + settingsRepository.getSonarCoverageMetric()); + } + + if (settingsRepository.getCoverageRoundingDigits() != 0) { + buildLog.println(BUILD_LOG_PREFIX + "using round-up to digits: " + settingsRepository.getCoverageRoundingDigits()); + } + buildLog.println(BUILD_LOG_PREFIX + "getting master coverage..."); MasterCoverageRepository masterCoverageRepository = ServiceRegistry.getMasterCoverageRepository(buildLog, sonarLogin, sonarPassword); final GHRepository gitHubRepository = ServiceRegistry.getPullRequestRepository().getGitHubRepository(gitUrl); diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java index 92eb46b..35f4a61 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java @@ -78,7 +78,11 @@ public static Boolean isUseSonarForMasterCoverage() { } public static String getSonarCoverageMetric() { - return DESCRIPTOR.getSonarCoverageMetric(); + return DESCRIPTOR.getSonarCoverageMetric(); + } + + public static int getCoverageRoundingDigits() { + return DESCRIPTOR.getCoverageRoundingDigits(); } public static void setMasterCoverage(final String repo, final float coverage) { @@ -112,6 +116,7 @@ public static final class ConfigurationDescriptor extends Descriptor 0 ? "+" : "") + toWholeNoSignString(value); } diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java index e492b3c..3734587 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java @@ -23,4 +23,6 @@ interface SettingsRepository { String getSonarToken(); String getSonarCoverageMetric(); + + int getCoverageRoundingDigits(); } diff --git a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy index 4ecfd3e..53565cf 100644 --- a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy +++ b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy @@ -49,7 +49,11 @@ f.section(title: descriptor.displayName) { } f.entry(field: "sonarCoverageMetric", title: _("Sonar coverage metric")) { - f.password() + f.textbox() + } + + f.entry(field: "coverageRoundingDigits", title: _("Rounding digits")) { + f.textbox() } f.entry(field: "disableSimpleCov", title: _("Disable SimpleCov coverage parser")) { diff --git a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-coverageRoundingDigits.html b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-coverageRoundingDigits.html new file mode 100644 index 0000000..4dbb7f3 --- /dev/null +++ b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-coverageRoundingDigits.html @@ -0,0 +1,3 @@ +
+ Number of digits to round at(Optional). If not specified, no rounding will be performed. In order to match SonarQube, it always rounds up. +
\ No newline at end of file diff --git a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/MessageTest.java b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/MessageTest.java index 642e1ac..72c4900 100644 --- a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/MessageTest.java +++ b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/MessageTest.java @@ -17,13 +17,26 @@ */ package com.github.terma.jenkins.githubprcoveragestatus; +import static org.mockito.Mockito.mock; + +import java.io.IOException; + import org.junit.Assert; +import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; public class MessageTest { + private SettingsRepository settingsRepository = mock(SettingsRepository.class); + + @Before + public void initMocks() throws IOException { + ServiceRegistry.setSettingsRepository(settingsRepository); + } @Test - public void buildNiceForConsole() { + public void buildNiceForConsoleDefault() { + Mockito.when(settingsRepository.getCoverageRoundingDigits()).thenReturn(0); Assert.assertEquals("Coverage 100% changed 0.0% vs master 100%", new Message(1, 1).forConsole()); Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0, 0).forConsole()); Assert.assertEquals("Coverage 50% changed +50.0% vs master 0%", new Message(0.5f, 0).forConsole()); @@ -33,8 +46,22 @@ public void buildNiceForConsole() { Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0.000007f, 0.000005f).forConsole()); } + @Test + public void buildNiceForConsoleSonar3Rounding() { + Mockito.when(settingsRepository.getCoverageRoundingDigits()).thenReturn(3); + Assert.assertEquals("Coverage 100% changed 0.0% vs master 100%", new Message(1, 1).forConsole()); + Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0, 0).forConsole()); + Assert.assertEquals("Coverage 50% changed +50.0% vs master 0%", new Message(0.5f, 0).forConsole()); + Assert.assertEquals("Coverage 0% changed -50.0% vs master 50%", new Message(0, 0.5f).forConsole()); + Assert.assertEquals("Coverage 70% changed +20.0% vs master 50%", new Message(0.7f, 0.5f).forConsole()); + Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0.0007f, 0.0005f).forConsole()); + Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0.000007f, 0.000005f).forConsole()); + Assert.assertEquals("Coverage 0% changed 0.0% vs master 0%", new Message(0.0041317f, 0.005f).forConsole()); + } + @Test public void buildNiceForIcon() { + Mockito.when(settingsRepository.getCoverageRoundingDigits()).thenReturn(0); Assert.assertEquals("100% (0.0%) vs master 100%", new Message(1, 1).forIcon()); Assert.assertEquals("0% (0.0%) vs master 0%", new Message(0, 0).forIcon()); Assert.assertEquals("50% (+50.0%) vs master 0%", new Message(0.5f, 0).forIcon()); From c62eb0159ec13b9ffdbbcd0e95881be081d2fb7e Mon Sep 17 00:00:00 2001 From: Andrew Knowles Date: Mon, 2 Oct 2017 10:41:35 -0400 Subject: [PATCH 3/5] Allow for aggregate parsers. This allows for state when processing multi-modules. By default, this plugin just averages all of the coverage results. With aggregation, it will recalculate coverage based off of total missed/coverage aggregates. --- .../githubprcoveragestatus/CloverParser.java | 9 +++++ .../CoberturaParser.java | 9 +++++ .../githubprcoveragestatus/Configuration.java | 11 ++++++ .../CoverageReportParser.java | 3 ++ .../GetCoverageCallable.java | 4 ++ .../githubprcoveragestatus/JacocoParser.java | 38 +++++++++++++++---- .../SettingsRepository.java | 2 + .../SimpleCovParser.java | 9 +++++ .../Configuration/global.groovy | 4 ++ .../help-useAggregatesForCoverage.html | 3 ++ .../JacocoParserTest.java | 28 ++++++++++++++ .../JacocoParserTest/jacoco-multiple.xml | 5 +++ 12 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-useAggregatesForCoverage.html create mode 100644 src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CloverParser.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CloverParser.java index b5f2040..f07df78 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CloverParser.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CloverParser.java @@ -36,6 +36,15 @@ class CloverParser implements CoverageReportParser { private static final String TOTAL_STATEMENTS_XPATH = "/coverage/project/metrics/@statements"; private static final String COVER_STATEMENTS_XPATH = "/coverage/project/metrics/@coveredstatements"; + @Override + public boolean canAggregate() { + return false; + } + @Override + public float getAggregate() { + throw new UnsupportedOperationException(); + } + private int getByXpath(final String filePath, final String content, final String xpath) { try { return Integer.parseInt(XmlUtils.findInXml(content, xpath)); diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CoberturaParser.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CoberturaParser.java index 1960013..72d777e 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CoberturaParser.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CoberturaParser.java @@ -29,6 +29,15 @@ */ class CoberturaParser implements CoverageReportParser { + @Override + public boolean canAggregate() { + return false; + } + @Override + public float getAggregate() { + throw new UnsupportedOperationException(); + } + private static String findFirst(String string, String pattern) { String result = findFirstOrNull(string, pattern); if (result != null) { diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java index 35f4a61..b3954ea 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/Configuration.java @@ -89,6 +89,10 @@ public static void setMasterCoverage(final String repo, final float coverage) { DESCRIPTOR.set(repo, coverage); } + public static Boolean isUseAggregatesForCoverage() { + return DESCRIPTOR.isUseAggregatesForCoverage(); + } + @Override public ConfigurationDescriptor getDescriptor() { return DESCRIPTOR; @@ -108,6 +112,7 @@ public static final class ConfigurationDescriptor extends Descriptor getFloats(File ws, String path, CoverageReportParser String[] files = ds.getIncludedFiles(); List cov = new ArrayList(); for (String file : files) cov.add(parser.get(new File(ds.getBasedir(), file).getAbsolutePath())); + if (parser.canAggregate()) { + cov.clear(); + cov.add(parser.getAggregate()); + } return cov; } diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParser.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParser.java index ba3ab60..2b0840e 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParser.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParser.java @@ -31,14 +31,14 @@ */ class JacocoParser implements CoverageReportParser { - private static final String MISSED_INSTRUCTION_XPATH = "/report/counter[@type='INSTRUCTION']/@missed"; - private static final String COVERAGE_INSTRUCTION_XPATH = "/report/counter[@type='INSTRUCTION']/@covered"; - private static final String MISSED_LINE_XPATH = "/report/counter[@type='LINE']/@missed"; - private static final String COVERAGE_LINE_XPATH = "/report/counter[@type='LINE']/@covered"; - private static final String MISSED_BRANCH_XPATH = "/report/counter[@type='BRANCH']/@missed"; - private static final String COVERAGE_BRANCH_XPATH = "/report/counter[@type='BRANCH']/@covered"; + public static final String MISSED_INSTRUCTION_XPATH = "/report/counter[@type='INSTRUCTION']/@missed"; + public static final String COVERAGE_INSTRUCTION_XPATH = "/report/counter[@type='INSTRUCTION']/@covered"; + public static final String MISSED_LINE_XPATH = "/report/counter[@type='LINE']/@missed"; + public static final String COVERAGE_LINE_XPATH = "/report/counter[@type='LINE']/@covered"; + public static final String MISSED_BRANCH_XPATH = "/report/counter[@type='BRANCH']/@missed"; + public static final String COVERAGE_BRANCH_XPATH = "/report/counter[@type='BRANCH']/@covered"; - private float getByXpath(final String filePath, final String content, final String xpath) { + public static float getByXpath(final String filePath, final String content, final String xpath) { try { return Float.parseFloat(XmlUtils.findInXml(content, xpath)); } catch (NumberFormatException e) { @@ -49,7 +49,16 @@ private float getByXpath(final String filePath, final String content, final Stri "from:\n" + content); } } + + @Override + public boolean canAggregate() { + final SettingsRepository settingsRepository = ServiceRegistry.getSettingsRepository(); + return settingsRepository.isUseAggregatesForCoverage(); + } + private volatile float totalCovered = 0.0f; + private volatile float totalMissed = 0.0f; + @Override public float get(final String jacocoFilePath) { final String content; @@ -76,6 +85,10 @@ public float get(final String jacocoFilePath) { final float countMissed = getByXpath(jacocoFilePath, content, missedMetric); final float countCovered = getByXpath(jacocoFilePath, content, coverageMetric); + + totalCovered += countCovered; + totalMissed += countMissed; + final float count = countCovered + countMissed; if (count == 0) { return 0; @@ -83,5 +96,16 @@ public float get(final String jacocoFilePath) { return countCovered / (count); } } + + @Override + public float getAggregate () { + final float count = totalCovered + totalMissed; + if (count == 0) { + return 0; + } else { + return totalCovered / (count); + } + + } } diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java index 3734587..a0426d4 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SettingsRepository.java @@ -17,6 +17,8 @@ interface SettingsRepository { boolean isUseSonarForMasterCoverage(); boolean isDisableSimpleCov(); + + boolean isUseAggregatesForCoverage(); String getSonarUrl(); diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SimpleCovParser.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SimpleCovParser.java index 4ebb1c1..7925307 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SimpleCovParser.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SimpleCovParser.java @@ -13,6 +13,15 @@ public class SimpleCovParser implements CoverageReportParser { private static final String METRIC_PATH = "$.metrics.covered_percent"; + @Override + public boolean canAggregate() { + return false; + } + @Override + public float getAggregate() { + throw new UnsupportedOperationException(); + } + @Override public float get(String simpleCovFilePath) { final String content; diff --git a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy index 53565cf..d40a502 100644 --- a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy +++ b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/global.groovy @@ -56,6 +56,10 @@ f.section(title: descriptor.displayName) { f.textbox() } + f.entry(field: "useAggregatesForCoverage", title: _("Use aggregate calculations for coverage")) { + f.checkbox() + } + f.entry(field: "disableSimpleCov", title: _("Disable SimpleCov coverage parser")) { f.checkbox() } diff --git a/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-useAggregatesForCoverage.html b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-useAggregatesForCoverage.html new file mode 100644 index 0000000..dd89c1e --- /dev/null +++ b/src/main/resources/com/github/terma/jenkins/githubprcoveragestatus/Configuration/help-useAggregatesForCoverage.html @@ -0,0 +1,3 @@ +
+ When enabled compatible plugins will calculate coverage based on aggregate covered/missed amounts, not as an average of all sub-modules. +
\ No newline at end of file diff --git a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java index 0813ccb..0fc52b7 100644 --- a/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java +++ b/src/test/java/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest.java @@ -53,6 +53,34 @@ public void extractCoverageFromJacocoReportInstruction() throws IOException { Assert.assertEquals(0.22, new JacocoParser().get(filePath), 0.1); } + @Test + public void extractCoverageFromJacocoReportAggregate() throws IOException { + String filePath1 = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + String filePath2 = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml").getFile(); + + Mockito.when(settingsRepository.isUseAggregatesForCoverage()).thenReturn(true); + JacocoParser parser = new JacocoParser(); + parser.get(filePath1); + parser.get(filePath2); + Assert.assertEquals(0.22, parser.getAggregate(), 0.1); + } + + @Test + public void extractCoverageFromJacocoReportNoAggregate() throws IOException { + String filePath1 = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco.xml").getFile(); + String filePath2 = JacocoParserTest.class.getResource( + "/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml").getFile(); + + Mockito.when(settingsRepository.isUseAggregatesForCoverage()).thenReturn(false); + JacocoParser parser = new JacocoParser(); + Float a = parser.get(filePath1); + Float b = parser.get(filePath2); + Assert.assertEquals(0.61, (a+b)/2, 0.1); + } + @Test public void extractCoverageFromJacocoReportLine() throws IOException { String filePath = JacocoParserTest.class.getResource( diff --git a/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml new file mode 100644 index 0000000..cd651fc --- /dev/null +++ b/src/test/resources/com/github/terma/jenkins/githubprcoveragestatus/JacocoParserTest/jacoco-multiple.xml @@ -0,0 +1,5 @@ + + + + \ No newline at end of file From e3be8cfe3bf0255e7e5c18064338f0f40b69a733 Mon Sep 17 00:00:00 2001 From: Andrew Knowles Date: Mon, 2 Oct 2017 12:11:01 -0400 Subject: [PATCH 4/5] Fix double-count when no files found (only for aggregates). --- .../jenkins/githubprcoveragestatus/CompareCoverageAction.java | 2 ++ .../jenkins/githubprcoveragestatus/GetCoverageCallable.java | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java index dfdfe9b..aa0dade 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/CompareCoverageAction.java @@ -113,6 +113,8 @@ public void perform( buildLog.println(BUILD_LOG_PREFIX + "using round-up to digits: " + settingsRepository.getCoverageRoundingDigits()); } + buildLog.println(BUILD_LOG_PREFIX + "using aggregates for coverage: " + settingsRepository.isUseAggregatesForCoverage()); + buildLog.println(BUILD_LOG_PREFIX + "getting master coverage..."); MasterCoverageRepository masterCoverageRepository = ServiceRegistry.getMasterCoverageRepository(buildLog, sonarLogin, sonarPassword); final GHRepository gitHubRepository = ServiceRegistry.getPullRequestRepository().getGitHubRepository(gitUrl); diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/GetCoverageCallable.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/GetCoverageCallable.java index 9dcf332..9609d3e 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/GetCoverageCallable.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/GetCoverageCallable.java @@ -44,7 +44,7 @@ private static List getFloats(File ws, String path, CoverageReportParser String[] files = ds.getIncludedFiles(); List cov = new ArrayList(); for (String file : files) cov.add(parser.get(new File(ds.getBasedir(), file).getAbsolutePath())); - if (parser.canAggregate()) { + if (cov.size() > 0 && parser.canAggregate()) { cov.clear(); cov.add(parser.getAggregate()); } From ee2ddb238ed3ef768c410082455d69a6764ad810 Mon Sep 17 00:00:00 2001 From: Andrew Knowles Date: Thu, 5 Oct 2017 10:17:56 -0400 Subject: [PATCH 5/5] Add exact match resolution for multiple repo matches. --- .../SonarMasterCoverageRepository.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepository.java b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepository.java index 37290bc..cb0acb9 100644 --- a/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepository.java +++ b/src/main/java/com/github/terma/jenkins/githubprcoveragestatus/SonarMasterCoverageRepository.java @@ -94,7 +94,13 @@ private SonarProject getSonarProject(final String repoName) throws SonarProjectR log("Found project for repo name %s - %s", repoName, sonarProjects.get(0)); return sonarProjects.get(0); } else { - log("Found multiple projects for repo name %s - found %s - returning first result", repoName, sonarProjects); + log("Found multiple projects for repo name %s - found %s - looking for repo/key exact match", repoName, sonarProjects); + for (SonarProject project: sonarProjects) { + if (project.getKey().equalsIgnoreCase(repoName)) { + return project; + } + } + log("No repo/key exact match, returning first result"); return sonarProjects.get(0); } } catch (final Exception e) {