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

coverage: Allow specifying coverage flags via a yaml file #1954

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkgs/coverage/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.12.0-wip

- Introduced support for specifying coverage flags through a YAML file.

## 1.11.1

- Update `package:vm_service` constraints to '>=12.0.0 <16.0.0'.
Expand Down
28 changes: 19 additions & 9 deletions pkgs/coverage/bin/collect_coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import 'dart:io';

import 'package:args/args.dart';
import 'package:coverage/src/collect.dart';
import 'package:coverage/src/coverage_options.dart';
import 'package:logging/logging.dart';
import 'package:path/path.dart' as p;
import 'package:stack_trace/stack_trace.dart';

Future<void> main(List<String> arguments) async {
Expand All @@ -17,7 +19,8 @@ Future<void> main(List<String> arguments) async {
print('${rec.level.name}: ${rec.time}: ${rec.message}');
});

final options = _parseArgs(arguments);
final defaultOptions = CoverageOptionsProvider().coverageOptions;
final options = _parseArgs(arguments, defaultOptions);
await Chain.capture(() async {
final coverage = await collect(options.serviceUri, options.resume,
options.waitPaused, options.includeDart, options.scopedOutput,
Expand Down Expand Up @@ -58,7 +61,7 @@ class Options {
final Set<String> scopedOutput;
}

Options _parseArgs(List<String> arguments) {
Options _parseArgs(List<String> arguments, CoverageOptions defaultOptions) {
final parser = ArgParser()
..addOption('host',
abbr: 'H',
Expand All @@ -69,11 +72,11 @@ Options _parseArgs(List<String> arguments) {
help: 'remote VM port. DEPRECATED: use --uri',
defaultsTo: '8181')
..addOption('uri', abbr: 'u', help: 'VM observatory service URI')
..addOption('out',
abbr: 'o', defaultsTo: 'stdout', help: 'output: may be file or stdout')
..addOption('out', abbr: 'o', help: 'output: may be file or stdout')
..addOption('connect-timeout',
abbr: 't', help: 'connect timeout in seconds')
..addMultiOption('scope-output',
defaultsTo: defaultOptions.scopeOutput,
help: 'restrict coverage results so that only scripts that start with '
'the provided package path are considered')
..addFlag('wait-paused',
Expand All @@ -85,10 +88,12 @@ Options _parseArgs(List<String> arguments) {
..addFlag('include-dart',
abbr: 'd', defaultsTo: false, help: 'include "dart:" libraries')
..addFlag('function-coverage',
abbr: 'f', defaultsTo: false, help: 'Collect function coverage info')
abbr: 'f',
defaultsTo: defaultOptions.functionCoverage,
help: 'Collect function coverage info')
..addFlag('branch-coverage',
abbr: 'b',
defaultsTo: false,
defaultsTo: defaultOptions.branchCoverage,
help: 'Collect branch coverage info (Dart VM must also be run with '
'--branch-coverage for this to work)')
..addFlag('help', abbr: 'h', negatable: false, help: 'show this help');
Expand Down Expand Up @@ -126,12 +131,17 @@ Options _parseArgs(List<String> arguments) {

final scopedOutput = args['scope-output'] as List<String>;
IOSink out;
if (args['out'] == 'stdout') {
final outPath = args['out'] as String?;
if (outPath == 'stdout' ||
(outPath == null && defaultOptions.outputDirectory == null)) {
out = stdout;
} else {
final outfile = File(args['out'] as String)..createSync(recursive: true);
out = outfile.openWrite();
final outFilePath = p.normalize(outPath ??
p.absolute(defaultOptions.outputDirectory!, 'coverage.json'));
final outFile = File(outFilePath)..createSync(recursive: true);
out = outFile.openWrite();
}

final timeout = (args['connect-timeout'] == null)
? null
: Duration(seconds: int.parse(args['connect-timeout'] as String));
Expand Down
59 changes: 42 additions & 17 deletions pkgs/coverage/bin/format_coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:io';

import 'package:args/args.dart';
import 'package:coverage/coverage.dart';
import 'package:coverage/src/coverage_options.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;

Expand Down Expand Up @@ -51,7 +52,8 @@ class Environment {
}

Future<void> main(List<String> arguments) async {
final env = parseArgs(arguments);
final defaultOptions = CoverageOptionsProvider().coverageOptions;
final env = parseArgs(arguments, defaultOptions);

final files = filesToProcess(env.input);
if (env.verbose) {
Expand Down Expand Up @@ -129,21 +131,38 @@ Future<void> main(List<String> arguments) async {

/// Checks the validity of the provided arguments. Does not initialize actual
/// processing.
Environment parseArgs(List<String> arguments) {
Environment parseArgs(List<String> arguments, CoverageOptions defaultOptions) {
final parser = ArgParser();

parser
..addOption('sdk-root', abbr: 's', help: 'path to the SDK root')
..addOption('packages', help: '[DEPRECATED] path to the package spec file')
..addOption(
'sdk-root',
abbr: 's',
help: 'path to the SDK root',
)
..addOption(
'packages',
help: '[DEPRECATED] path to the package spec file',
)
..addOption('package',
help: 'root directory of the package', defaultsTo: '.')
..addOption('in', abbr: 'i', help: 'input(s): may be file or directory')
..addOption('out',
abbr: 'o', defaultsTo: 'stdout', help: 'output: may be file or stdout')
..addMultiOption('report-on',
help: 'which directories or files to report coverage on')
..addOption('workers',
abbr: 'j', defaultsTo: '1', help: 'number of workers')
help: 'root directory of the package',
defaultsTo: defaultOptions.packageDirectory)
..addOption(
'in',
abbr: 'i',
help: 'input(s): may be file or directory',
)
..addOption('out', abbr: 'o', help: 'output: may be file or stdout')
..addMultiOption(
'report-on',
help: 'which directories or files to report coverage on',
)
..addOption(
'workers',
abbr: 'j',
defaultsTo: '1',
help: 'number of workers',
)
..addOption('bazel-workspace',
defaultsTo: '', help: 'Bazel workspace directory')
..addOption('base-directory',
Expand Down Expand Up @@ -220,19 +239,25 @@ Environment parseArgs(List<String> arguments) {
fail('Package spec "${args["package"]}" not found, or not a directory.');
}

if (args['in'] == null) fail('No input files given.');
final input = p.absolute(p.normalize(args['in'] as String));
if (args['in'] == null && defaultOptions.outputDirectory == null) {
fail('No input files given.');
}
final input = p.normalize((args['in'] as String?) ??
p.absolute(defaultOptions.outputDirectory!, 'coverage.json'));
if (!FileSystemEntity.isDirectorySync(input) &&
!FileSystemEntity.isFileSync(input)) {
fail('Provided input "${args["in"]}" is neither a directory nor a file.');
}

IOSink output;
if (args['out'] == 'stdout') {
final outPath = args['out'] as String?;
if (outPath == 'stdout' ||
(outPath == null && defaultOptions.outputDirectory == null)) {
output = stdout;
} else {
final outpath = p.absolute(p.normalize(args['out'] as String));
final outfile = File(outpath)..createSync(recursive: true);
final outFilePath = p.normalize(
outPath ?? p.absolute(defaultOptions.outputDirectory!, 'lcov.info'));
final outfile = File(outFilePath)..createSync(recursive: true);
output = outfile.openWrite();
}

Expand Down
23 changes: 15 additions & 8 deletions pkgs/coverage/bin/test_with_coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';
import 'dart:io';

import 'package:args/args.dart';
import 'package:coverage/src/coverage_options.dart';
import 'package:coverage/src/util.dart'
show StandardOutExtension, extractVMServiceUri;
import 'package:package_config/package_config.dart';
Expand Down Expand Up @@ -50,37 +51,41 @@ void _watchExitSignal(ProcessSignal signal) {
});
}

ArgParser _createArgParser() => ArgParser()
ArgParser _createArgParser(CoverageOptions defaultOptions) => ArgParser()
..addOption(
'package',
help: 'Root directory of the package to test.',
defaultsTo: '.',
defaultsTo: defaultOptions.packageDirectory,
)
..addOption(
'package-name',
help: 'Name of the package to test. '
'Deduced from --package if not provided.',
defaultsTo: defaultOptions.packageName,
)
..addOption('port', help: 'VM service port.', defaultsTo: '8181')
..addOption(
'out',
defaultsTo: defaultOptions.outputDirectory,
abbr: 'o',
help: 'Output directory. Defaults to <package-dir>/coverage.',
)
..addOption('test', help: 'Test script to run.', defaultsTo: 'test')
..addOption('test',
help: 'Test script to run.', defaultsTo: defaultOptions.testScript)
..addFlag(
'function-coverage',
abbr: 'f',
defaultsTo: false,
defaultsTo: defaultOptions.functionCoverage,
help: 'Collect function coverage info.',
)
..addFlag(
'branch-coverage',
abbr: 'b',
defaultsTo: false,
defaultsTo: defaultOptions.branchCoverage,
help: 'Collect branch coverage info.',
)
..addMultiOption('scope-output',
defaultsTo: defaultOptions.scopeOutput,
help: 'restrict coverage results so that only scripts that start with '
'the provided package path are considered. Defaults to the name of '
'the package under test.')
Expand Down Expand Up @@ -110,8 +115,9 @@ class Flags {
final List<String> rest;
}

Future<Flags> _parseArgs(List<String> arguments) async {
final parser = _createArgParser();
Future<Flags> _parseArgs(
List<String> arguments, CoverageOptions defaultOptions) async {
final parser = _createArgParser(defaultOptions);
final args = parser.parse(arguments);

void printUsage() {
Expand Down Expand Up @@ -166,7 +172,8 @@ ${parser.usage}
}

Future<void> main(List<String> arguments) async {
final flags = await _parseArgs(arguments);
final defaultOptions = CoverageOptionsProvider().coverageOptions;
final flags = await _parseArgs(arguments, defaultOptions);
final outJson = path.join(flags.outDir, 'coverage.json');
final outLcov = path.join(flags.outDir, 'lcov.info');

Expand Down
120 changes: 120 additions & 0 deletions pkgs/coverage/lib/src/coverage_options.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import 'dart:io';
import 'package:cli_config/cli_config.dart';
import 'package:path/path.dart' as path;

class CoverageOptions {
const CoverageOptions({
this.outputDirectory,
required this.scopeOutput,
required this.functionCoverage,
required this.branchCoverage,
required this.packageDirectory,
this.packageName,
required this.testScript,
});

factory CoverageOptions.fromConfig(
Config options, CoverageOptions defaultOptions, String? optionsFilePath) {
var outputDirectory = options.optionalString('output_directory') ??
defaultOptions.outputDirectory;
var packageDirectory = options.optionalString('package_directory') ??
defaultOptions.packageDirectory;

if (optionsFilePath != null) {
if (outputDirectory != null && !path.isAbsolute(outputDirectory)) {
outputDirectory = path.normalize(
path.absolute(path.dirname(optionsFilePath), outputDirectory));
}
if (!path.isAbsolute(packageDirectory)) {
packageDirectory = path.normalize(
path.absolute(path.dirname(optionsFilePath), packageDirectory));
}
}

return CoverageOptions(
Dhruv-Maradiya marked this conversation as resolved.
Show resolved Hide resolved
outputDirectory: outputDirectory,
scopeOutput: options.optionalStringList('scope_output') ??
defaultOptions.scopeOutput,
functionCoverage: options.optionalBool('function_coverage') ??
defaultOptions.functionCoverage,
branchCoverage: options.optionalBool('branch_coverage') ??
defaultOptions.branchCoverage,
packageDirectory: packageDirectory,
packageName:
options.optionalString('package_name') ?? defaultOptions.packageName,
testScript:
options.optionalString('test_script') ?? defaultOptions.testScript,
);
}

final String? outputDirectory;
final List<String> scopeOutput;
final bool functionCoverage;
final bool branchCoverage;
final String packageDirectory;
final String? packageName;
final String testScript;
}

class CoverageOptionsProvider {
CoverageOptionsProvider({
String? filePath,
}) {
final file = _getOptionsFile(filePath);
final fileContents = file?.readAsStringSync();

final isFileEmpty = fileContents?.isEmpty ?? false;

// Pass null to fileContents if the file is empty
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a question: Why do you need to pass null to fileContents if the file is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we pass empty file content to fromConfigFileContents, it throws a FormatException. Should we also handle cases where the file is empty but contains only blank spaces?

final options = Config.fromConfigFileContents(
fileContents: isFileEmpty ? null : fileContents,
fileSourceUri: file?.uri,
);

coverageOptions =
CoverageOptions.fromConfig(options, defaultOptions, optionsFilePath);
}

late final CoverageOptions coverageOptions;
late final String? optionsFilePath;
static const defaultFilePath = 'coverage_options.yaml';

File? _getOptionsFile(String? filePath) {
filePath ??= _findOptionsFilePath();

optionsFilePath = filePath != null ? path.canonicalize(filePath) : null;
Dhruv-Maradiya marked this conversation as resolved.
Show resolved Hide resolved

if (optionsFilePath == null) {
return null;
}

final file = File(optionsFilePath!);
return file.existsSync() ? file : null;
}

String? _findOptionsFilePath() {
var currentDir = Directory.current;

while (true) {
final pubSpecFilePath = path.join(currentDir.path, 'pubspec.yaml');
if (File(pubSpecFilePath).existsSync()) {
return path.join(currentDir.path, defaultFilePath);
}
final parentDir = currentDir.parent;
if (parentDir.path == currentDir.path) {
return null;
}
currentDir = parentDir;
}
}

static const defaultOptions = CoverageOptions(
outputDirectory: null,
scopeOutput: [],
functionCoverage: false,
branchCoverage: false,
packageDirectory: '.',
packageName: null,
testScript: 'test',
);
}
3 changes: 2 additions & 1 deletion pkgs/coverage/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: coverage
version: 1.11.1
version: 1.12.0-wip
description: Coverage data manipulation and formatting
repository: https://github.com/dart-lang/tools/tree/main/pkgs/coverage
issue_tracker: https://github.com/dart-lang/tools/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Acoverage
Expand All @@ -9,6 +9,7 @@ environment:

dependencies:
args: ^2.0.0
cli_config: ^0.2.0
glob: ^2.1.2
logging: ^1.0.0
meta: ^1.0.2
Expand Down
Loading
Loading