Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support wildcards for coverage report paths #2742

Merged
merged 9 commits into from
Aug 9, 2021
12 changes: 12 additions & 0 deletions its/plugin/projects/lcov-wildcard/bar/bar.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function isArrayLike (obj) {
return Object.prototype.toString.call(obj) === '[object Array]';
}

function size (obj) {
if (obj == null) {
return 0;
}
return isArrayLike(obj) ? obj.length : Object.keys(obj).length;
}

module.exports = size;
24 changes: 24 additions & 0 deletions its/plugin/projects/lcov-wildcard/bar/bar.lcov
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
TN:
SF:./bar.js
FN:1,isArrayLike
FN:5,size
FNF:2
FNH:2
FNDA:2,isArrayLike
FNDA:2,size
DA:1,1
DA:2,2
DA:5,1
DA:6,2
DA:7,0
DA:9,2
DA:12,1
LF:7
LH:6
BRDA:6,1,0,0
BRDA:6,1,1,2
BRDA:9,2,0,1
BRDA:9,2,1,1
BRF:4
BRH:3
end_of_record
12 changes: 12 additions & 0 deletions its/plugin/projects/lcov-wildcard/baz/baz.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function isArrayLike (obj) {
return Object.prototype.toString.call(obj) === '[object Array]';
}

function size (obj) {
if (obj == null) {
return 0;
}
return isArrayLike(obj) ? obj.length : Object.keys(obj).length;
}

module.exports = size;
24 changes: 24 additions & 0 deletions its/plugin/projects/lcov-wildcard/baz/baz.lcov
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
TN:
SF:./baz.js
FN:1,isArrayLike
FN:5,size
FNF:2
FNH:2
FNDA:2,isArrayLike
FNDA:2,size
DA:1,1
DA:2,2
DA:5,1
DA:6,2
DA:7,0
DA:9,2
DA:12,1
LF:7
LH:6
BRDA:6,1,0,0
BRDA:6,1,1,2
BRDA:9,2,0,1
BRDA:9,2,1,1
BRF:4
BRH:3
end_of_record
12 changes: 12 additions & 0 deletions its/plugin/projects/lcov-wildcard/baz/qux/qux.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function isArrayLike (obj) {
return Object.prototype.toString.call(obj) === '[object Array]';
}

function size (obj) {
if (obj == null) {
return 0;
}
return isArrayLike(obj) ? obj.length : Object.keys(obj).length;
}

module.exports = size;
24 changes: 24 additions & 0 deletions its/plugin/projects/lcov-wildcard/baz/qux/qux.lcov
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
TN:
SF:./qux.js
FN:1,isArrayLike
FN:5,size
FNF:2
FNH:2
FNDA:2,isArrayLike
FNDA:2,size
DA:1,1
DA:2,2
DA:5,1
DA:6,2
DA:7,0
DA:9,2
DA:12,1
LF:7
LH:6
BRDA:6,1,0,0
BRDA:6,1,1,2
BRDA:9,2,0,1
BRDA:9,2,1,1
BRF:4
BRH:3
end_of_record
12 changes: 12 additions & 0 deletions its/plugin/projects/lcov-wildcard/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
function isArrayLike (obj) {
return Object.prototype.toString.call(obj) === '[object Array]';
}

function size (obj) {
if (obj == null) {
return 0;
}
return isArrayLike(obj) ? obj.length : Object.keys(obj).length;
}

module.exports = size;
24 changes: 24 additions & 0 deletions its/plugin/projects/lcov-wildcard/foo.lcov
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
TN:
SF:./foo.js
FN:1,isArrayLike
FN:5,size
FNF:2
FNH:2
FNDA:2,isArrayLike
FNDA:2,size
DA:1,1
DA:2,2
DA:5,1
DA:6,2
DA:7,0
DA:9,2
DA:12,1
LF:7
LH:6
BRDA:6,1,0,0
BRDA:6,1,1,2
BRDA:9,2,0,1
BRDA:9,2,1,1
BRF:4
BRH:3
end_of_record
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,23 @@ public void conditions_on_non_executable_lines() {
assertThat(getMeasureAsInt(projectKey, "conditions_to_cover")).isEqualTo(2);
assertThat(getMeasureAsInt(projectKey, "uncovered_conditions")).isEqualTo(1);
}

