From 3078c44cf8dd58238ac05ee39b77dff660a20560 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Fri, 8 Sep 2017 12:27:26 +0200 Subject: [PATCH] SONAR-8855 Make it obvious that generic coverage report expect an XML file --- .../sonar/api/batch/fs/InputComponent.java | 1 + .../GenericCoverageReportParser.java | 11 ++-- .../GenericTestExecutionReportParser.java | 9 ++- .../GenericCoverageReportParserTest.java | 60 +++++++++++-------- .../GenericTestExecutionReportParserTest.java | 29 +++++---- 5 files changed, 68 insertions(+), 42 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputComponent.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputComponent.java index 34b0d9edd12..7e622fb47fd 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputComponent.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/fs/InputComponent.java @@ -32,6 +32,7 @@ public interface InputComponent { /** * Component key shared by all part of SonarQube (scanner, server, WS...). * It doesn't include the branch. + * Warning. Do not use in SonarLint. */ String key(); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParser.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParser.java index 0be3ce89536..0890c8544b3 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParser.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParser.java @@ -20,6 +20,7 @@ package org.sonar.scanner.genericcoverage; import com.google.common.base.Preconditions; +import java.io.File; import java.io.FileInputStream; import java.io.InputStream; import java.util.ArrayList; @@ -32,6 +33,7 @@ import org.codehaus.staxmate.in.SMInputCursor; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.coverage.NewCoverage; +import org.sonar.api.utils.MessageException; import org.sonar.api.utils.StaxParser; public class GenericCoverageReportParser { @@ -47,15 +49,16 @@ public class GenericCoverageReportParser { private final List firstUnknownFiles = new ArrayList<>(); private final Set matchedFileKeys = new HashSet<>(); - public void parse(java.io.File reportFile, SensorContext context) { + public void parse(File reportFile, SensorContext context) { try (InputStream inputStream = new FileInputStream(reportFile)) { parse(inputStream, context); } catch (Exception e) { - throw new IllegalStateException("Error during parsing of coverage report " + reportFile, e); + throw MessageException.of("Error during parsing of the generic coverage report '" + reportFile + "'. Look at SonarQube documentation to know the expected XML format.", + e); } } - void parse(InputStream inputStream, SensorContext context) throws XMLStreamException { + private void parse(InputStream inputStream, SensorContext context) throws XMLStreamException { new StaxParser(rootCursor -> { rootCursor.advance(); parseRootNode(rootCursor, context); @@ -88,7 +91,7 @@ public class GenericCoverageReportParser { "Line %s of report refers to a file with an unknown language: %s", fileCursor.getCursorLocation().getLineNumber(), filePath); - matchedFileKeys.add(inputFile.absolutePath()); + matchedFileKeys.add(inputFile.key()); NewCoverage newCoverage = context.newCoverage().onFile(inputFile); SMInputCursor lineToCoverCursor = fileCursor.childElementCursor(); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParser.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParser.java index 40b908d061a..68b883627b5 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParser.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParser.java @@ -20,6 +20,7 @@ package org.sonar.scanner.genericcoverage; import com.google.common.base.Preconditions; +import java.io.File; import java.io.FileInputStream; import java.io.InputStream; import java.util.ArrayList; @@ -34,6 +35,7 @@ import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.test.MutableTestCase; import org.sonar.api.test.MutableTestPlan; import org.sonar.api.test.TestCase; +import org.sonar.api.utils.MessageException; import org.sonar.api.utils.StaxParser; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -71,15 +73,16 @@ public class GenericTestExecutionReportParser { this.testPlanBuilder = testPlanBuilder; } - public void parse(java.io.File reportFile, SensorContext context) { + public void parse(File reportFile, SensorContext context) { try (InputStream inputStream = new FileInputStream(reportFile)) { parse(inputStream, context); } catch (Exception e) { - throw new IllegalStateException("Error during parsing of test execution report " + reportFile, e); + throw MessageException.of( + "Error during parsing of generic test execution report '" + reportFile + "'. Look at the SonarQube documentation to know the expected XML format.", e); } } - public void parse(InputStream inputStream, SensorContext context) throws XMLStreamException { + private void parse(InputStream inputStream, SensorContext context) throws XMLStreamException { new StaxParser(rootCursor -> { rootCursor.advance(); parseRootNode(rootCursor, context); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParserTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParserTest.java index 24d62f2e876..058d23ecb8a 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParserTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericCoverageReportParserTest.java @@ -19,18 +19,26 @@ */ package org.sonar.scanner.genericcoverage; -import java.io.ByteArrayInputStream; import java.io.File; +import java.nio.charset.StandardCharsets; +import org.apache.commons.io.FileUtils; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.utils.MessageException; import static org.assertj.core.api.Assertions.assertThat; public class GenericCoverageReportParserTest { + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + private DefaultInputFile fileWithBranches; private DefaultInputFile fileWithoutBranch; private DefaultInputFile emptyFile; @@ -48,7 +56,7 @@ public class GenericCoverageReportParserTest { public void empty_file() throws Exception { addFileToFs(emptyFile); GenericCoverageReportParser parser = new GenericCoverageReportParser(); - parser.parse(this.getClass().getResourceAsStream("coverage.xml"), context); + parser.parse(new File(this.getClass().getResource("coverage.xml").toURI()), context); assertThat(parser.numberOfMatchedFiles()).isEqualTo(1); assertThat(parser.numberOfUnknownFiles()).isEqualTo(3); assertThat(parser.firstUnknownFiles()).hasSize(3); @@ -58,7 +66,7 @@ public class GenericCoverageReportParserTest { public void file_without_branch() throws Exception { addFileToFs(fileWithoutBranch); GenericCoverageReportParser parser = new GenericCoverageReportParser(); - parser.parse(this.getClass().getResourceAsStream("coverage.xml"), context); + parser.parse(new File(this.getClass().getResource("coverage.xml").toURI()), context); assertThat(parser.numberOfMatchedFiles()).isEqualTo(1); assertThat(context.lineHits(fileWithoutBranch.key(), 2)).isEqualTo(0); @@ -72,7 +80,7 @@ public class GenericCoverageReportParserTest { public void file_with_branches() throws Exception { addFileToFs(fileWithBranches); GenericCoverageReportParser parser = new GenericCoverageReportParser(); - parser.parse(this.getClass().getResourceAsStream("coverage.xml"), context); + parser.parse(new File(this.getClass().getResource("coverage.xml").toURI()), context); assertThat(parser.numberOfMatchedFiles()).isEqualTo(1); assertThat(context.lineHits(fileWithBranches.key(), 3)).isEqualTo(1); @@ -85,60 +93,60 @@ public class GenericCoverageReportParserTest { assertThat(context.coveredConditions(fileWithBranches.key(), 4)).isEqualTo(0); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_invalid_root_node_name() throws Exception { - new GenericCoverageReportParser().parse(new ByteArrayInputStream("".getBytes()), context); + parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_invalid_report_version() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_no_report_version() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_invalid_file_node_name() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unitTest_invalid_file_node_name() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_missing_path_attribute() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unitTest_missing_path_attribute() throws Exception { parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_invalid_lineToCover_node_name() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_missing_lineNumber_in_lineToCover() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_lineNumber_in_lineToCover_should_be_a_number() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_lineNumber_in_lineToCover_should_be_positive() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); @@ -152,54 +160,54 @@ public class GenericCoverageReportParserTest { + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_missing_covered_in_lineToCover() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_covered_in_lineToCover_should_be_a_boolean() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_branchesToCover_in_lineToCover_should_be_a_number() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_branchesToCover_in_lineToCover_should_not_be_negative() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_coveredBranches_in_lineToCover_should_be_a_number() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_coveredBranches_in_lineToCover_should_not_be_negative() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void coverage_coveredBranches_should_not_be_greater_than_branchesToCover() throws Exception { addFileToFs(setupFile("file1")); parseCoverageReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void testUnknownFile() throws Exception { parseCoverageReportFile("xxx.xml"); } @@ -209,7 +217,9 @@ public class GenericCoverageReportParserTest { } private void parseCoverageReport(String string) throws Exception { - new GenericCoverageReportParser().parse(new ByteArrayInputStream(string.getBytes()), context); + File report = temp.newFile(); + FileUtils.write(report, string, StandardCharsets.UTF_8); + new GenericCoverageReportParser().parse(report, context); } private void parseCoverageReportFile(String reportLocation) throws Exception { diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParserTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParserTest.java index 2089cc237d9..3fee0fae9da 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParserTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/genericcoverage/GenericTestExecutionReportParserTest.java @@ -19,16 +19,20 @@ */ package org.sonar.scanner.genericcoverage; -import java.io.ByteArrayInputStream; import java.io.File; +import java.nio.charset.StandardCharsets; +import org.apache.commons.io.FileUtils; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.TestInputFileBuilder; import org.sonar.api.batch.sensor.internal.SensorContextTester; import org.sonar.api.test.MutableTestCase; import org.sonar.api.test.MutableTestPlan; +import org.sonar.api.utils.MessageException; import org.sonar.scanner.deprecated.test.TestPlanBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -42,6 +46,9 @@ import static org.mockito.Mockito.when; public class GenericTestExecutionReportParserTest { + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + private TestPlanBuilder testPlanBuilder; private DefaultInputFile fileWithBranches; private DefaultInputFile emptyFile; @@ -81,45 +88,45 @@ public class GenericTestExecutionReportParserTest { verify(testPlan).addTestCase("test3"); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_invalid_root_node_name() throws Exception { parseUnitTestReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_invalid_report_version() throws Exception { parseUnitTestReport(""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_duration_in_testCase_should_be_a_number() throws Exception { addFileToFs(setupFile("file1")); parseUnitTestReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_failure_should_have_a_message() throws Exception { addFileToFs(setupFile("file1")); parseUnitTestReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_error_should_have_a_message() throws Exception { addFileToFs(setupFile("file1")); parseUnitTestReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_skipped_should_have_a_message() throws Exception { addFileToFs(setupFile("file1")); parseUnitTestReport("" + ""); } - @Test(expected = IllegalStateException.class) + @Test(expected = MessageException.class) public void unittest_duration_in_testCase_should_not_be_negative() throws Exception { addFileToFs(setupFile("file1")); parseUnitTestReport("" @@ -132,13 +139,15 @@ public class GenericTestExecutionReportParserTest { private GenericTestExecutionReportParser parseUnitTestReport(String string) throws Exception { GenericTestExecutionReportParser parser = new GenericTestExecutionReportParser(testPlanBuilder); - parser.parse(new ByteArrayInputStream(string.getBytes()), context); + File report = temp.newFile(); + FileUtils.write(report, string, StandardCharsets.UTF_8); + parser.parse(report, context); return parser; } private GenericTestExecutionReportParser parseReportFile(String reportLocation) throws Exception { GenericTestExecutionReportParser parser = new GenericTestExecutionReportParser(testPlanBuilder); - parser.parse(this.getClass().getResourceAsStream(reportLocation), context); + parser.parse(new File(this.getClass().getResource(reportLocation).toURI()), context); return parser; } -- 2.39.5