Support wildcards for coverage report paths #2742

yassin-kammoun-sonarsource · 2021-08-05T14:19:29Z

Fixes #578

vilchik-elena · 2021-08-06T10:25:23Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/DirectoryScanner.java

+import org.apache.commons.io.filefilter.TrueFileFilter;
+import org.sonar.api.utils.WildcardPattern;
+
+// Taken from https://github.com/SonarSource/sonar-python/blob/7a683a8da47ed8db51378fbc6c50a76d98b45097/sonar-python-plugin/src/main/java/org/sonar/plugins/python/DirectoryScanner.java


yeah, we should definitely move it into commons

vilchik-elena · 2021-08-06T10:28:40Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/CoverageSensor.java

+    List<File> lcovFiles = new ArrayList<>();
+    for (String reportPath : reportPaths) {
+      LOG.debug("Using pattern '{}' to resolve LCOV files", reportPath);
+      DirectoryScanner scanner = new DirectoryScanner(baseDir, WildcardPattern.create(reportPath));


how about passing reportPath directly to constructor to hide this WildcardPattern usage?

vilchik-elena · 2021-08-06T10:29:38Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/DirectoryScanner.java

+
+// Taken from https://github.com/SonarSource/sonar-python/blob/7a683a8da47ed8db51378fbc6c50a76d98b45097/sonar-python-plugin/src/main/java/org/sonar/plugins/python/DirectoryScanner.java
+
+public class DirectoryScanner {


I would call this class PatternDirectoryScanner

vilchik-elena · 2021-08-06T10:30:10Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/DirectoryScanner.java

+    this.pattern = pattern;
+  }
+
+  public List<File> getIncludedFiles() {


getMatchingFiles

vilchik-elena · 2021-08-06T10:33:44Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/CoverageSensor.java

+      if (includedFiles.isEmpty()) {
+        File file = new File(reportPath);
+        if (!file.exists()) {
+          LOG.debug("No LCOV files were found using pattern {}", reportPath);


I think it should be INFO

vilchik-elena · 2021-08-06T10:47:47Z

sonar-javascript-plugin/src/main/java/org/sonar/plugins/javascript/lcov/CoverageSensor.java

+  private static List<File> getLcovFiles(File baseDir, Set<String> reportPaths) {
+    List<File> lcovFiles = new ArrayList<>();
+    for (String reportPath : reportPaths) {
+      LOG.debug("Using pattern '{}' to resolve LCOV files", reportPath);


how about drop pattern from this message to not confuse users not using patterns?

vilchik-elena · 2021-08-06T10:50:21Z

sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/lcov/CoverageSensorTest.java

@@ -285,6 +285,28 @@ public void should_resolve_absolute_path() throws Exception {
    assertThat(context.lineHits(file2Key, 2)).isEqualTo(5);
  }

+  @Test
+  public void should_resolve_wildcard_report_paths() throws Exception {
+    settings.setProperty(JavaScriptPlugin.LCOV_REPORT_PATHS, "**/reports/*.lcov");


it's not clear which reports should match

vilchik-elena · 2021-08-09T10:09:42Z

sonar-javascript-plugin/src/test/resources/coverage/wildcard/nested/report.lcov

@@ -0,0 +1,12 @@
+TN:
+SF:tests/file1.js


let's simplify these 2 reports as much as possible (I think DA:2,1 is enough)

vilchik-elena · 2021-08-09T10:10:11Z

sonar-javascript-plugin/src/test/resources/coverage/wildcard/report.lcov

@@ -0,0 +1,29 @@
+TN:
+SF:deep/nested/dir/js/file1.js


why having this long path?

vilchik-elena · 2021-08-09T10:10:36Z

sonar-javascript-plugin/src/test/resources/coverage/wildcard/report.lcov

+BRH:0
+end_of_record
+TN:
+SF:file1.js


why having another file in the report to prove that it's imported?

sonarsource-next · 2021-08-09T11:36:58Z

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

jackgeek · 2021-08-27T11:13:48Z

It appears that this PR has broken support for using .. in paths.
Our AzureDevOps pipeline configuration looks like this:

          - task: SonarCloudPrepare@1
            displayName: 'Prepare SonarCloud analysis'
            inputs:
              ...
              extraProperties: |
                sonar.projectBaseDir=packages/
                sonar.javascript.lcov.reportPaths=../lcov.info

And this was working fine before this PR was released. However now it does not work.

Before this PR:

INFO: Sensor JavaScript/TypeScript Coverage [javascript]
INFO: Analysing [/home/vsts/work/1/s/packages/../lcov.info]

After this PR:

INFO: Sensor JavaScript/TypeScript Coverage [javascript]
INFO: No LCOV files were found using ../lcov.info

yassin-kammoun-sonarsource force-pushed the issue-578 branch from 39feb3d to fd5a001 Compare August 5, 2021 15:04

Support wildcards for coverage report paths

9bf3b2f

yassin-kammoun-sonarsource force-pushed the issue-578 branch from fd5a001 to 9bf3b2f Compare August 6, 2021 06:55

yassin-kammoun-sonarsource marked this pull request as ready for review August 6, 2021 07:33

yassin-kammoun-sonarsource requested a review from vilchik-elena August 6, 2021 07:34

vilchik-elena requested changes Aug 6, 2021

View reviewed changes

yassin-kammoun-sonarsource added 6 commits August 6, 2021 13:38

Fix log level

47e5bc3

Fix message

760dfa1

Improve unit test

62be868

Use org.sonarsource.analyzer.commons.FileProvider

8e98881

Merge branch 'master' of github.com:SonarSource/SonarJS into issue-578

987a11c

Fix message

67eb1a1

vilchik-elena requested changes Aug 9, 2021

View reviewed changes

yassin-kammoun-sonarsource added 2 commits August 9, 2021 12:57

Fix after review

6268871

Add non-matching report

6f98691

vilchik-elena approved these changes Aug 9, 2021

View reviewed changes

yassin-kammoun-sonarsource merged commit 1dd6ef1 into master Aug 9, 2021

yassin-kammoun-sonarsource deleted the issue-578 branch August 9, 2021 12:03

yassin-kammoun-sonarsource mentioned this pull request Sep 24, 2021

Fix resolving of relative paths of coverage reports #2804

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support wildcards for coverage report paths #2742

Support wildcards for coverage report paths #2742

yassin-kammoun-sonarsource commented Aug 5, 2021

vilchik-elena Aug 6, 2021

vilchik-elena Aug 6, 2021

yassin-kammoun-sonarsource Aug 6, 2021

vilchik-elena Aug 6, 2021

yassin-kammoun-sonarsource Aug 6, 2021

vilchik-elena Aug 6, 2021

yassin-kammoun-sonarsource Aug 6, 2021

vilchik-elena Aug 6, 2021

vilchik-elena Aug 6, 2021

vilchik-elena Aug 6, 2021

vilchik-elena Aug 9, 2021

vilchik-elena Aug 9, 2021

vilchik-elena Aug 9, 2021

sonarsource-next bot commented Aug 9, 2021

jackgeek commented Aug 27, 2021 •

edited

Loading


		// Taken from https://github.com/SonarSource/sonar-python/blob/7a683a8da47ed8db51378fbc6c50a76d98b45097/sonar-python-plugin/src/main/java/org/sonar/plugins/python/DirectoryScanner.java

		public class DirectoryScanner {

		@@ -0,0 +1,12 @@
		TN:
		SF:tests/file1.js

		@@ -0,0 +1,29 @@
		TN:
		SF:deep/nested/dir/js/file1.js

Support wildcards for coverage report paths #2742

Support wildcards for coverage report paths #2742

Conversation

yassin-kammoun-sonarsource commented Aug 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarsource-next bot commented Aug 9, 2021

jackgeek commented Aug 27, 2021 • edited Loading

jackgeek commented Aug 27, 2021 •

edited

Loading