@Test
public void wildcard_LCOV_report_paths() {
final String projectKey = "LcovWildcardReportPaths";
SonarScanner build = Tests.createScanner()
.setProjectDir(TestUtils.projectDir("lcov-wildcard"))
.setProjectKey(projectKey)
.setProjectName(projectKey)
.setProjectVersion("1.0")
.setSourceDirs(".")
.setProperty("sonar.javascript.lcov.reportPaths", "foo.lcov,bar/*.lcov,**/qux/*.lcov");
Tests.setEmptyProfile(projectKey);
orchestrator.executeBuild(build);

assertThat(getMeasureAsInt(projectKey + ":foo.js", "uncovered_lines")).isEqualTo(1);
assertThat(getMeasureAsInt(projectKey + ":bar/bar.js", "uncovered_lines")).isEqualTo(1);
assertThat(getMeasureAsInt(projectKey + ":baz/baz.js", "uncovered_lines")).isEqualTo(5);
assertThat(getMeasureAsInt(projectKey + ":baz/qux/qux.js", "uncovered_lines")).isEqualTo(1);
}
}
5 changes: 5 additions & 0 deletions sonar-javascript-plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
</dependency>
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.8.0</version>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@
package org.sonar.plugins.javascript.lcov;

import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
import org.sonar.api.batch.fs.FilePredicate;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.batch.fs.InputFile;
Expand All @@ -36,6 +34,7 @@
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.batch.sensor.coverage.NewCoverage;
import org.sonar.api.utils.WildcardPattern;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.plugins.javascript.JavaScriptLanguage;
Expand Down Expand Up @@ -64,18 +63,33 @@ public void execute(SensorContext context) {
reports.addAll(Arrays.asList(context.config().getStringArray(JavaScriptPlugin.TS_LCOV_REPORT_PATHS)));
}

List<File> lcovFiles = reports.stream()
.map(report -> getIOFile(context.fileSystem().baseDir(), report))
.filter(Objects::nonNull)
.collect(Collectors.toList());

List<File> lcovFiles = getLcovFiles(context.fileSystem().baseDir(), reports);
if (lcovFiles.isEmpty()) {
LOG.warn("No coverage information will be saved because all LCOV files cannot be found.");
return;
}
saveCoverageFromLcovFiles(context, lcovFiles);
}

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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

DirectoryScanner scanner = new DirectoryScanner(baseDir, WildcardPattern.create(reportPath));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

List<File> includedFiles = scanner.getIncludedFiles();
if (includedFiles.isEmpty()) {
File file = new File(reportPath);
if (!file.exists()) {
LOG.debug("No LCOV files were found using pattern {}", reportPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be INFO

} else {
includedFiles.add(file);
}
}
lcovFiles.addAll(includedFiles);
}
return lcovFiles;
}

private static void saveCoverageFromLcovFiles(SensorContext context, List<File> lcovFiles) {
LOG.info("Analysing {}", lcovFiles);

Expand Down Expand Up @@ -111,22 +125,4 @@ private static void saveCoverageFromLcovFiles(SensorContext context, List<File>
LOG.warn("Found {} inconsistencies in coverage report. Re-run analyse in debug mode to see details.", inconsistenciesNumber);
}
}

/**
* Returns a java.io.File for the given path.
* If path is not absolute, returns a File with module base directory as parent path.
*/
@CheckForNull
private static File getIOFile(File baseDir, String path) {
File file = new File(path);
if (!file.isAbsolute()) {
file = new File(baseDir, path);
}
if (!file.isFile()) {
LOG.warn("No coverage information will be saved because LCOV file cannot be found.");
LOG.warn("Provided LCOV file path: {}. Seek file with path: {}", path, file.getAbsolutePath());
return null;
}
return file;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011-2021 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.plugins.javascript.lcov;

import java.io.File;
import java.util.ArrayList;
import java.util.List;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.filefilter.IOFileFilter;
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we should definitely move it into commons


public class DirectoryScanner {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would call this class PatternDirectoryScanner

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed


private final File baseDir;
private final WildcardPattern pattern;

public DirectoryScanner(File baseDir, WildcardPattern pattern) {
this.baseDir = baseDir;
this.pattern = pattern;
}

public List<File> getIncludedFiles() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getMatchingFiles

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

final String baseDirAbsolutePath = baseDir.getAbsolutePath();
IOFileFilter fileFilter = new IOFileFilter() {

@Override
public boolean accept(File dir, String name) {
return accept(new File(dir, name));
}

@Override
public boolean accept(File file) {
String path = file.getAbsolutePath();
path = path.substring(Math.min(baseDirAbsolutePath.length(), path.length()));
return pattern.match(FilenameUtils.separatorsToUnix(path));
}
};
return new ArrayList<>(FileUtils.listFiles(baseDir, fileFilter, TrueFileFilter.INSTANCE));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public void test_unresolved_path() {
.contains("Could not resolve 2 file paths in [" + moduleBaseDir.getAbsolutePath() + fileName + "]")
.contains("First unresolved path: unresolved/file1.js (Run in DEBUG mode to get full list of unresolved paths)");
assertThat(logTester.logs(LoggerLevel.DEBUG))
.isEmpty();
.contains("Using pattern 'reports/report_with_unresolved_path.lcov' to resolve LCOV files");
}

@Test
Expand Down Expand Up @@ -202,7 +202,7 @@ public void should_log_warning_when_wrong_data() throws Exception {
assertThat(context.coveredConditions("moduleKey:file1.js", 2)).isEqualTo(2);

assertThat(logTester.logs(LoggerLevel.DEBUG)).contains("Problem during processing LCOV report: can't save DA data for line 3 of coverage report file (java.lang.NumberFormatException: For input string: \"1.\").");
String stringIndexOutOfBoundLogMessage = logTester.logs(LoggerLevel.DEBUG).get(1);
String stringIndexOutOfBoundLogMessage = logTester.logs(LoggerLevel.DEBUG).get(2);
assertThat(stringIndexOutOfBoundLogMessage).startsWith("Problem during processing LCOV report: can't save DA data for line 4 of coverage report file (java.lang.StringIndexOutOfBoundsException:");
assertThat(logTester.logs(LoggerLevel.DEBUG).get(logTester.logs(LoggerLevel.DEBUG).size() - 1)).startsWith("Problem during processing LCOV report: can't save BRDA data for line 6 of coverage report file (java.lang.ArrayIndexOutOfBoundsException: ");
assertThat(logTester.logs(LoggerLevel.WARN)).contains("Found 3 inconsistencies in coverage report. Re-run analyse in debug mode to see details.");
Expand Down Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's not clear which reports should match

inputFile("deep/nested/dir/js/file1.js", Type.MAIN);
inputFile("deep/nested/dir/js/file2.js", Type.MAIN);
coverageSensor.execute(context);

String file1Key = "moduleKey:deep/nested/dir/js/file1.js";
assertThat(context.lineHits(file1Key, 0)).isNull();
assertThat(context.lineHits(file1Key, 1)).isEqualTo(2);
assertThat(context.lineHits(file1Key, 2)).isEqualTo(2);

assertThat(context.conditions(file1Key, 102)).isNull();
assertThat(context.conditions(file1Key, 2)).isEqualTo(4);
assertThat(context.coveredConditions(file1Key, 2)).isEqualTo(2);

String file2Key = "moduleKey:deep/nested/dir/js/file2.js";
assertThat(context.lineHits(file2Key, 0)).isNull();
assertThat(context.lineHits(file2Key, 1)).isEqualTo(5);
assertThat(context.lineHits(file2Key, 2)).isEqualTo(5);
}

@Test
public void should_import_coverage_for_ts() throws Exception {
DefaultInputFile inputFile = new TestInputFileBuilder("moduleKey", "src/file1.ts")
Expand Down