From 8d7bf11b50d940dc4dfb5aa545804ce826b9122a Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Thu, 1 Oct 2026 15:07:10 +0200 Subject: [PATCH 1/8] refactor(test): reduce cognitive complexity of the test runners Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/src/cli/flutter_cli.dart | 82 +- lib/src/cli/test_cli_runner.dart | 844 +++++++++++------- .../dart/commands/dart_test_command.dart | 73 +- lib/src/commands/test/test.dart | 95 +- 4 files changed, 644 insertions(+), 450 deletions(-) diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 2a922c8f9..3f245d553 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -70,50 +70,48 @@ class CoverageMetrics { /// Generate coverage metrics from a list of lcov records. factory fromLcovRecords(List records, {String? excludeFromCoverage}) { - final globs = []; + final excludedGlobs = _parseGlobs(excludeFromCoverage); + return records + .whereNot((record) => _isExcluded(record.file, excludedGlobs)) + .fold( + const CoverageMetrics(), + (metrics, record) => metrics._add(record), + ); + } - if (excludeFromCoverage != null && excludeFromCoverage.isNotEmpty) { - for (final glob in excludeFromCoverage.trim().split(' ')) { - if (glob.isNotEmpty) globs.add(Glob(glob)); - } - } + /// Parses space-separated glob patterns, ignoring empty segments. + static List _parseGlobs(String? excludeFromCoverage) => [ + for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) + if (pattern.isNotEmpty) Glob(pattern), + ]; + + static bool _isExcluded(String? file, List excludedGlobs) => + file != null && excludedGlobs.any((glob) => glob.matches(file)); + + /// Line numbers in [record] that were instrumented but never hit. + static List _uncoveredLineNumbersOf(Record record) => [ + for (final detail in record.lines?.details ?? const []) + if (detail.line case final line? when (detail.hit ?? 1) == 0) line, + ]; + + /// Returns new metrics with the counts and uncovered lines of [record]. + CoverageMetrics _add(Record record) { + final lines = record.lines; + return CoverageMetrics( + totalFound: totalFound + (lines?.found ?? 0), + totalHits: totalHits + (lines?.hit ?? 0), + uncoveredLines: _uncoveredLinesWith(record), + ); + } - return records.fold(const CoverageMetrics(), ( - current, - record, - ) { - final found = record.lines?.found ?? 0; - final hit = record.lines?.hit ?? 0; - if (globs.isNotEmpty && record.file != null) { - for (final glob in globs) { - if (glob.matches(record.file!)) return current; - } - } - - final file = record.file; - final details = record.lines?.details; - final uncoveredLines = Map>.from( - current.uncoveredLines, - ); - - if (file != null && details != null) { - for (final line in details) { - if ((line.hit ?? 1) == 0 && line.line != null) { - uncoveredLines.update( - file, - (lines) => [...lines, line.line!], - ifAbsent: () => [line.line!], - ); - } - } - } - - return CoverageMetrics( - totalFound: current.totalFound + found, - totalHits: current.totalHits + hit, - uncoveredLines: uncoveredLines, - ); - }); + Map> _uncoveredLinesWith(Record record) { + final file = record.file; + final newLines = _uncoveredLineNumbersOf(record); + return { + ...uncoveredLines, + if (file != null && newLines.isNotEmpty) + file: [...?uncoveredLines[file], ...newLines], + }; } /// Total number of lines hit (covered) across all included files. diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 2b3fba6cc..ca8191e2d 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -171,172 +171,246 @@ class TestCLIRunner { overrideTestRunner ?? (testType == TestRunType.flutter ? flutterTest : dartTest); + final coverageOptions = _CoverageOptions( + collect: collectCoverage, + collectFrom: collectCoverageFrom, + minCoverage: minCoverage, + showUncovered: showUncovered, + excludeFromCoverage: excludeFromCoverage, + reportOn: reportOn ?? const ['lib'], + checkIgnore: checkIgnore, + ); + return _runCommand( - cmd: (cwd) async { - final lcovPath = p.join(cwd, 'coverage', 'lcov.info'); - final lcovFile = File(lcovPath); - - if (collectCoverage && lcovFile.existsSync()) { - await lcovFile.delete(); - } - - void noop(String? _) {} - final workingDirectory = Directory(p.normalize(cwd)).absolute.path; - final relativePath = p.relative(workingDirectory, from: initialCwd); - final path = relativePath == '.' - ? '.' - : '.${p.context.separator}$relativePath'; - - stdout?.call('Running "${testType.name} test" in $path ...\n'); - - if (!Directory(p.join(workingDirectory, 'test')).existsSync()) { - stdout?.call('No test folder found in $path\n'); - return ExitCode.success.code; - } - - if (randomSeed != null) { - stdout?.call( - '''Shuffling test order with --test-randomize-ordering-seed=$randomSeed\n''', - ); - } - final optimization = await optimizer.apply( - packageRoot: workingDirectory, - logger: logger, - ); - - if (optimization.isEmptyShard) { - stdout?.call( - 'No tests found for shard ${optimization.shardIndex} in $path\n', - ); - await optimization.cleanUp(); - // The merge step downstream still expects a report from every - // shard, so leave an empty one behind. - if (collectCoverage) await lcovFile.create(recursive: true); - return ExitCode.success.code; - } - - return await _overrideAnsiOutput( - forceAnsi, - () => - _testCommand( - cwd: cwd, - collectCoverage: collectCoverage, - testRunner: testRunner, - testType: testType, - optimization: optimization, - arguments: [ - ...?arguments, - if (randomSeed != null) ...[ - '--test-randomize-ordering-seed', - randomSeed, - ], - ...optimization.testTargets, - ], - stdout: stdout ?? noop, - stderr: stderr ?? noop, - ).whenComplete(() async { - await optimization.cleanUp(); - - // Dart don't directly generate lcov files, so we need - // to read the json that is generates and convert it to lcov. - if (testType == TestRunType.dart && collectCoverage) { - final files = _dartCoverageFilesToProcess( - p.join(cwd, 'coverage'), - ); - - final resolvedCwd = Directory(cwd).resolveSymbolicLinksSync(); - final resolvedReportOn = [ - for (final path in reportOn ?? ['lib']) - p.join(resolvedCwd, path), - ]; - - final hitmap = await coverage.HitMap.parseFiles( - files, - packagePath: resolvedCwd, - checkIgnoredLines: checkIgnore, - ); - - final resolver = await coverage.Resolver.create( - packagePath: resolvedCwd, - ); - - final output = hitmap.formatLcov( - resolver, - reportOn: resolvedReportOn, - basePath: resolvedCwd, - ); - - // Write the lcov output to the file. - await lcovFile.create(recursive: true); - await lcovFile.writeAsString(output); - - // If collectCoverageFrom is 'all', enhance with untested - // files - if (collectCoverageFrom == CoverageCollectionMode.all) { - await _enhanceLcovWithUntestedFiles( - cwd: cwd, - lcovPath: lcovPath, - reportOn: reportOn ?? ['lib'], - excludeFromCoverage: excludeFromCoverage, - ); - } - } - - if (collectCoverage) { - assert( - lcovFile.existsSync(), - 'coverage/lcov.info must exist', - ); - - // For Flutter tests with collectCoverageFrom = all, - // enhance lcov. - if (testType == TestRunType.flutter && - collectCoverageFrom == CoverageCollectionMode.all) { - await _enhanceLcovWithUntestedFiles( - lcovPath: lcovPath, - cwd: cwd, - reportOn: reportOn ?? ['lib'], - excludeFromCoverage: excludeFromCoverage, - ); - } - } - - if (minCoverage != null || showUncovered) { - final records = await Parser.parse(lcovPath); - final coverageMetrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: excludeFromCoverage, - ); - final coverage = coverageMetrics.percentage; - final uncoveredLines = - showUncovered && coverageMetrics.uncoveredLines.isNotEmpty - ? coverageMetrics.uncoveredLines - : null; - - if (minCoverage != null && coverage < minCoverage) { - throw MinCoverageNotMet( - coverage, - uncoveredLines: uncoveredLines, - ); - } - - // When coverage passes but is below 100%, - // show uncovered lines as informational output. - if (showUncovered && - uncoveredLines != null && - uncoveredLines.isNotEmpty) { - stdout?.call('${formatUncoveredLines(uncoveredLines)}\n'); - } - } - }), - ); - }, + cmd: (cwd) => _testPackage( + cwd: cwd, + initialCwd: initialCwd, + logger: logger, + testType: testType, + testRunner: testRunner, + optimizer: optimizer, + coverageOptions: coverageOptions, + randomSeed: randomSeed, + forceAnsi: forceAnsi, + arguments: arguments, + stdout: stdout, + stderr: stderr, + ), cwd: cwd, ignore: ignore, recursive: recursive, ); } + /// Runs the tests of the single package rooted at [cwd]. + static Future _testPackage({ + required String cwd, + required String initialCwd, + required Logger logger, + required TestRunType testType, + required VeryGoodTestRunner testRunner, + required TestOptimizer optimizer, + required _CoverageOptions coverageOptions, + required String? randomSeed, + required bool? forceAnsi, + required List? arguments, + required void Function(String)? stdout, + required void Function(String)? stderr, + }) async { + final lcovPath = p.join(cwd, 'coverage', 'lcov.info'); + final lcovFile = File(lcovPath); + + if (coverageOptions.collect && lcovFile.existsSync()) { + await lcovFile.delete(); + } + + void noop(String? _) {} + final workingDirectory = Directory(p.normalize(cwd)).absolute.path; + final path = _displayPath(workingDirectory, from: initialCwd); + + stdout?.call('Running "${testType.name} test" in $path ...\n'); + + if (!Directory(p.join(workingDirectory, 'test')).existsSync()) { + stdout?.call('No test folder found in $path\n'); + return ExitCode.success.code; + } + + if (randomSeed != null) { + stdout?.call( + '''Shuffling test order with --test-randomize-ordering-seed=$randomSeed\n''', + ); + } + final optimization = await optimizer.apply( + packageRoot: workingDirectory, + logger: logger, + ); + + if (optimization.isEmptyShard) { + stdout?.call( + 'No tests found for shard ${optimization.shardIndex} in $path\n', + ); + await optimization.cleanUp(); + // The merge step downstream still expects a report from every + // shard, so leave an empty one behind. + if (coverageOptions.collect) await lcovFile.create(recursive: true); + return ExitCode.success.code; + } + + return await _overrideAnsiOutput( + forceAnsi, + () => + _testCommand( + cwd: cwd, + collectCoverage: coverageOptions.collect, + testRunner: testRunner, + testType: testType, + optimization: optimization, + arguments: [ + ...?arguments, + if (randomSeed != null) ...[ + '--test-randomize-ordering-seed', + randomSeed, + ], + ...optimization.testTargets, + ], + stdout: stdout ?? noop, + stderr: stderr ?? noop, + ).whenComplete(() async { + await optimization.cleanUp(); + await _reportCoverage( + cwd: cwd, + lcovPath: lcovPath, + testType: testType, + options: coverageOptions, + stdout: stdout, + ); + }), + ); + } + + /// The path of [workingDirectory] relative to [from], as shown to the user. + static String _displayPath(String workingDirectory, {required String from}) { + final relativePath = p.relative(workingDirectory, from: from); + return relativePath == '.' ? '.' : '.${p.context.separator}$relativePath'; + } + + /// Writes the lcov report of a finished test run when coverage is + /// collected, then enforces the coverage threshold when one is set. + static Future _reportCoverage({ + required String cwd, + required String lcovPath, + required TestRunType testType, + required _CoverageOptions options, + required void Function(String)? stdout, + }) async { + if (options.collect) { + await _writeLcov( + cwd: cwd, + lcovPath: lcovPath, + testType: testType, + options: options, + ); + } + + if (options.minCoverage != null || options.showUncovered) { + await _checkCoverage( + lcovPath: lcovPath, + options: options, + stdout: stdout, + ); + } + } + + /// Leaves the coverage of the test run in `coverage/lcov.info`. + static Future _writeLcov({ + required String cwd, + required String lcovPath, + required TestRunType testType, + required _CoverageOptions options, + }) async { + // Dart don't directly generate lcov files, so we need + // to read the json that is generates and convert it to lcov. + if (testType == TestRunType.dart) { + await _convertDartCoverageToLcov( + cwd: cwd, + lcovFile: File(lcovPath), + options: options, + ); + } + + assert(File(lcovPath).existsSync(), 'coverage/lcov.info must exist'); + + if (options.collectFrom == CoverageCollectionMode.all) { + await _enhanceLcovWithUntestedFiles( + lcovPath: lcovPath, + cwd: cwd, + reportOn: options.reportOn, + excludeFromCoverage: options.excludeFromCoverage, + ); + } + } + + /// Converts the json coverage `dart test` writes into [lcovFile]. + static Future _convertDartCoverageToLcov({ + required String cwd, + required File lcovFile, + required _CoverageOptions options, + }) async { + final files = _dartCoverageFilesToProcess(p.join(cwd, 'coverage')); + + final resolvedCwd = Directory(cwd).resolveSymbolicLinksSync(); + final resolvedReportOn = [ + for (final path in options.reportOn) p.join(resolvedCwd, path), + ]; + + final hitmap = await coverage.HitMap.parseFiles( + files, + packagePath: resolvedCwd, + checkIgnoredLines: options.checkIgnore, + ); + + final resolver = await coverage.Resolver.create(packagePath: resolvedCwd); + + final output = hitmap.formatLcov( + resolver, + reportOn: resolvedReportOn, + basePath: resolvedCwd, + ); + + await lcovFile.create(recursive: true); + await lcovFile.writeAsString(output); + } + + /// Throws [MinCoverageNotMet] when the coverage in [lcovPath] is below the + /// threshold, and otherwise lists the uncovered lines when asked to. + static Future _checkCoverage({ + required String lcovPath, + required _CoverageOptions options, + required void Function(String)? stdout, + }) async { + final records = await Parser.parse(lcovPath); + final coverageMetrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: options.excludeFromCoverage, + ); + final percentage = coverageMetrics.percentage; + final uncoveredLines = + options.showUncovered && coverageMetrics.uncoveredLines.isNotEmpty + ? coverageMetrics.uncoveredLines + : null; + + final minCoverage = options.minCoverage; + if (minCoverage != null && percentage < minCoverage) { + throw MinCoverageNotMet(percentage, uncoveredLines: uncoveredLines); + } + + // When coverage passes but is below 100%, + // show uncovered lines as informational output. + if (uncoveredLines != null) { + stdout?.call('${formatUncoveredLines(uncoveredLines)}\n'); + } + } + static T _overrideAnsiOutput(bool? enableAnsiOutput, T Function() body) => enableAnsiOutput == null ? body.call() @@ -435,57 +509,55 @@ class TestCLIRunner { // Parse existing lcov to find covered files final existingRecords = await Parser.parse(lcovPath); - final coveredFiles = existingRecords - .where((r) => r.file != null) - .map((r) => r.file!) - .toSet(); + final coveredFiles = existingRecords.map((r) => r.file).nonNulls.toSet(); - // Find uncovered files final uncoveredFiles = allDartFiles.where((file) { final normalizedFile = p.normalize(file); - for (final covered in coveredFiles) { - if (p.normalize(covered).endsWith(normalizedFile)) { - return false; // File is covered - } - } - return true; // File is uncovered + return !coveredFiles.any( + (covered) => p.normalize(covered).endsWith(normalizedFile), + ); }).toList(); if (uncoveredFiles.isEmpty) return; // Append uncovered files to lcov - final lcovContent = await lcovFile.readAsString(); - final buffer = StringBuffer(lcovContent); + final buffer = StringBuffer(await lcovFile.readAsString()); for (final file in uncoveredFiles) { - final absolutePath = p.join(cwd, file); - final dartFile = File(absolutePath); - if (dartFile.existsSync()) { - final lines = await dartFile.readAsLines(); - buffer.writeln('SF:${file.replaceAll(r'\', '/')}'); - // Mark non-trivial lines as uncovered - var linesFound = 0; - for (var i = 1; i <= lines.length; i++) { - final line = lines[i - 1].trim(); - if (line.isNotEmpty && - !line.startsWith('//') && - !line.startsWith('import') && - !line.startsWith('export') && - !line.startsWith('part')) { - buffer.writeln('DA:$i,0'); - linesFound++; - } - } - buffer - ..writeln('LF:$linesFound') - ..writeln('LH:0') - ..writeln('end_of_record'); - } + final dartFile = File(p.join(cwd, file)); + if (!dartFile.existsSync()) continue; + buffer.write(_untestedFileRecord(file, await dartFile.readAsLines())); } await lcovFile.writeAsString(buffer.toString()); } + /// The lcov record of a [file] no test reached, given its [lines], where + /// every non-trivial line is marked as uncovered. + static String _untestedFileRecord(String file, List lines) { + final uncoveredLineNumbers = [ + for (final (index, line) in lines.indexed) + if (_isCoverableLine(line.trim())) index + 1, + ]; + + final record = StringBuffer()..writeln('SF:${file.replaceAll(r'\', '/')}'); + for (final lineNumber in uncoveredLineNumbers) { + record.writeln('DA:$lineNumber,0'); + } + record + ..writeln('LF:${uncoveredLineNumbers.length}') + ..writeln('LH:0') + ..writeln('end_of_record'); + return record.toString(); + } + + /// Whether a [trimmedLine] of source counts towards coverage. + static bool _isCoverableLine(String trimmedLine) => + trimmedLine.isNotEmpty && + !_nonCoverableLinePrefixes.any(trimmedLine.startsWith); + + static const _nonCoverableLinePrefixes = ['//', 'import', 'export', 'part']; + static List _dartCoverageFilesToProcess(String absPath) { return Directory(absPath) .listSync(recursive: true) @@ -495,10 +567,47 @@ class TestCLIRunner { } } +/// The coverage settings of a [TestCLIRunner.test] run. +class _CoverageOptions { + const new({ + required this.collect, + required this.collectFrom, + required this.minCoverage, + required this.showUncovered, + required this.excludeFromCoverage, + required this.reportOn, + required this.checkIgnore, + }); + + /// Whether to collect coverage into `coverage/lcov.info`. + final bool collect; + + /// Which files the lcov report accounts for. + final CoverageCollectionMode collectFrom; + + /// The minimum coverage percentage the run must reach, if any. + final double? minCoverage; + + /// Whether to list the lines left uncovered. + final bool showUncovered; + + /// A glob of the files left out of the coverage. + final String? excludeFromCoverage; + + /// The directories, relative to the package, the coverage reports on. + final List reportOn; + + /// Whether to honor the `coverage:ignore` comments. + final bool checkIgnore; +} + /// The exit code `dart test` and `flutter test` use when no test ran, for /// example because `--exclude-tags` filtered out every test. const _noTestsRanExitCode = 79; +/// Clears the current terminal line and moves the cursor to its start. +const _clearLine = '\u001B[2K\r'; + Future _testCommand({ required void Function(String) stdout, required void Function(String) stderr, @@ -509,32 +618,17 @@ Future _testCommand({ bool collectCoverage = false, List? arguments, }) { - const clearLine = '\u001B[2K\r'; - final completer = Completer(); - final suites = {}; - final groups = {}; - final tests = {}; - final failedTestErrorMessages = >{}; + final reporter = _TestEventReporter( + stdout: stdout, + stderr: stderr, + optimization: optimization, + cwd: cwd, + ); final sigintWatch = ProcessSignalOverrides.current?.sigintWatch ?? ProcessSignal.sigint.watch(); - var successCount = 0; - var skipCount = 0; - - String computeStats() { - final passingTests = successCount.formatSuccess(); - final failingTests = failedTestErrorMessages.values - .expand((e) => e) - .length - .formatFailure(); - final skippedTests = skipCount.formatSkipped(); - final result = [passingTests, failingTests, skippedTests] - ..removeWhere((element) => element.isEmpty); - return result.join(' '); - } - final timerSubscription = Stream.periodic( const Duration(seconds: 1), @@ -542,7 +636,7 @@ Future _testCommand({ ).listen((tick) { if (completer.isCompleted) return; final timeElapsed = Duration(seconds: tick).formatted(); - stdout('$clearLine$timeElapsed ...'); + stdout('$_clearLine$timeElapsed ...'); }); late final StreamSubscription subscription; @@ -559,148 +653,210 @@ Future _testCommand({ testRunner( workingDirectory: cwd, arguments: [ - if (collectCoverage) - if (testType == TestRunType.flutter) - '--coverage' - else - '--coverage=coverage', + if (collectCoverage) _coverageArgument(testType), ...?arguments, ], runInShell: true, ).listen( (event) async { if (event.shouldCancelTimer()) unawaited(timerSubscription.cancel()); - if (event is SuiteTestEvent) suites[event.suite.id] = event.suite; - if (event is GroupTestEvent) groups[event.group.id] = event.group; - if (event is TestStartEvent) tests[event.test.id] = event.test; - - if (event is MessageTestEvent) { - if (event.message.startsWith('Skip:')) { - stdout('$clearLine${lightYellow.wrap(event.message)}\n'); - } else if (event.message.contains('EXCEPTION')) { - stderr('$clearLine${event.message}'); - } else { - stdout('$clearLine${event.message}\n'); - } - } - - if (event is ErrorTestEvent) { - stderr('$clearLine${event.error}'); - - if (event.stackTrace.trim().isNotEmpty) { - stderr('$clearLine${event.stackTrace}'); - } - - final test = tests[event.testID]!; - final suite = suites[test.suiteID]!; - final prefix = event.isFailure ? '[FAILED]' : '[ERROR]'; - - final report = optimization.resolveReport( - suitePath: suite.path!, - testName: test.name, - groupName: _topGroupName(test, groups), - ); + reporter.report(event); - final relativeTestPath = p.relative(report.path, from: cwd); - failedTestErrorMessages[relativeTestPath] = [ - ...failedTestErrorMessages[relativeTestPath] ?? [], - '$prefix ${report.name}', - ]; - } - - if (event is TestDoneEvent) { - if (event.hidden) return; - - final test = tests[event.testID]!; - final suite = suites[test.suiteID]!; - - final report = optimization.resolveReport( - suitePath: suite.path!, - testName: test.name, - groupName: _topGroupName(test, groups), - ); - final testPath = report.path; - final testName = report.name; - - if (event.skipped) { - stdout( - '''$clearLine${lightYellow.wrap('$testName $testPath (SKIPPED)')}\n''', - ); - skipCount++; - } else if (event.result == TestResult.success) { - successCount++; - } else { - stderr('$clearLine$testName $testPath (FAILED)'); - } - - final timeElapsed = Duration(milliseconds: event.time).formatted(); - final stats = computeStats(); - final truncatedTestName = testName.toSingleLine().truncated( - _lineLength - (timeElapsed.length + stats.length + 2), - ); - stdout('''$clearLine$timeElapsed $stats: $truncatedTestName'''); - } - - if (event is DoneTestEvent) { - final timeElapsed = Duration(milliseconds: event.time).formatted(); - final stats = computeStats(); - final summary = event.success ?? false - ? lightGreen.wrap('All tests passed!')! - : lightRed.wrap('Some tests failed.')!; - - stdout( - '$clearLine${darkGray.wrap(timeElapsed)} $stats: $summary\n', - ); - - if (event.success != true) { - assert( - failedTestErrorMessages.isNotEmpty, - 'Invalid state: test event report as failed ' - 'but no failed tests were gathered', - ); - final title = styleBold.wrap('Failing Tests:'); - - final lines = StringBuffer('$clearLine$title\n'); - for (final testSuiteErrorMessages - in failedTestErrorMessages.entries) { - lines.writeln('$clearLine - ${testSuiteErrorMessages.key} '); - - for (final errorMessage in testSuiteErrorMessages.value) { - lines.writeln('$clearLine \t- $errorMessage'); - } - } - - stderr(lines.toString()); - } - } - - if (event is ExitTestEvent) { - if (completer.isCompleted) return; - unawaited(subscription.cancel()); - unawaited(sigintWatchSubscription.cancel()); - - // A shard can end up holding only tests that the given tags - // filter out, which is expected and not a failure. - final noTestsRanInShard = - optimization.shardIndex != null && - event.exitCode == _noTestsRanExitCode; - - completer.complete( - event.exitCode == ExitCode.success.code || noTestsRanInShard - ? ExitCode.success.code - : ExitCode.unavailable.code, - ); - } + if (event is! ExitTestEvent || completer.isCompleted) return; + unawaited(subscription.cancel()); + unawaited(sigintWatchSubscription.cancel()); + completer.complete(_exitCodeOf(event, optimization)); }, onError: (Object error, StackTrace stackTrace) { - stderr('$clearLine$error'); - stderr('$clearLine$stackTrace'); + stderr('$_clearLine$error'); + stderr('$_clearLine$stackTrace'); }, ); return completer.future; } +/// The argument that makes [testType] collect coverage. +String _coverageArgument(TestRunType testType) => switch (testType) { + TestRunType.flutter => '--coverage', + TestRunType.dart => '--coverage=coverage', +}; + +/// The exit code a test run that ended with [event] reports. +int _exitCodeOf(ExitTestEvent event, TestOptimization optimization) { + // A shard can end up holding only tests that the given tags + // filter out, which is expected and not a failure. + final noTestsRanInShard = + optimization.shardIndex != null && event.exitCode == _noTestsRanExitCode; + + return event.exitCode == ExitCode.success.code || noTestsRanInShard + ? ExitCode.success.code + : ExitCode.unavailable.code; +} + +/// Prints the progress of a test run as its [TestEvent]s arrive, and keeps +/// the tally of passing, skipped and failing tests the summary is made of. +class _TestEventReporter { + new({ + required this.stdout, + required this.stderr, + required this.optimization, + required this.cwd, + }); + + final void Function(String) stdout; + final void Function(String) stderr; + final TestOptimization optimization; + final String cwd; + + final _suites = {}; + final _groups = {}; + final _tests = {}; + final _failedTestErrorMessages = >{}; + + var _successCount = 0; + var _skipCount = 0; + + void report(TestEvent event) { + switch (event) { + case SuiteTestEvent(:final suite): + _suites[suite.id] = suite; + case GroupTestEvent(:final group): + _groups[group.id] = group; + case TestStartEvent(:final test): + _tests[test.id] = test; + case MessageTestEvent(): + _reportMessage(event); + case ErrorTestEvent(): + _reportError(event); + case TestDoneEvent(): + _reportTestDone(event); + case DoneTestEvent(): + _reportDone(event); + } + } + + void _reportMessage(MessageTestEvent event) { + final message = event.message; + if (message.startsWith('Skip:')) { + stdout('$_clearLine${lightYellow.wrap(message)}\n'); + } else if (message.contains('EXCEPTION')) { + stderr('$_clearLine$message'); + } else { + stdout('$_clearLine$message\n'); + } + } + + void _reportError(ErrorTestEvent event) { + stderr('$_clearLine${event.error}'); + + if (event.stackTrace.trim().isNotEmpty) { + stderr('$_clearLine${event.stackTrace}'); + } + + final report = _resolveReport(event.testID); + final prefix = event.isFailure ? '[FAILED]' : '[ERROR]'; + + final relativeTestPath = p.relative(report.path, from: cwd); + _failedTestErrorMessages[relativeTestPath] = [ + ...?_failedTestErrorMessages[relativeTestPath], + '$prefix ${report.name}', + ]; + } + + void _reportTestDone(TestDoneEvent event) { + if (event.hidden) return; + + final (:path, :name) = _resolveReport(event.testID); + _tallyResult(event, testPath: path, testName: name); + + final timeElapsed = Duration(milliseconds: event.time).formatted(); + final stats = _stats(); + final truncatedTestName = name.toSingleLine().truncated( + _lineLength - (timeElapsed.length + stats.length + 2), + ); + stdout('''$_clearLine$timeElapsed $stats: $truncatedTestName'''); + } + + void _tallyResult( + TestDoneEvent event, { + required String testPath, + required String testName, + }) { + if (event.skipped) { + stdout( + '''$_clearLine${lightYellow.wrap('$testName $testPath (SKIPPED)')}\n''', + ); + _skipCount++; + } else if (event.result == TestResult.success) { + _successCount++; + } else { + stderr('$_clearLine$testName $testPath (FAILED)'); + } + } + + void _reportDone(DoneTestEvent event) { + final timeElapsed = Duration(milliseconds: event.time).formatted(); + final stats = _stats(); + final success = event.success ?? false; + final summary = success + ? lightGreen.wrap('All tests passed!')! + : lightRed.wrap('Some tests failed.')!; + + stdout('$_clearLine${darkGray.wrap(timeElapsed)} $stats: $summary\n'); + + if (success) return; + + assert( + _failedTestErrorMessages.isNotEmpty, + 'Invalid state: test event report as failed ' + 'but no failed tests were gathered', + ); + stderr(_failingTestsSummary()); + } + + String _failingTestsSummary() { + final title = styleBold.wrap('Failing Tests:'); + + final lines = StringBuffer('$_clearLine$title\n'); + for (final MapEntry(key: testPath, value: errorMessages) + in _failedTestErrorMessages.entries) { + lines.writeln('$_clearLine - $testPath '); + + for (final errorMessage in errorMessages) { + lines.writeln('$_clearLine \t- $errorMessage'); + } + } + + return lines.toString(); + } + + /// The file and name [testID] is reported under, once the bundling of the + /// test optimizer is undone. + ({String path, String name}) _resolveReport(int testID) { + final test = _tests[testID]!; + final suite = _suites[test.suiteID]!; + + return optimization.resolveReport( + suitePath: suite.path!, + testName: test.name, + groupName: _topGroupName(test, _groups), + ); + } + + String _stats() { + final passingTests = _successCount.formatSuccess(); + final failingTests = _failedTestErrorMessages.values + .expand((e) => e) + .length + .formatFailure(); + final skippedTests = _skipCount.formatSkipped(); + final result = [passingTests, failingTests, skippedTests] + ..removeWhere((element) => element.isEmpty); + return result.join(' '); + } +} + /// The name of the outermost non-empty group [test] belongs to. /// /// For a test running inside the optimized bundle this is the path of the file diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index ebcfbf509..d38a63f61 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -222,6 +222,18 @@ class DartTestOptions { optimizePerformance && !TestCLIRunner.isTargettingTestFiles(rest) && platform == null; + + /// The arguments forwarded verbatim to `dart test`. + List get _testArguments => [ + if (excludeTags != null) ...['-x', excludeTags!], + if (tags != null) ...['-t', tags!], + if (failFast) '--fail-fast', + if (runSkipped) '--run-skipped', + if (platform != null) ...['--platform', platform!], + if (platform == null) ...['-j', concurrency], + if (fileReporter != null) '--file-reporter=$fileReporter', + ...rest, + ]; } /// Signature for the [Dart.installed] method. @@ -429,24 +441,10 @@ class DartTestCommand extends Command { @override Future run() async { final targetPath = path.normalize(Directory.current.absolute.path); - final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; - if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { - _logger.err(''' -Cannot target specific test files together with --recursive. -Test targets are resolved against a single package root, so the same path -cannot apply to every package. Drop --recursive and run from the package -that contains them.'''); - return ExitCode.usage.code; - } - - if (!recursive && !pubspec.existsSync()) { - _logger.err(''' -Could not find a pubspec.yaml in $targetPath. -This command should be run from the root of your Dart project.'''); - return ExitCode.noInput.code; - } + final targetError = _validateTarget(targetPath, recursive: recursive); + if (targetError != null) return targetError; final config = VeryGoodConfig.load(Directory(targetPath), logger: _logger); if (config == null) return ExitCode.config.code; @@ -468,6 +466,37 @@ This command should be run from the root of your Dart project.'''); return ExitCode.usage.code; } + return await _runTests(options, recursive: recursive); + } + + /// Returns the exit code to stop with when the run cannot target + /// [targetPath], or `null` when it can proceed. + int? _validateTarget(String targetPath, {required bool recursive}) { + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + + final pubspec = File(path.join(targetPath, 'pubspec.yaml')); + if (!recursive && !pubspec.existsSync()) { + _logger.err(''' +Could not find a pubspec.yaml in $targetPath. +This command should be run from the root of your Dart project.'''); + return ExitCode.noInput.code; + } + + return null; + } + + /// Runs `dart test` with [options] and maps its outcome to an exit code. + Future _runTests( + DartTestOptions options, { + required bool recursive, + }) async { // A threshold inherited from very_good.yaml only applies to un-sharded // runs: a single shard covers a fraction of the code and would fail it. final minCoverage = options.totalShards == null @@ -492,17 +521,7 @@ This command should be run from the root of your Dart project.'''); collectCoverageFrom: options.collectCoverageFrom, randomSeed: options.randomSeed, forceAnsi: options.forceAnsi, - arguments: [ - if (options.excludeTags != null) ...['-x', options.excludeTags!], - if (options.tags != null) ...['-t', options.tags!], - if (options.failFast) '--fail-fast', - if (options.runSkipped) '--run-skipped', - if (options.platform != null) ...['--platform', options.platform!], - if (options.platform == null) ...['-j', options.concurrency], - if (options.fileReporter != null) - '--file-reporter=${options.fileReporter}', - ...options.rest, - ], + arguments: options._testArguments, reportOn: options.reportOn.isEmpty ? null : options.reportOn, checkIgnore: options.checkIgnore, shardIndex: int.tryParse(options.shardIndex ?? ''), diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 8cbe2ea1f..90c56a935 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -262,6 +262,30 @@ class FlutterTestOptions { !TestCLIRunner.isTargettingTestFiles(rest) && !updateGoldens && platform == null; + + /// The arguments forwarded to `flutter test`. + List get _flutterTestArguments => [ + if (excludeTags != null) ...['-x', excludeTags!], + if (tags != null) ...['-t', tags!], + if (updateGoldens) '--update-goldens', + if (failFast) '--fail-fast', + if (runSkipped) '--run-skipped', + if (flavor != null) ...['--flavor', flavor!], + if (platform != null) ...['--platform', platform!], + ..._dartDefineArguments, + if (platform == null) ...['-j', concurrency], + '--no-pub', + if (timeout != null) '--timeout=${timeout!.inSeconds}s', + if (fileReporter != null) '--file-reporter=$fileReporter', + ...rest, + ]; + + /// The `--dart-define` and `--dart-define-from-file` arguments. + List get _dartDefineArguments => [ + for (final value in dartDefine ?? const []) '--dart-define=$value', + for (final value in dartDefineFromFile ?? const []) + '--dart-define-from-file=$value', + ]; } /// Signature for the [Flutter.installed] method. @@ -508,24 +532,10 @@ class TestCommand extends Command { @override Future run() async { final targetPath = path.normalize(Directory.current.absolute.path); - final pubspec = File(path.join(targetPath, 'pubspec.yaml')); final recursive = _argResults['recursive'] as bool; - if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { - _logger.err(''' -Cannot target specific test files together with --recursive. -Test targets are resolved against a single package root, so the same path -cannot apply to every package. Drop --recursive and run from the package -that contains them.'''); - return ExitCode.usage.code; - } - - if (!recursive && !pubspec.existsSync()) { - _logger.err(''' -Could not find a pubspec.yaml in $targetPath. -This command should be run from the root of your Flutter project.'''); - return ExitCode.noInput.code; - } + final targetError = _validateTarget(targetPath, recursive: recursive); + if (targetError != null) return targetError; final config = VeryGoodConfig.load(Directory(targetPath), logger: _logger); if (config == null) return ExitCode.config.code; @@ -547,6 +557,37 @@ This command should be run from the root of your Flutter project.'''); return ExitCode.usage.code; } + return await _runFlutterTest(options, recursive: recursive); + } + + /// Logs and returns the exit code for a [targetPath] the command cannot run + /// against, or returns `null` when the target is valid. + int? _validateTarget(String targetPath, {required bool recursive}) { + if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { + _logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + + final pubspec = File(path.join(targetPath, 'pubspec.yaml')); + if (!recursive && !pubspec.existsSync()) { + _logger.err(''' +Could not find a pubspec.yaml in $targetPath. +This command should be run from the root of your Flutter project.'''); + return ExitCode.noInput.code; + } + + return null; + } + + /// Runs `flutter test` with [options] and maps its outcome to an exit code. + Future _runFlutterTest( + FlutterTestOptions options, { + required bool recursive, + }) async { // A threshold inherited from very_good.yaml only applies to un-sharded // runs: a single shard covers a fraction of the code and would fail it. final minCoverage = options.totalShards == null @@ -574,27 +615,7 @@ This command should be run from the root of your Flutter project.'''); reportOn: options.reportOn.isEmpty ? null : options.reportOn, shardIndex: int.tryParse(options.shardIndex ?? ''), totalShards: int.tryParse(options.totalShards ?? ''), - arguments: [ - if (options.excludeTags != null) ...['-x', options.excludeTags!], - if (options.tags != null) ...['-t', options.tags!], - if (options.updateGoldens) '--update-goldens', - if (options.failFast) '--fail-fast', - if (options.runSkipped) '--run-skipped', - if (options.flavor != null) ...['--flavor', options.flavor!], - if (options.platform != null) ...['--platform', options.platform!], - if (options.dartDefine != null) - for (final value in options.dartDefine!) '--dart-define=$value', - if (options.dartDefineFromFile != null) - for (final value in options.dartDefineFromFile!) - '--dart-define-from-file=$value', - if (options.platform == null) ...['-j', options.concurrency], - '--no-pub', - if (options.timeout != null) - '--timeout=${options.timeout!.inSeconds}s', - if (options.fileReporter != null) - '--file-reporter=${options.fileReporter}', - ...options.rest, - ], + arguments: options._flutterTestArguments, ); if (results.any((code) => code != ExitCode.success.code)) { From 8c331335f3050b98bdb3e3890652a322c6dac012 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 13:18:45 +0300 Subject: [PATCH 2/8] refactor(test): address review nits in the test runner Fix the grammar of the dart lcov conversion comment, document that excludeFromCoverage holds space-separated globs, drop the unused async from the test event listener, and skip computing uncovered lines for lcov records without a source file. --- lib/src/cli/flutter_cli.dart | 5 +++-- lib/src/cli/test_cli_runner.dart | 8 ++++---- test/src/cli/flutter_cli_test.dart | 15 +++++++++++++++ 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 3f245d553..7abcaf312 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -106,11 +106,12 @@ class CoverageMetrics { Map> _uncoveredLinesWith(Record record) { final file = record.file; + if (file == null) return uncoveredLines; + final newLines = _uncoveredLineNumbersOf(record); return { ...uncoveredLines, - if (file != null && newLines.isNotEmpty) - file: [...?uncoveredLines[file], ...newLines], + if (newLines.isNotEmpty) file: [...?uncoveredLines[file], ...newLines], }; } diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index ca8191e2d..77d60ffb4 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -328,8 +328,8 @@ class TestCLIRunner { required TestRunType testType, required _CoverageOptions options, }) async { - // Dart don't directly generate lcov files, so we need - // to read the json that is generates and convert it to lcov. + // Dart doesn't generate lcov files directly, so convert the json + // coverage it writes into lcov. if (testType == TestRunType.dart) { await _convertDartCoverageToLcov( cwd: cwd, @@ -591,7 +591,7 @@ class _CoverageOptions { /// Whether to list the lines left uncovered. final bool showUncovered; - /// A glob of the files left out of the coverage. + /// Space-separated globs of the files left out of the coverage. final String? excludeFromCoverage; /// The directories, relative to the package, the coverage reports on. @@ -658,7 +658,7 @@ Future _testCommand({ ], runInShell: true, ).listen( - (event) async { + (event) { if (event.shouldCancelTimer()) unawaited(timerSubscription.cancel()); reporter.report(event); diff --git a/test/src/cli/flutter_cli_test.dart b/test/src/cli/flutter_cli_test.dart index 9c675e8a1..f4427b053 100644 --- a/test/src/cli/flutter_cli_test.dart +++ b/test/src/cli/flutter_cli_test.dart @@ -396,6 +396,21 @@ void main() { ); }); + test('counts records without a source file but skips their lines', () { + final records = parseRecords([ + 'DA:1,0', + 'LF:1', + 'LH:0', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect(records.single.file, isNull); + expect(metrics.totalFound, equals(1)); + expect(metrics.uncoveredLines, isEmpty); + }); + test('handles records with no DA entries', () { final records = parseRecords([ 'SF:lib/a.dart', From 348440d009672481b9d3318e27e7381eaa93919a Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 13:18:46 +0300 Subject: [PATCH 3/8] refactor(test): share target validation between test commands TestCommand and DartTestCommand carried the same _validateTarget, only differing in the project name of the message. Move it to TestCLIRunner.validateTarget and align the run helpers on _runTests. The rest-argument tests read argResults.rest inside verify(), which made mocktail verify that getter instead of the runner call. They now verify the runner with a local list, since validateTarget reads rest up front. --- lib/src/cli/test_cli_runner.dart | 33 +++++++++++++++++ .../dart/commands/dart_test_command.dart | 31 ++++------------ lib/src/commands/test/test.dart | 35 +++++-------------- .../dart/commands/dart_test_test.dart | 10 +++--- test/src/commands/test/test_test.dart | 10 +++--- 5 files changed, 61 insertions(+), 58 deletions(-) diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 77d60ffb4..43136eb85 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -142,6 +142,39 @@ class TestCLIRunner { return null; } + /// Validates that the tests can run against [targetPath]. + /// + /// Logs the problem and returns the exit code to stop with, or returns + /// `null` when the run can proceed. [rest] are the positional arguments of + /// the command and [projectKind] names the project in the messages, such as + /// `Flutter` or `Dart`. + static int? validateTarget({ + required String targetPath, + required bool recursive, + required List rest, + required String projectKind, + required Logger logger, + }) { + if (recursive && isTargettingTestFiles(rest)) { + logger.err(''' +Cannot target specific test files together with --recursive. +Test targets are resolved against a single package root, so the same path +cannot apply to every package. Drop --recursive and run from the package +that contains them.'''); + return ExitCode.usage.code; + } + + final pubspec = File(p.join(targetPath, 'pubspec.yaml')); + if (!recursive && !pubspec.existsSync()) { + logger.err(''' +Could not find a pubspec.yaml in $targetPath. +This command should be run from the root of your $projectKind project.'''); + return ExitCode.noInput.code; + } + + return null; + } + /// Run tests (`flutter test`). /// Returns a list of exit codes for each test process. static Future> test({ diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index d38a63f61..18858cf21 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -443,7 +443,13 @@ class DartTestCommand extends Command { final targetPath = path.normalize(Directory.current.absolute.path); final recursive = _argResults['recursive'] as bool; - final targetError = _validateTarget(targetPath, recursive: recursive); + final targetError = TestCLIRunner.validateTarget( + targetPath: targetPath, + recursive: recursive, + rest: _argResults.rest, + projectKind: 'Dart', + logger: _logger, + ); if (targetError != null) return targetError; final config = VeryGoodConfig.load(Directory(targetPath), logger: _logger); @@ -469,29 +475,6 @@ class DartTestCommand extends Command { return await _runTests(options, recursive: recursive); } - /// Returns the exit code to stop with when the run cannot target - /// [targetPath], or `null` when it can proceed. - int? _validateTarget(String targetPath, {required bool recursive}) { - if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { - _logger.err(''' -Cannot target specific test files together with --recursive. -Test targets are resolved against a single package root, so the same path -cannot apply to every package. Drop --recursive and run from the package -that contains them.'''); - return ExitCode.usage.code; - } - - final pubspec = File(path.join(targetPath, 'pubspec.yaml')); - if (!recursive && !pubspec.existsSync()) { - _logger.err(''' -Could not find a pubspec.yaml in $targetPath. -This command should be run from the root of your Dart project.'''); - return ExitCode.noInput.code; - } - - return null; - } - /// Runs `dart test` with [options] and maps its outcome to an exit code. Future _runTests( DartTestOptions options, { diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 90c56a935..819949220 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -534,7 +534,13 @@ class TestCommand extends Command { final targetPath = path.normalize(Directory.current.absolute.path); final recursive = _argResults['recursive'] as bool; - final targetError = _validateTarget(targetPath, recursive: recursive); + final targetError = TestCLIRunner.validateTarget( + targetPath: targetPath, + recursive: recursive, + rest: _argResults.rest, + projectKind: 'Flutter', + logger: _logger, + ); if (targetError != null) return targetError; final config = VeryGoodConfig.load(Directory(targetPath), logger: _logger); @@ -557,34 +563,11 @@ class TestCommand extends Command { return ExitCode.usage.code; } - return await _runFlutterTest(options, recursive: recursive); - } - - /// Logs and returns the exit code for a [targetPath] the command cannot run - /// against, or returns `null` when the target is valid. - int? _validateTarget(String targetPath, {required bool recursive}) { - if (recursive && TestCLIRunner.isTargettingTestFiles(_argResults.rest)) { - _logger.err(''' -Cannot target specific test files together with --recursive. -Test targets are resolved against a single package root, so the same path -cannot apply to every package. Drop --recursive and run from the package -that contains them.'''); - return ExitCode.usage.code; - } - - final pubspec = File(path.join(targetPath, 'pubspec.yaml')); - if (!recursive && !pubspec.existsSync()) { - _logger.err(''' -Could not find a pubspec.yaml in $targetPath. -This command should be run from the root of your Flutter project.'''); - return ExitCode.noInput.code; - } - - return null; + return await _runTests(options, recursive: recursive); } /// Runs `flutter test` with [options] and maps its outcome to an exit code. - Future _runFlutterTest( + Future _runTests( FlutterTestOptions options, { required bool recursive, }) async { diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 974fc0213..81decfc3e 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -807,14 +807,15 @@ void main() { test( '''disables optimizePerformance when rest arguement is not an option''', () async { - when(() => argResults.rest).thenReturn(['my-test.dart']); + final rest = ['my-test.dart']; + when(() => argResults.rest).thenReturn(rest); final result = await testCommand.run(); expect(result, equals(ExitCode.success.code)); verify( () => dartTest( - arguments: [...defaultArguments, ...argResults.rest], + arguments: [...defaultArguments, ...rest], logger: logger, stdout: logger.write, stderr: logger.err, @@ -858,7 +859,8 @@ void main() { test( 'enables optimizePerformance when rest arguement is an option', () async { - when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + final rest = ['--track-wdiget-creation']; + when(() => argResults.rest).thenReturn(rest); final result = await testCommand.run(); @@ -866,7 +868,7 @@ void main() { verify( () => dartTest( optimizePerformance: true, - arguments: [...defaultArguments, ...argResults.rest], + arguments: [...defaultArguments, ...rest], logger: logger, stdout: logger.write, stderr: logger.err, diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 4233e6a40..bad9066e6 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -580,14 +580,15 @@ void main() { test( '''disables optimizePerformance when rest arguement is not an option''', () async { - when(() => argResults.rest).thenReturn(['my-test.dart']); + final rest = ['my-test.dart']; + when(() => argResults.rest).thenReturn(rest); final result = await testCommand.run(); expect(result, equals(ExitCode.success.code)); verify( () => flutterTest( - arguments: [...defaultArguments, ...argResults.rest], + arguments: [...defaultArguments, ...rest], logger: logger, stdout: logger.write, stderr: logger.err, @@ -632,7 +633,8 @@ void main() { test( 'enables optimizePerformance when rest arguement is an option', () async { - when(() => argResults.rest).thenReturn(['--track-wdiget-creation']); + final rest = ['--track-wdiget-creation']; + when(() => argResults.rest).thenReturn(rest); final result = await testCommand.run(); @@ -640,7 +642,7 @@ void main() { verify( () => flutterTest( optimizePerformance: true, - arguments: [...defaultArguments, ...argResults.rest], + arguments: [...defaultArguments, ...rest], logger: logger, stdout: logger.write, stderr: logger.err, From f7febe708238db5f95c39a0f33f0f3abb818b465 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 13:18:46 +0300 Subject: [PATCH 4/8] fix(test): honor every exclude-coverage glob for untested files With collect-coverage-from all, the untested-files step parsed --exclude-coverage as a single glob, so the documented '**/*.g.dart **/*.freezed.dart' excluded nothing. It also matched the absolute file path, which package:glob resolves against the process working directory rather than the package root. Parse the value the same way CoverageMetrics does and match the path relative to the package, as the lcov SF entries are. --- lib/src/cli/flutter_cli.dart | 15 ++++--- lib/src/cli/test_cli_runner.dart | 6 +-- test/src/cli/test_cli_runner_test.dart | 60 ++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 7abcaf312..8d0d90fa0 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -70,7 +70,7 @@ class CoverageMetrics { /// Generate coverage metrics from a list of lcov records. factory fromLcovRecords(List records, {String? excludeFromCoverage}) { - final excludedGlobs = _parseGlobs(excludeFromCoverage); + final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); return records .whereNot((record) => _isExcluded(record.file, excludedGlobs)) .fold( @@ -79,12 +79,6 @@ class CoverageMetrics { ); } - /// Parses space-separated glob patterns, ignoring empty segments. - static List _parseGlobs(String? excludeFromCoverage) => [ - for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) - if (pattern.isNotEmpty) Glob(pattern), - ]; - static bool _isExcluded(String? file, List excludedGlobs) => file != null && excludedGlobs.any((glob) => glob.matches(file)); @@ -133,6 +127,13 @@ class CoverageMetrics { } } +/// Parses the space-separated glob patterns of an `--exclude-coverage` +/// value, ignoring empty segments. +List _parseExcludeGlobs(String? excludeFromCoverage) => [ + for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) + if (pattern.isNotEmpty) Glob(pattern), +]; + /// Flutter CLI class Flutter { /// Determine whether flutter is installed. diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 43136eb85..ac9ae4166 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -508,7 +508,7 @@ This command should be run from the root of your $projectKind project.'''); required List reportOn, String? excludeFromCoverage, }) { - final glob = excludeFromCoverage != null ? Glob(excludeFromCoverage) : null; + final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); return reportOn.expand((dir) { final reportOnPath = p.join(cwd, dir); @@ -520,8 +520,8 @@ This command should be run from the root of your $projectKind project.'''); .listSync(recursive: true) .whereType() .where((file) => file.path.endsWith('.dart')) - .where((file) => glob == null || !glob.matches(file.path)) - .map((file) => p.relative(file.path, from: cwd)); + .map((file) => p.relative(file.path, from: cwd)) + .whereNot((file) => excludedGlobs.any((glob) => glob.matches(file))); }).toList(); } diff --git a/test/src/cli/test_cli_runner_test.dart b/test/src/cli/test_cli_runner_test.dart index e847bfc91..4ed5a2879 100644 --- a/test/src/cli/test_cli_runner_test.dart +++ b/test/src/cli/test_cli_runner_test.dart @@ -2071,6 +2071,66 @@ void main() { expect(lcovFile.existsSync(), isTrue); }); + + test('respects every space-separated exclude-coverage pattern ' + 'when enhancing lcov', () async { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final libDir = Directory(p.join(tempDirectory.path, 'lib')) + ..createSync(recursive: true); + for (final name in [ + 'main.dart', + 'untested.dart', + 'main.g.dart', + 'main.freezed.dart', + ]) { + File(p.join(libDir.path, name)).writeAsStringSync('void f() {}'); + } + + File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync(); + Directory(p.join(tempDirectory.path, 'test')).createSync(); + + final lcovFile = File( + p.join(tempDirectory.path, 'coverage', 'lcov.info'), + ); + + await expectLater( + TestCLIRunner.test( + testType: TestRunType.flutter, + cwd: tempDirectory.path, + logger: logger, + collectCoverage: true, + collectCoverageFrom: CoverageCollectionMode.all, + excludeFromCoverage: '**/*.g.dart **/*.freezed.dart', + stdout: stdoutLogs.add, + stderr: stderrLogs.add, + overrideTestRunner: testRunner( + Stream.fromIterable([ + const DoneTestEvent(success: true, time: 0), + const ExitTestEvent(exitCode: 0, time: 0), + ]), + onStart: () { + lcovFile + ..createSync(recursive: true) + ..writeAsStringSync( + 'SF:lib/main.dart\n' + 'DA:1,1\n' + 'LF:1\n' + 'LH:1\n' + 'end_of_record\n', + ); + }, + ), + ), + completion(equals([ExitCode.success.code])), + ); + + final lcov = lcovFile.readAsStringSync(); + expect(lcov, contains('SF:lib/untested.dart')); + expect(lcov, isNot(contains('main.g.dart'))); + expect(lcov, isNot(contains('main.freezed.dart'))); + }); }); test( From 8eabcd2ccba52a69309e10b3ac959230ce4d9e16 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 13:24:53 +0300 Subject: [PATCH 5/8] refactor(coverage): move coverage handling into lib/src/coverage TestCLIRunner wrote, converted and checked coverage through a chain of static helpers that each took the package root, the lcov path, the test type and the coverage options, and branched on the test type to tell dart and flutter runs apart. Coverage now lives in its own library. CoverageOptions holds the settings of a run, and a sealed CoverageReport per package owns the lcov file: FlutterCoverageReport keeps what flutter test wrote, while DartCoverageReport converts the json coverage of dart test. The runner picks the report once and no longer branches on the test type for coverage, including the argument that turns coverage collection on. CoverageMetrics, CoverageCollectionMode, MinCoverageNotMet and formatUncoveredLines move along with it, so lib/src/cli depends on lib/src/coverage and not the other way around. The CoverageMetrics tests move to test/src/coverage, next to new CoverageReport tests. --- .gitignore | 3 + lib/src/cli/cli.dart | 3 +- lib/src/cli/flutter_cli.dart | 78 ---- lib/src/cli/test_cli_runner.dart | 351 +----------------- .../dart/commands/dart_test_command.dart | 1 + lib/src/commands/test/test.dart | 1 + lib/src/coverage/coverage.dart | 11 + lib/src/coverage/coverage_metrics.dart | 132 +++++++ lib/src/coverage/coverage_report.dart | 190 ++++++++++ lib/src/coverage/untested_files.dart | 90 +++++ test/src/cli/flutter_cli_test.dart | 236 ------------ test/src/cli/test_cli_runner_test.dart | 1 + .../dart/commands/dart_test_test.dart | 1 + test/src/commands/test/test_test.dart | 1 + test/src/coverage/coverage_metrics_test.dart | 254 +++++++++++++ test/src/coverage/coverage_report_test.dart | 279 ++++++++++++++ 16 files changed, 984 insertions(+), 648 deletions(-) create mode 100644 lib/src/coverage/coverage.dart create mode 100644 lib/src/coverage/coverage_metrics.dart create mode 100644 lib/src/coverage/coverage_report.dart create mode 100644 lib/src/coverage/untested_files.dart create mode 100644 test/src/coverage/coverage_metrics_test.dart create mode 100644 test/src/coverage/coverage_report_test.dart diff --git a/.gitignore b/.gitignore index 4ea56d1fb..bab489442 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,9 @@ doc/api/ # Files generated during tests .test_coverage.dart coverage/ +# The coverage library and its tests are sources, not reports. +!lib/src/coverage/ +!test/src/coverage/ .test_optimizer.dart !bricks/test_optimizer/__brick__/test/.test_optimizer.dart *.vm.json diff --git a/lib/src/cli/cli.dart b/lib/src/cli/cli.dart index 3e90a116f..edc35778d 100644 --- a/lib/src/cli/cli.dart +++ b/lib/src/cli/cli.dart @@ -2,14 +2,13 @@ import 'dart:async'; import 'dart:math'; import 'package:collection/collection.dart'; -import 'package:coverage/coverage.dart' as coverage; import 'package:glob/glob.dart'; -import 'package:lcov_parser/lcov_parser.dart'; import 'package:mason/mason.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:pubspec_parse/pubspec_parse.dart'; import 'package:universal_io/io.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/test_optimizer/test_optimizer.dart'; import 'package:very_good_test_runner/very_good_test_runner.dart'; diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 8d0d90fa0..9a8656e17 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -56,84 +56,6 @@ class _ProcessSignalOverridesScope extends ProcessSignalOverrides { /// Thrown when `flutter pub get` is executed without a `pubspec.yaml`. class PubspecNotFound implements Exception; -/// {@template coverage_metrics} -/// Aggregated coverage metrics computed from a list of LCOV records. -/// {@endtemplate} -class CoverageMetrics { - /// {@macro coverage_metrics} - @visibleForTesting - const new({ - this.totalHits = 0, - this.totalFound = 0, - this.uncoveredLines = const {}, - }); - - /// Generate coverage metrics from a list of lcov records. - factory fromLcovRecords(List records, {String? excludeFromCoverage}) { - final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); - return records - .whereNot((record) => _isExcluded(record.file, excludedGlobs)) - .fold( - const CoverageMetrics(), - (metrics, record) => metrics._add(record), - ); - } - - static bool _isExcluded(String? file, List excludedGlobs) => - file != null && excludedGlobs.any((glob) => glob.matches(file)); - - /// Line numbers in [record] that were instrumented but never hit. - static List _uncoveredLineNumbersOf(Record record) => [ - for (final detail in record.lines?.details ?? const []) - if (detail.line case final line? when (detail.hit ?? 1) == 0) line, - ]; - - /// Returns new metrics with the counts and uncovered lines of [record]. - CoverageMetrics _add(Record record) { - final lines = record.lines; - return CoverageMetrics( - totalFound: totalFound + (lines?.found ?? 0), - totalHits: totalHits + (lines?.hit ?? 0), - uncoveredLines: _uncoveredLinesWith(record), - ); - } - - Map> _uncoveredLinesWith(Record record) { - final file = record.file; - if (file == null) return uncoveredLines; - - final newLines = _uncoveredLineNumbersOf(record); - return { - ...uncoveredLines, - if (newLines.isNotEmpty) file: [...?uncoveredLines[file], ...newLines], - }; - } - - /// Total number of lines hit (covered) across all included files. - final int totalHits; - - /// Total number of instrumented lines found across all included files. - final int totalFound; - - /// Lines not covered. - /// Keyed by file path, values are sorted line numbers. - final Map> uncoveredLines; - - /// Coverage percentage: [totalHits] / [totalFound] * 100. - /// - /// Returns `0` when [totalFound] is less than 1. - double get percentage { - return totalFound < 1 ? 0 : (totalHits / totalFound * 100); - } -} - -/// Parses the space-separated glob patterns of an `--exclude-coverage` -/// value, ignoring empty segments. -List _parseExcludeGlobs(String? excludeFromCoverage) => [ - for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) - if (pattern.isNotEmpty) Glob(pattern), -]; - /// Flutter CLI class Flutter { /// Determine whether flutter is installed. diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index ac9ae4166..25b584df2 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -18,40 +18,6 @@ enum TestRunType { dart, } -/// How to collect coverage. -enum CoverageCollectionMode { - /// Collect coverage from imported files only (default behavior). - imports, - - /// Collect coverage from all files in the project. - all; - - /// Parses a string value into a [CoverageCollectionMode]. - static CoverageCollectionMode fromString(String value) { - return CoverageCollectionMode.values.firstWhere( - (mode) => mode.name == value, - orElse: () => CoverageCollectionMode.imports, - ); - } -} - -/// {@template coverage_not_met} -/// Thrown when `flutter test ---coverage --min-coverage` -/// does not meet the provided minimum coverage threshold. -/// {@endtemplate} -class MinCoverageNotMet implements Exception { - /// {@macro coverage_not_met} - const new(this.coverage, {this.uncoveredLines}); - - /// The measured coverage percentage (total hits / total found * 100). - final double coverage; - - /// Lines not covered, keyed by file path, values are line numbers. - /// - /// Only populated when `--show-uncovered` is set. - final Map>? uncoveredLines; -} - /// A class to run test command from a CLI command, like `flutter` or `dart`. /// /// It abstracts common functionalities like the test optimization, coverage @@ -204,7 +170,7 @@ This command should be run from the root of your $projectKind project.'''); overrideTestRunner ?? (testType == TestRunType.flutter ? flutterTest : dartTest); - final coverageOptions = _CoverageOptions( + final coverageOptions = CoverageOptions( collect: collectCoverage, collectFrom: collectCoverageFrom, minCoverage: minCoverage, @@ -214,6 +180,17 @@ This command should be run from the root of your $projectKind project.'''); checkIgnore: checkIgnore, ); + CoverageReport coverageReportOf(String packageRoot) => switch (testType) { + TestRunType.flutter => FlutterCoverageReport( + packageRoot: packageRoot, + options: coverageOptions, + ), + TestRunType.dart => DartCoverageReport( + packageRoot: packageRoot, + options: coverageOptions, + ), + }; + return _runCommand( cmd: (cwd) => _testPackage( cwd: cwd, @@ -222,7 +199,7 @@ This command should be run from the root of your $projectKind project.'''); testType: testType, testRunner: testRunner, optimizer: optimizer, - coverageOptions: coverageOptions, + coverageReport: coverageReportOf(cwd), randomSeed: randomSeed, forceAnsi: forceAnsi, arguments: arguments, @@ -243,19 +220,14 @@ This command should be run from the root of your $projectKind project.'''); required TestRunType testType, required VeryGoodTestRunner testRunner, required TestOptimizer optimizer, - required _CoverageOptions coverageOptions, + required CoverageReport coverageReport, required String? randomSeed, required bool? forceAnsi, required List? arguments, required void Function(String)? stdout, required void Function(String)? stderr, }) async { - final lcovPath = p.join(cwd, 'coverage', 'lcov.info'); - final lcovFile = File(lcovPath); - - if (coverageOptions.collect && lcovFile.existsSync()) { - await lcovFile.delete(); - } + await coverageReport.clean(); void noop(String? _) {} final workingDirectory = Directory(p.normalize(cwd)).absolute.path; @@ -285,7 +257,7 @@ This command should be run from the root of your $projectKind project.'''); await optimization.cleanUp(); // The merge step downstream still expects a report from every // shard, so leave an empty one behind. - if (coverageOptions.collect) await lcovFile.create(recursive: true); + await coverageReport.writeEmpty(); return ExitCode.success.code; } @@ -294,11 +266,10 @@ This command should be run from the root of your $projectKind project.'''); () => _testCommand( cwd: cwd, - collectCoverage: coverageOptions.collect, testRunner: testRunner, - testType: testType, optimization: optimization, arguments: [ + ...coverageReport.collectArguments, ...?arguments, if (randomSeed != null) ...[ '--test-randomize-ordering-seed', @@ -310,13 +281,7 @@ This command should be run from the root of your $projectKind project.'''); stderr: stderr ?? noop, ).whenComplete(() async { await optimization.cleanUp(); - await _reportCoverage( - cwd: cwd, - lcovPath: lcovPath, - testType: testType, - options: coverageOptions, - stdout: stdout, - ); + await coverageReport.finalize(stdout: stdout); }), ); } @@ -327,123 +292,6 @@ This command should be run from the root of your $projectKind project.'''); return relativePath == '.' ? '.' : '.${p.context.separator}$relativePath'; } - /// Writes the lcov report of a finished test run when coverage is - /// collected, then enforces the coverage threshold when one is set. - static Future _reportCoverage({ - required String cwd, - required String lcovPath, - required TestRunType testType, - required _CoverageOptions options, - required void Function(String)? stdout, - }) async { - if (options.collect) { - await _writeLcov( - cwd: cwd, - lcovPath: lcovPath, - testType: testType, - options: options, - ); - } - - if (options.minCoverage != null || options.showUncovered) { - await _checkCoverage( - lcovPath: lcovPath, - options: options, - stdout: stdout, - ); - } - } - - /// Leaves the coverage of the test run in `coverage/lcov.info`. - static Future _writeLcov({ - required String cwd, - required String lcovPath, - required TestRunType testType, - required _CoverageOptions options, - }) async { - // Dart doesn't generate lcov files directly, so convert the json - // coverage it writes into lcov. - if (testType == TestRunType.dart) { - await _convertDartCoverageToLcov( - cwd: cwd, - lcovFile: File(lcovPath), - options: options, - ); - } - - assert(File(lcovPath).existsSync(), 'coverage/lcov.info must exist'); - - if (options.collectFrom == CoverageCollectionMode.all) { - await _enhanceLcovWithUntestedFiles( - lcovPath: lcovPath, - cwd: cwd, - reportOn: options.reportOn, - excludeFromCoverage: options.excludeFromCoverage, - ); - } - } - - /// Converts the json coverage `dart test` writes into [lcovFile]. - static Future _convertDartCoverageToLcov({ - required String cwd, - required File lcovFile, - required _CoverageOptions options, - }) async { - final files = _dartCoverageFilesToProcess(p.join(cwd, 'coverage')); - - final resolvedCwd = Directory(cwd).resolveSymbolicLinksSync(); - final resolvedReportOn = [ - for (final path in options.reportOn) p.join(resolvedCwd, path), - ]; - - final hitmap = await coverage.HitMap.parseFiles( - files, - packagePath: resolvedCwd, - checkIgnoredLines: options.checkIgnore, - ); - - final resolver = await coverage.Resolver.create(packagePath: resolvedCwd); - - final output = hitmap.formatLcov( - resolver, - reportOn: resolvedReportOn, - basePath: resolvedCwd, - ); - - await lcovFile.create(recursive: true); - await lcovFile.writeAsString(output); - } - - /// Throws [MinCoverageNotMet] when the coverage in [lcovPath] is below the - /// threshold, and otherwise lists the uncovered lines when asked to. - static Future _checkCoverage({ - required String lcovPath, - required _CoverageOptions options, - required void Function(String)? stdout, - }) async { - final records = await Parser.parse(lcovPath); - final coverageMetrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: options.excludeFromCoverage, - ); - final percentage = coverageMetrics.percentage; - final uncoveredLines = - options.showUncovered && coverageMetrics.uncoveredLines.isNotEmpty - ? coverageMetrics.uncoveredLines - : null; - - final minCoverage = options.minCoverage; - if (minCoverage != null && percentage < minCoverage) { - throw MinCoverageNotMet(percentage, uncoveredLines: uncoveredLines); - } - - // When coverage passes but is below 100%, - // show uncovered lines as informational output. - if (uncoveredLines != null) { - stdout?.call('${formatUncoveredLines(uncoveredLines)}\n'); - } - } - static T _overrideAnsiOutput(bool? enableAnsiOutput, T Function() body) => enableAnsiOutput == null ? body.call() @@ -482,156 +330,6 @@ This command should be run from the root of your $projectKind project.'''); return ExitCode.software.code; } - - /// Formats a map of uncovered lines into a human-readable string. - /// - /// The [uncoveredLines] map is keyed by file path, with values being lists - /// of uncovered line numbers. - /// - /// Example output: - /// ```dart - /// Lines not covered: - /// - lib/src/foo.dart: 10, 20, 30 - /// - lib/src/bar.dart: 5 - /// ``` - static String formatUncoveredLines(Map> uncoveredLines) { - final lines = uncoveredLines.entries.map((entry) { - final sortedLines = [...entry.value]..sort(); - return '\t- ${entry.key}: ${sortedLines.join(', ')}'; - }); - return 'Lines not covered:\n${lines.join('\n')}'; - } - - /// Discovers all Dart files in the specified directories for coverage. - static List _discoverDartFilesForCoverage({ - required String cwd, - required List reportOn, - String? excludeFromCoverage, - }) { - final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); - - return reportOn.expand((dir) { - final reportOnPath = p.join(cwd, dir); - final directory = Directory(reportOnPath); - - if (!directory.existsSync()) return []; - - return directory - .listSync(recursive: true) - .whereType() - .where((file) => file.path.endsWith('.dart')) - .map((file) => p.relative(file.path, from: cwd)) - .whereNot((file) => excludedGlobs.any((glob) => glob.matches(file))); - }).toList(); - } - - /// Enhances an existing lcov file by adding uncovered files with 0% coverage. - static Future _enhanceLcovWithUntestedFiles({ - required String lcovPath, - required String cwd, - required List reportOn, - String? excludeFromCoverage, - }) async { - final lcovFile = File(lcovPath); - - final allDartFiles = _discoverDartFilesForCoverage( - cwd: cwd, - reportOn: reportOn, - excludeFromCoverage: excludeFromCoverage, - ); - - // Parse existing lcov to find covered files - final existingRecords = await Parser.parse(lcovPath); - final coveredFiles = existingRecords.map((r) => r.file).nonNulls.toSet(); - - final uncoveredFiles = allDartFiles.where((file) { - final normalizedFile = p.normalize(file); - return !coveredFiles.any( - (covered) => p.normalize(covered).endsWith(normalizedFile), - ); - }).toList(); - - if (uncoveredFiles.isEmpty) return; - - // Append uncovered files to lcov - final buffer = StringBuffer(await lcovFile.readAsString()); - - for (final file in uncoveredFiles) { - final dartFile = File(p.join(cwd, file)); - if (!dartFile.existsSync()) continue; - buffer.write(_untestedFileRecord(file, await dartFile.readAsLines())); - } - - await lcovFile.writeAsString(buffer.toString()); - } - - /// The lcov record of a [file] no test reached, given its [lines], where - /// every non-trivial line is marked as uncovered. - static String _untestedFileRecord(String file, List lines) { - final uncoveredLineNumbers = [ - for (final (index, line) in lines.indexed) - if (_isCoverableLine(line.trim())) index + 1, - ]; - - final record = StringBuffer()..writeln('SF:${file.replaceAll(r'\', '/')}'); - for (final lineNumber in uncoveredLineNumbers) { - record.writeln('DA:$lineNumber,0'); - } - record - ..writeln('LF:${uncoveredLineNumbers.length}') - ..writeln('LH:0') - ..writeln('end_of_record'); - return record.toString(); - } - - /// Whether a [trimmedLine] of source counts towards coverage. - static bool _isCoverableLine(String trimmedLine) => - trimmedLine.isNotEmpty && - !_nonCoverableLinePrefixes.any(trimmedLine.startsWith); - - static const _nonCoverableLinePrefixes = ['//', 'import', 'export', 'part']; - - static List _dartCoverageFilesToProcess(String absPath) { - return Directory(absPath) - .listSync(recursive: true) - .whereType() - .where((e) => e.path.endsWith('.json')) - .toList(); - } -} - -/// The coverage settings of a [TestCLIRunner.test] run. -class _CoverageOptions { - const new({ - required this.collect, - required this.collectFrom, - required this.minCoverage, - required this.showUncovered, - required this.excludeFromCoverage, - required this.reportOn, - required this.checkIgnore, - }); - - /// Whether to collect coverage into `coverage/lcov.info`. - final bool collect; - - /// Which files the lcov report accounts for. - final CoverageCollectionMode collectFrom; - - /// The minimum coverage percentage the run must reach, if any. - final double? minCoverage; - - /// Whether to list the lines left uncovered. - final bool showUncovered; - - /// Space-separated globs of the files left out of the coverage. - final String? excludeFromCoverage; - - /// The directories, relative to the package, the coverage reports on. - final List reportOn; - - /// Whether to honor the `coverage:ignore` comments. - final bool checkIgnore; } /// The exit code `dart test` and `flutter test` use when no test ran, for @@ -645,10 +343,8 @@ Future _testCommand({ required void Function(String) stdout, required void Function(String) stderr, required VeryGoodTestRunner testRunner, - required TestRunType testType, required TestOptimization optimization, String cwd = '.', - bool collectCoverage = false, List? arguments, }) { final completer = Completer(); @@ -685,10 +381,7 @@ Future _testCommand({ subscription = testRunner( workingDirectory: cwd, - arguments: [ - if (collectCoverage) _coverageArgument(testType), - ...?arguments, - ], + arguments: arguments, runInShell: true, ).listen( (event) { @@ -709,12 +402,6 @@ Future _testCommand({ return completer.future; } -/// The argument that makes [testType] collect coverage. -String _coverageArgument(TestRunType testType) => switch (testType) { - TestRunType.flutter => '--coverage', - TestRunType.dart => '--coverage=coverage', -}; - /// The exit code a test run that ended with [event] reports. int _exitCodeOf(ExitTestEvent event, TestOptimization optimization) { // A shard can end up holding only tests that the given tags diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index 18858cf21..51acfa995 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -7,6 +7,7 @@ import 'package:mason/mason.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:very_good_cli/src/cli/cli.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Dart test command. diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 819949220..4b8cf5e29 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -7,6 +7,7 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:universal_io/io.dart'; import 'package:very_good_cli/src/cli/cli.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Flutter test command. diff --git a/lib/src/coverage/coverage.dart b/lib/src/coverage/coverage.dart new file mode 100644 index 000000000..e4e5d4c2b --- /dev/null +++ b/lib/src/coverage/coverage.dart @@ -0,0 +1,11 @@ +import 'package:collection/collection.dart'; +import 'package:coverage/coverage.dart' as coverage; +import 'package:glob/glob.dart'; +import 'package:lcov_parser/lcov_parser.dart'; +import 'package:meta/meta.dart'; +import 'package:path/path.dart' as p; +import 'package:universal_io/io.dart'; + +part 'coverage_metrics.dart'; +part 'coverage_report.dart'; +part 'untested_files.dart'; diff --git a/lib/src/coverage/coverage_metrics.dart b/lib/src/coverage/coverage_metrics.dart new file mode 100644 index 000000000..f88b5e909 --- /dev/null +++ b/lib/src/coverage/coverage_metrics.dart @@ -0,0 +1,132 @@ +part of 'coverage.dart'; + +/// How to collect coverage. +enum CoverageCollectionMode { + /// Collect coverage from imported files only (default behavior). + imports, + + /// Collect coverage from all files in the project. + all; + + /// Parses a string value into a [CoverageCollectionMode]. + static CoverageCollectionMode fromString(String value) { + return CoverageCollectionMode.values.firstWhere( + (mode) => mode.name == value, + orElse: () => CoverageCollectionMode.imports, + ); + } +} + +/// {@template coverage_not_met} +/// Thrown when `flutter test ---coverage --min-coverage` +/// does not meet the provided minimum coverage threshold. +/// {@endtemplate} +class MinCoverageNotMet implements Exception { + /// {@macro coverage_not_met} + const new(this.coverage, {this.uncoveredLines}); + + /// The measured coverage percentage (total hits / total found * 100). + final double coverage; + + /// Lines not covered, keyed by file path, values are line numbers. + /// + /// Only populated when `--show-uncovered` is set. + final Map>? uncoveredLines; +} + +/// {@template coverage_metrics} +/// Aggregated coverage metrics computed from a list of LCOV records. +/// {@endtemplate} +class CoverageMetrics { + /// {@macro coverage_metrics} + @visibleForTesting + const new({ + this.totalHits = 0, + this.totalFound = 0, + this.uncoveredLines = const {}, + }); + + /// Generate coverage metrics from a list of lcov records. + factory fromLcovRecords(List records, {String? excludeFromCoverage}) { + final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); + return records + .whereNot((record) => _isExcluded(record.file, excludedGlobs)) + .fold( + const CoverageMetrics(), + (metrics, record) => metrics._add(record), + ); + } + + static bool _isExcluded(String? file, List excludedGlobs) => + file != null && excludedGlobs.any((glob) => glob.matches(file)); + + /// Line numbers in [record] that were instrumented but never hit. + static List _uncoveredLineNumbersOf(Record record) => [ + for (final detail in record.lines?.details ?? const []) + if (detail.line case final line? when (detail.hit ?? 1) == 0) line, + ]; + + /// Returns new metrics with the counts and uncovered lines of [record]. + CoverageMetrics _add(Record record) { + final lines = record.lines; + return CoverageMetrics( + totalFound: totalFound + (lines?.found ?? 0), + totalHits: totalHits + (lines?.hit ?? 0), + uncoveredLines: _uncoveredLinesWith(record), + ); + } + + Map> _uncoveredLinesWith(Record record) { + final file = record.file; + if (file == null) return uncoveredLines; + + final newLines = _uncoveredLineNumbersOf(record); + return { + ...uncoveredLines, + if (newLines.isNotEmpty) file: [...?uncoveredLines[file], ...newLines], + }; + } + + /// Total number of lines hit (covered) across all included files. + final int totalHits; + + /// Total number of instrumented lines found across all included files. + final int totalFound; + + /// Lines not covered. + /// Keyed by file path, values are sorted line numbers. + final Map> uncoveredLines; + + /// Coverage percentage: [totalHits] / [totalFound] * 100. + /// + /// Returns `0` when [totalFound] is less than 1. + double get percentage { + return totalFound < 1 ? 0 : (totalHits / totalFound * 100); + } +} + +/// Parses the space-separated glob patterns of an `--exclude-coverage` +/// value, ignoring empty segments. +List _parseExcludeGlobs(String? excludeFromCoverage) => [ + for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) + if (pattern.isNotEmpty) Glob(pattern), +]; + +/// Formats a map of uncovered lines into a human-readable string. +/// +/// The [uncoveredLines] map is keyed by file path, with values being lists +/// of uncovered line numbers. +/// +/// Example output: +/// ```dart +/// Lines not covered: +/// - lib/src/foo.dart: 10, 20, 30 +/// - lib/src/bar.dart: 5 +/// ``` +String formatUncoveredLines(Map> uncoveredLines) { + final lines = uncoveredLines.entries.map((entry) { + final sortedLines = [...entry.value]..sort(); + return '\t- ${entry.key}: ${sortedLines.join(', ')}'; + }); + return 'Lines not covered:\n${lines.join('\n')}'; +} diff --git a/lib/src/coverage/coverage_report.dart b/lib/src/coverage/coverage_report.dart new file mode 100644 index 000000000..1afa6f78f --- /dev/null +++ b/lib/src/coverage/coverage_report.dart @@ -0,0 +1,190 @@ +part of 'coverage.dart'; + +/// {@template coverage_options} +/// The coverage settings shared by every package of a test run. +/// {@endtemplate} +@immutable +class CoverageOptions { + /// {@macro coverage_options} + const new({ + this.collect = false, + this.collectFrom = CoverageCollectionMode.imports, + this.minCoverage, + this.showUncovered = false, + this.excludeFromCoverage, + this.reportOn = const ['lib'], + this.checkIgnore = false, + }); + + /// Whether to collect coverage into `coverage/lcov.info`. + final bool collect; + + /// Which files the lcov report accounts for. + final CoverageCollectionMode collectFrom; + + /// The minimum coverage percentage the run must reach, if any. + final double? minCoverage; + + /// Whether to list the lines left uncovered. + final bool showUncovered; + + /// Space-separated globs of the files left out of the coverage. + final String? excludeFromCoverage; + + /// The directories, relative to the package, the coverage reports on. + final List reportOn; + + /// Whether to honor the `coverage:ignore` comments. + final bool checkIgnore; +} + +/// {@template coverage_report} +/// The coverage of the tests of the package at [packageRoot], kept in +/// `coverage/lcov.info` and checked against the [options]. +/// +/// Each test runner writes coverage differently, so there is one report per +/// runner: [FlutterCoverageReport] and [DartCoverageReport]. +/// {@endtemplate} +sealed class CoverageReport { + /// {@macro coverage_report} + const new({required this.packageRoot, required this.options}); + + /// The root of the package whose tests are covered. + final String packageRoot; + + /// The coverage settings of the test run. + final CoverageOptions options; + + /// The lcov file the report is kept in. + File get lcovFile => File(p.join(packageRoot, 'coverage', 'lcov.info')); + + /// The arguments that make the test runner collect coverage, which are + /// empty when coverage is not collected. + List get collectArguments => [if (options.collect) _collectArgument]; + + String get _collectArgument; + + /// Deletes the report of a previous run, when coverage is collected. + Future clean() async { + if (options.collect && lcovFile.existsSync()) await lcovFile.delete(); + } + + /// Leaves an empty report behind, when coverage is collected, for a run + /// that had no tests to run. + Future writeEmpty() async { + if (options.collect) await lcovFile.create(recursive: true); + } + + /// Writes the report of a finished test run, when coverage is collected, + /// then checks it against [CoverageOptions.minCoverage] and lists the + /// uncovered lines to [stdout] when [CoverageOptions.showUncovered] is set. + /// + /// Throws [MinCoverageNotMet] when the coverage is below the threshold. + Future finalize({void Function(String)? stdout}) async { + if (options.collect) await _write(); + + if (options.minCoverage != null || options.showUncovered) { + await _check(stdout: stdout); + } + } + + Future _write() async { + await _writeLcov(); + + assert(lcovFile.existsSync(), 'coverage/lcov.info must exist'); + + if (options.collectFrom == CoverageCollectionMode.all) { + await _addUntestedFiles( + lcovPath: lcovFile.path, + cwd: packageRoot, + reportOn: options.reportOn, + excludeFromCoverage: options.excludeFromCoverage, + ); + } + } + + /// Leaves the coverage the test runner collected in [lcovFile]. + Future _writeLcov(); + + Future _check({required void Function(String)? stdout}) async { + final records = await Parser.parse(lcovFile.path); + final coverageMetrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: options.excludeFromCoverage, + ); + final percentage = coverageMetrics.percentage; + final uncoveredLines = + options.showUncovered && coverageMetrics.uncoveredLines.isNotEmpty + ? coverageMetrics.uncoveredLines + : null; + + final minCoverage = options.minCoverage; + if (minCoverage != null && percentage < minCoverage) { + throw MinCoverageNotMet(percentage, uncoveredLines: uncoveredLines); + } + + // When coverage passes but is below 100%, + // show uncovered lines as informational output. + if (uncoveredLines != null) { + stdout?.call('${formatUncoveredLines(uncoveredLines)}\n'); + } + } +} + +/// {@template flutter_coverage_report} +/// The [CoverageReport] of a `flutter test` run, which writes the lcov file +/// itself. +/// {@endtemplate} +final class FlutterCoverageReport extends CoverageReport { + /// {@macro flutter_coverage_report} + const new({required super.packageRoot, required super.options}); + + @override + String get _collectArgument => '--coverage'; + + @override + Future _writeLcov() async {} +} + +/// {@template dart_coverage_report} +/// The [CoverageReport] of a `dart test` run, which writes json coverage +/// that the report converts into lcov. +/// {@endtemplate} +final class DartCoverageReport extends CoverageReport { + /// {@macro dart_coverage_report} + const new({required super.packageRoot, required super.options}); + + @override + String get _collectArgument => '--coverage=coverage'; + + @override + Future _writeLcov() async { + final files = Directory(p.join(packageRoot, 'coverage')) + .listSync(recursive: true) + .whereType() + .where((file) => file.path.endsWith('.json')) + .toList(); + + final resolvedRoot = Directory(packageRoot).resolveSymbolicLinksSync(); + final resolvedReportOn = [ + for (final path in options.reportOn) p.join(resolvedRoot, path), + ]; + + final hitmap = await coverage.HitMap.parseFiles( + files, + packagePath: resolvedRoot, + checkIgnoredLines: options.checkIgnore, + ); + + final resolver = await coverage.Resolver.create(packagePath: resolvedRoot); + + final output = hitmap.formatLcov( + resolver, + reportOn: resolvedReportOn, + basePath: resolvedRoot, + ); + + await lcovFile.create(recursive: true); + await lcovFile.writeAsString(output); + } +} diff --git a/lib/src/coverage/untested_files.dart b/lib/src/coverage/untested_files.dart new file mode 100644 index 000000000..a1ad2551e --- /dev/null +++ b/lib/src/coverage/untested_files.dart @@ -0,0 +1,90 @@ +part of 'coverage.dart'; + +/// Discovers all Dart files in the specified directories for coverage. +List _discoverDartFilesForCoverage({ + required String cwd, + required List reportOn, + String? excludeFromCoverage, +}) { + final excludedGlobs = _parseExcludeGlobs(excludeFromCoverage); + + return reportOn.expand((dir) { + final reportOnPath = p.join(cwd, dir); + final directory = Directory(reportOnPath); + + if (!directory.existsSync()) return []; + + return directory + .listSync(recursive: true) + .whereType() + .where((file) => file.path.endsWith('.dart')) + .map((file) => p.relative(file.path, from: cwd)) + .whereNot((file) => excludedGlobs.any((glob) => glob.matches(file))); + }).toList(); +} + +/// Enhances an existing lcov file by adding uncovered files with 0% coverage. +Future _addUntestedFiles({ + required String lcovPath, + required String cwd, + required List reportOn, + String? excludeFromCoverage, +}) async { + final lcovFile = File(lcovPath); + + final allDartFiles = _discoverDartFilesForCoverage( + cwd: cwd, + reportOn: reportOn, + excludeFromCoverage: excludeFromCoverage, + ); + + // Parse existing lcov to find covered files + final existingRecords = await Parser.parse(lcovPath); + final coveredFiles = existingRecords.map((r) => r.file).nonNulls.toSet(); + + final uncoveredFiles = allDartFiles.where((file) { + final normalizedFile = p.normalize(file); + return !coveredFiles.any( + (covered) => p.normalize(covered).endsWith(normalizedFile), + ); + }).toList(); + + if (uncoveredFiles.isEmpty) return; + + // Append uncovered files to lcov + final buffer = StringBuffer(await lcovFile.readAsString()); + + for (final file in uncoveredFiles) { + final dartFile = File(p.join(cwd, file)); + if (!dartFile.existsSync()) continue; + buffer.write(_untestedFileRecord(file, await dartFile.readAsLines())); + } + + await lcovFile.writeAsString(buffer.toString()); +} + +/// The lcov record of a [file] no test reached, given its [lines], where +/// every non-trivial line is marked as uncovered. +String _untestedFileRecord(String file, List lines) { + final uncoveredLineNumbers = [ + for (final (index, line) in lines.indexed) + if (_isCoverableLine(line.trim())) index + 1, + ]; + + final record = StringBuffer()..writeln('SF:${file.replaceAll(r'\', '/')}'); + for (final lineNumber in uncoveredLineNumbers) { + record.writeln('DA:$lineNumber,0'); + } + record + ..writeln('LF:${uncoveredLineNumbers.length}') + ..writeln('LH:0') + ..writeln('end_of_record'); + return record.toString(); +} + +/// Whether a [trimmedLine] of source counts towards coverage. +bool _isCoverableLine(String trimmedLine) => + trimmedLine.isNotEmpty && + !_nonCoverableLinePrefixes.any(trimmedLine.startsWith); + +const _nonCoverableLinePrefixes = ['//', 'import', 'export', 'part']; diff --git a/test/src/cli/flutter_cli_test.dart b/test/src/cli/flutter_cli_test.dart index f4427b053..9ddd1ca7b 100644 --- a/test/src/cli/flutter_cli_test.dart +++ b/test/src/cli/flutter_cli_test.dart @@ -2,7 +2,6 @@ import 'dart:async'; -import 'package:lcov_parser/lcov_parser.dart'; import 'package:mason/mason.dart'; import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; @@ -317,239 +316,4 @@ void main() { }); }); }); - - group(CoverageMetrics, () { - List parseRecords(List lines) => Parser.parseLines(lines); - - group('.fromLcovRecords', () { - test('returns empty metrics for an empty record list', () { - final metrics = CoverageMetrics.fromLcovRecords([]); - - expect(metrics.totalHits, equals(0)); - expect(metrics.totalFound, equals(0)); - expect(metrics.uncoveredLines, isEmpty); - }); - - test('aggregates hits and found across records', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:10', - 'LH:8', - 'end_of_record', - 'SF:lib/b.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect(metrics.totalFound, equals(15)); - expect(metrics.totalHits, equals(13)); - }); - - test('collects uncovered lines per file', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'DA:1,1', - 'DA:2,0', - 'DA:3,0', - 'LF:3', - 'LH:1', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect( - metrics.uncoveredLines, - equals({ - 'lib/a.dart': [2, 3], - }), - ); - }); - - test('accumulates uncovered lines across multiple records', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'DA:10,0', - 'DA:20,1', - 'LF:2', - 'LH:1', - 'end_of_record', - 'SF:lib/b.dart', - 'DA:5,0', - 'DA:6,0', - 'LF:2', - 'LH:0', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect( - metrics.uncoveredLines, - equals({ - 'lib/a.dart': [10], - 'lib/b.dart': [5, 6], - }), - ); - }); - - test('counts records without a source file but skips their lines', () { - final records = parseRecords([ - 'DA:1,0', - 'LF:1', - 'LH:0', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect(records.single.file, isNull); - expect(metrics.totalFound, equals(1)); - expect(metrics.uncoveredLines, isEmpty); - }); - - test('handles records with no DA entries', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:0', - 'LH:0', - 'end_of_record', - 'SF:lib/b.dart', - 'LF:4', - 'LH:4', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect(metrics.totalFound, equals(4)); - expect(metrics.totalHits, equals(4)); - expect(metrics.uncoveredLines, isEmpty); - }); - - group('excludeFromCoverage', () { - test('handles null', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:3', - 'LH:3', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords(records); - - expect(metrics.totalFound, equals(3)); - expect(metrics.totalHits, equals(3)); - }); - - test('handles empty string', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:3', - 'LH:3', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: '', - ); - - expect(metrics.totalFound, equals(3)); - expect(metrics.totalHits, equals(3)); - }); - - test('excludes a single glob-matched file', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:10', - 'LH:8', - 'end_of_record', - 'SF:lib/generated/b.g.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: 'lib/generated/**', - ); - - expect(metrics.totalFound, equals(10)); - expect(metrics.totalHits, equals(8)); - }); - - test('excludes multiple space-separated globs', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:10', - 'LH:8', - 'end_of_record', - 'SF:lib/generated/b.g.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - 'SF:lib/mocks/mock_c.dart', - 'LF:4', - 'LH:4', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: 'lib/generated/** lib/mocks/**', - ); - - expect(metrics.totalFound, equals(10)); - expect(metrics.totalHits, equals(8)); - }); - - test('handles multiple consecutive spaces between globs', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:10', - 'LH:8', - 'end_of_record', - 'SF:lib/generated/b.g.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - 'SF:lib/mocks/mock_c.dart', - 'LF:4', - 'LH:4', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: 'lib/generated/** lib/mocks/**', - ); - - expect(metrics.totalFound, equals(10)); - expect(metrics.totalHits, equals(8)); - }); - - test('does not exclude file when glob does not match', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: 'lib/generated/**', - ); - - expect(metrics.totalFound, equals(5)); - expect(metrics.totalHits, equals(5)); - }); - }); - }); - }); } diff --git a/test/src/cli/test_cli_runner_test.dart b/test/src/cli/test_cli_runner_test.dart index 4ed5a2879..5521abc27 100644 --- a/test/src/cli/test_cli_runner_test.dart +++ b/test/src/cli/test_cli_runner_test.dart @@ -10,6 +10,7 @@ import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart' as stack_trace; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_test_runner/very_good_test_runner.dart'; import '../../fixtures/fixtures.dart'; diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 81decfc3e..15a2ceaa7 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -11,6 +11,7 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; import 'package:very_good_cli/src/commands/dart/commands/commands.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; import '../../../../helpers/helpers.dart'; diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index bad9066e6..215338767 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -11,6 +11,7 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; import 'package:very_good_cli/src/commands/test/test.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; import '../../../helpers/helpers.dart'; diff --git a/test/src/coverage/coverage_metrics_test.dart b/test/src/coverage/coverage_metrics_test.dart new file mode 100644 index 000000000..a9ff70b55 --- /dev/null +++ b/test/src/coverage/coverage_metrics_test.dart @@ -0,0 +1,254 @@ +import 'package:lcov_parser/lcov_parser.dart'; +import 'package:test/test.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; + +void main() { + group(CoverageMetrics, () { + List parseRecords(List lines) => Parser.parseLines(lines); + + group('.fromLcovRecords', () { + test('returns empty metrics for an empty record list', () { + final metrics = CoverageMetrics.fromLcovRecords([]); + + expect(metrics.totalHits, equals(0)); + expect(metrics.totalFound, equals(0)); + expect(metrics.uncoveredLines, isEmpty); + }); + + test('aggregates hits and found across records', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:10', + 'LH:8', + 'end_of_record', + 'SF:lib/b.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect(metrics.totalFound, equals(15)); + expect(metrics.totalHits, equals(13)); + }); + + test('collects uncovered lines per file', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'DA:1,1', + 'DA:2,0', + 'DA:3,0', + 'LF:3', + 'LH:1', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect( + metrics.uncoveredLines, + equals({ + 'lib/a.dart': [2, 3], + }), + ); + }); + + test('accumulates uncovered lines across multiple records', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'DA:10,0', + 'DA:20,1', + 'LF:2', + 'LH:1', + 'end_of_record', + 'SF:lib/b.dart', + 'DA:5,0', + 'DA:6,0', + 'LF:2', + 'LH:0', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect( + metrics.uncoveredLines, + equals({ + 'lib/a.dart': [10], + 'lib/b.dart': [5, 6], + }), + ); + }); + + test('counts records without a source file but skips their lines', () { + final records = parseRecords([ + 'DA:1,0', + 'LF:1', + 'LH:0', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect(records.single.file, isNull); + expect(metrics.totalFound, equals(1)); + expect(metrics.uncoveredLines, isEmpty); + }); + + test('handles records with no DA entries', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:0', + 'LH:0', + 'end_of_record', + 'SF:lib/b.dart', + 'LF:4', + 'LH:4', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect(metrics.totalFound, equals(4)); + expect(metrics.totalHits, equals(4)); + expect(metrics.uncoveredLines, isEmpty); + }); + + group('excludeFromCoverage', () { + test('handles null', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:3', + 'LH:3', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords(records); + + expect(metrics.totalFound, equals(3)); + expect(metrics.totalHits, equals(3)); + }); + + test('handles empty string', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:3', + 'LH:3', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: '', + ); + + expect(metrics.totalFound, equals(3)); + expect(metrics.totalHits, equals(3)); + }); + + test('excludes a single glob-matched file', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:10', + 'LH:8', + 'end_of_record', + 'SF:lib/generated/b.g.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: 'lib/generated/**', + ); + + expect(metrics.totalFound, equals(10)); + expect(metrics.totalHits, equals(8)); + }); + + test('excludes multiple space-separated globs', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:10', + 'LH:8', + 'end_of_record', + 'SF:lib/generated/b.g.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + 'SF:lib/mocks/mock_c.dart', + 'LF:4', + 'LH:4', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: 'lib/generated/** lib/mocks/**', + ); + + expect(metrics.totalFound, equals(10)); + expect(metrics.totalHits, equals(8)); + }); + + test('handles multiple consecutive spaces between globs', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:10', + 'LH:8', + 'end_of_record', + 'SF:lib/generated/b.g.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + 'SF:lib/mocks/mock_c.dart', + 'LF:4', + 'LH:4', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: 'lib/generated/** lib/mocks/**', + ); + + expect(metrics.totalFound, equals(10)); + expect(metrics.totalHits, equals(8)); + }); + + test('does not exclude file when glob does not match', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: 'lib/generated/**', + ); + + expect(metrics.totalFound, equals(5)); + expect(metrics.totalHits, equals(5)); + }); + }); + }); + }); + + group('formatUncoveredLines', () { + test('lists the sorted uncovered lines of every file', () { + expect( + formatUncoveredLines({ + 'lib/a.dart': [30, 10, 20], + 'lib/b.dart': [5], + }), + equals( + 'Lines not covered:\n\t- lib/a.dart: 10, 20, 30\n\t- lib/b.dart: 5', + ), + ); + }); + }); +} diff --git a/test/src/coverage/coverage_report_test.dart b/test/src/coverage/coverage_report_test.dart new file mode 100644 index 000000000..ddd1d679b --- /dev/null +++ b/test/src/coverage/coverage_report_test.dart @@ -0,0 +1,279 @@ +import 'dart:convert'; + +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:universal_io/io.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; + +void main() { + group(CoverageOptions, () { + test('has defaults that collect nothing', () { + const options = CoverageOptions(); + + expect(options.collect, isFalse); + expect(options.collectFrom, equals(CoverageCollectionMode.imports)); + expect(options.minCoverage, isNull); + expect(options.showUncovered, isFalse); + expect(options.excludeFromCoverage, isNull); + expect(options.reportOn, equals(['lib'])); + expect(options.checkIgnore, isFalse); + }); + }); + + group(CoverageReport, () { + late Directory packageRoot; + late File lcovFile; + late List stdoutLogs; + + setUp(() { + packageRoot = Directory.systemTemp.createTempSync('coverage_report_'); + addTearDown(() => packageRoot.deleteSync(recursive: true)); + lcovFile = File(p.join(packageRoot.path, 'coverage', 'lcov.info')); + stdoutLogs = []; + }); + + void writeLcov(String content) => lcovFile + ..createSync(recursive: true) + ..writeAsStringSync(content); + + void writeSource(String path, String content) => + File(p.join(packageRoot.path, path)) + ..createSync(recursive: true) + ..writeAsStringSync(content); + + FlutterCoverageReport flutterReport(CoverageOptions options) => + FlutterCoverageReport(packageRoot: packageRoot.path, options: options); + + test('keeps its lcov file under coverage/lcov.info', () { + final report = flutterReport(const CoverageOptions()); + + expect(report.lcovFile.path, equals(lcovFile.path)); + }); + + group('collectArguments', () { + test('are empty when coverage is not collected', () { + expect( + flutterReport(const CoverageOptions()).collectArguments, + isEmpty, + ); + expect( + DartCoverageReport( + packageRoot: packageRoot.path, + options: const CoverageOptions(), + ).collectArguments, + isEmpty, + ); + }); + + test('make flutter test collect coverage', () { + expect( + flutterReport(const CoverageOptions(collect: true)).collectArguments, + equals(['--coverage']), + ); + }); + + test('make dart test collect coverage into the coverage folder', () { + expect( + DartCoverageReport( + packageRoot: packageRoot.path, + options: const CoverageOptions(collect: true), + ).collectArguments, + equals(['--coverage=coverage']), + ); + }); + }); + + group('clean', () { + test('deletes the report of a previous run', () async { + writeLcov('SF:lib/a.dart\nend_of_record\n'); + + await flutterReport(const CoverageOptions(collect: true)).clean(); + + expect(lcovFile.existsSync(), isFalse); + }); + + test('completes when there is no previous report', () async { + await flutterReport(const CoverageOptions(collect: true)).clean(); + + expect(lcovFile.existsSync(), isFalse); + }); + + test('keeps the report when coverage is not collected', () async { + writeLcov('SF:lib/a.dart\nend_of_record\n'); + + await flutterReport(const CoverageOptions()).clean(); + + expect(lcovFile.existsSync(), isTrue); + }); + }); + + group('writeEmpty', () { + test('leaves an empty report behind', () async { + await flutterReport(const CoverageOptions(collect: true)).writeEmpty(); + + expect(lcovFile.readAsStringSync(), isEmpty); + }); + + test('writes nothing when coverage is not collected', () async { + await flutterReport(const CoverageOptions()).writeEmpty(); + + expect(lcovFile.existsSync(), isFalse); + }); + }); + + group('finalize', () { + const coveredLcov = + 'SF:lib/a.dart\n' + 'DA:1,1\n' + 'DA:2,0\n' + 'LF:2\n' + 'LH:1\n' + 'end_of_record\n'; + + test('keeps the lcov file flutter test wrote', () async { + writeLcov(coveredLcov); + + await flutterReport(const CoverageOptions(collect: true)) + .finalize(stdout: stdoutLogs.add); + + expect(lcovFile.readAsStringSync(), equals(coveredLcov)); + expect(stdoutLogs, isEmpty); + }); + + test('adds the untested files when collecting from all files', () async { + writeLcov(coveredLcov); + writeSource('lib/a.dart', 'void a() {}\n'); + writeSource( + 'lib/b.dart', + "import 'a.dart';\n" + '\n' + '// A comment.\n' + 'void b() {}\n', + ); + writeSource('lib/b.g.dart', 'void generated() {}\n'); + + await flutterReport( + const CoverageOptions( + collect: true, + collectFrom: CoverageCollectionMode.all, + excludeFromCoverage: '**/*.g.dart', + ), + ).finalize(); + + expect( + lcovFile.readAsStringSync(), + equals( + '${coveredLcov}SF:lib/b.dart\n' + 'DA:4,0\n' + 'LF:1\n' + 'LH:0\n' + 'end_of_record\n', + ), + ); + }); + + test('throws $MinCoverageNotMet when below the threshold', () async { + writeLcov(coveredLcov); + + await expectLater( + flutterReport( + const CoverageOptions( + collect: true, + minCoverage: 100, + showUncovered: true, + ), + ).finalize(stdout: stdoutLogs.add), + throwsA( + isA() + .having((error) => error.coverage, 'coverage', equals(50)) + .having( + (error) => error.uncoveredLines, + 'uncoveredLines', + equals({ + 'lib/a.dart': [2], + }), + ), + ), + ); + expect(stdoutLogs, isEmpty); + }); + + test('leaves the uncovered lines out unless asked to', () async { + writeLcov(coveredLcov); + + await expectLater( + flutterReport(const CoverageOptions(minCoverage: 100)).finalize(), + throwsA( + isA().having( + (error) => error.uncoveredLines, + 'uncoveredLines', + isNull, + ), + ), + ); + }); + + test('lists the uncovered lines when the threshold is met', () async { + writeLcov(coveredLcov); + + await flutterReport( + const CoverageOptions(minCoverage: 50, showUncovered: true), + ).finalize(stdout: stdoutLogs.add); + + expect(stdoutLogs, equals(['Lines not covered:\n\t- lib/a.dart: 2\n'])); + }); + + test('lists nothing when every line is covered', () async { + writeLcov('SF:lib/a.dart\nDA:1,1\nLF:1\nLH:1\nend_of_record\n'); + + await flutterReport(const CoverageOptions(showUncovered: true)) + .finalize(stdout: stdoutLogs.add); + + expect(stdoutLogs, isEmpty); + }); + + test('converts the json coverage of dart test into lcov', () async { + final source = File(p.join(packageRoot.path, 'lib', 'a.dart')) + ..createSync(recursive: true) + ..writeAsStringSync('void a() {}\nvoid b() {}\n'); + File(p.join(packageRoot.path, 'coverage', 'test', 'a_test.json')) + ..createSync(recursive: true) + ..writeAsStringSync( + jsonEncode({ + 'type': 'CodeCoverage', + 'coverage': [ + { + 'source': source.uri.toString(), + 'script': { + 'type': '@Script', + 'fixedId': true, + 'id': 'libraries/1/scripts/a', + 'uri': source.uri.toString(), + '_kind': 'library', + }, + 'hits': [1, 1, 2, 0], + }, + ], + }), + ); + + await DartCoverageReport( + packageRoot: packageRoot.path, + options: const CoverageOptions(collect: true), + ).finalize(); + + expect( + lcovFile.readAsStringSync(), + equals( + 'SF:lib/a.dart\n' + 'DA:1,1\n' + 'DA:2,0\n' + 'LF:2\n' + 'LH:1\n' + 'end_of_record\n', + ), + ); + }); + }); + }); +} From a506de1149cff55dd612db59c646ab553449f12e Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 13:27:54 +0300 Subject: [PATCH 6/8] refactor(test): move the test event reporter into its own file _TestEventReporter is a self-contained unit that turns test events into terminal output. Give it its own part of the cli library so test_cli_runner.dart only orchestrates the run. --- lib/src/cli/cli.dart | 1 + lib/src/cli/test_cli_runner.dart | 180 -------------------------- lib/src/cli/test_event_reporter.dart | 181 +++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 180 deletions(-) create mode 100644 lib/src/cli/test_event_reporter.dart diff --git a/lib/src/cli/cli.dart b/lib/src/cli/cli.dart index edc35778d..0c0f78024 100644 --- a/lib/src/cli/cli.dart +++ b/lib/src/cli/cli.dart @@ -18,6 +18,7 @@ part 'dart_cli.dart'; part 'flutter_cli.dart'; part 'git_cli.dart'; part 'test_cli_runner.dart'; +part 'test_event_reporter.dart'; const R Function( R Function(), { diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 25b584df2..8833d33b4 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -413,183 +413,3 @@ int _exitCodeOf(ExitTestEvent event, TestOptimization optimization) { ? ExitCode.success.code : ExitCode.unavailable.code; } - -/// Prints the progress of a test run as its [TestEvent]s arrive, and keeps -/// the tally of passing, skipped and failing tests the summary is made of. -class _TestEventReporter { - new({ - required this.stdout, - required this.stderr, - required this.optimization, - required this.cwd, - }); - - final void Function(String) stdout; - final void Function(String) stderr; - final TestOptimization optimization; - final String cwd; - - final _suites = {}; - final _groups = {}; - final _tests = {}; - final _failedTestErrorMessages = >{}; - - var _successCount = 0; - var _skipCount = 0; - - void report(TestEvent event) { - switch (event) { - case SuiteTestEvent(:final suite): - _suites[suite.id] = suite; - case GroupTestEvent(:final group): - _groups[group.id] = group; - case TestStartEvent(:final test): - _tests[test.id] = test; - case MessageTestEvent(): - _reportMessage(event); - case ErrorTestEvent(): - _reportError(event); - case TestDoneEvent(): - _reportTestDone(event); - case DoneTestEvent(): - _reportDone(event); - } - } - - void _reportMessage(MessageTestEvent event) { - final message = event.message; - if (message.startsWith('Skip:')) { - stdout('$_clearLine${lightYellow.wrap(message)}\n'); - } else if (message.contains('EXCEPTION')) { - stderr('$_clearLine$message'); - } else { - stdout('$_clearLine$message\n'); - } - } - - void _reportError(ErrorTestEvent event) { - stderr('$_clearLine${event.error}'); - - if (event.stackTrace.trim().isNotEmpty) { - stderr('$_clearLine${event.stackTrace}'); - } - - final report = _resolveReport(event.testID); - final prefix = event.isFailure ? '[FAILED]' : '[ERROR]'; - - final relativeTestPath = p.relative(report.path, from: cwd); - _failedTestErrorMessages[relativeTestPath] = [ - ...?_failedTestErrorMessages[relativeTestPath], - '$prefix ${report.name}', - ]; - } - - void _reportTestDone(TestDoneEvent event) { - if (event.hidden) return; - - final (:path, :name) = _resolveReport(event.testID); - _tallyResult(event, testPath: path, testName: name); - - final timeElapsed = Duration(milliseconds: event.time).formatted(); - final stats = _stats(); - final truncatedTestName = name.toSingleLine().truncated( - _lineLength - (timeElapsed.length + stats.length + 2), - ); - stdout('''$_clearLine$timeElapsed $stats: $truncatedTestName'''); - } - - void _tallyResult( - TestDoneEvent event, { - required String testPath, - required String testName, - }) { - if (event.skipped) { - stdout( - '''$_clearLine${lightYellow.wrap('$testName $testPath (SKIPPED)')}\n''', - ); - _skipCount++; - } else if (event.result == TestResult.success) { - _successCount++; - } else { - stderr('$_clearLine$testName $testPath (FAILED)'); - } - } - - void _reportDone(DoneTestEvent event) { - final timeElapsed = Duration(milliseconds: event.time).formatted(); - final stats = _stats(); - final success = event.success ?? false; - final summary = success - ? lightGreen.wrap('All tests passed!')! - : lightRed.wrap('Some tests failed.')!; - - stdout('$_clearLine${darkGray.wrap(timeElapsed)} $stats: $summary\n'); - - if (success) return; - - assert( - _failedTestErrorMessages.isNotEmpty, - 'Invalid state: test event report as failed ' - 'but no failed tests were gathered', - ); - stderr(_failingTestsSummary()); - } - - String _failingTestsSummary() { - final title = styleBold.wrap('Failing Tests:'); - - final lines = StringBuffer('$_clearLine$title\n'); - for (final MapEntry(key: testPath, value: errorMessages) - in _failedTestErrorMessages.entries) { - lines.writeln('$_clearLine - $testPath '); - - for (final errorMessage in errorMessages) { - lines.writeln('$_clearLine \t- $errorMessage'); - } - } - - return lines.toString(); - } - - /// The file and name [testID] is reported under, once the bundling of the - /// test optimizer is undone. - ({String path, String name}) _resolveReport(int testID) { - final test = _tests[testID]!; - final suite = _suites[test.suiteID]!; - - return optimization.resolveReport( - suitePath: suite.path!, - testName: test.name, - groupName: _topGroupName(test, _groups), - ); - } - - String _stats() { - final passingTests = _successCount.formatSuccess(); - final failingTests = _failedTestErrorMessages.values - .expand((e) => e) - .length - .formatFailure(); - final skippedTests = _skipCount.formatSkipped(); - final result = [passingTests, failingTests, skippedTests] - ..removeWhere((element) => element.isEmpty); - return result.join(' '); - } -} - -/// The name of the outermost non-empty group [test] belongs to. -/// -/// For a test running inside the optimized bundle this is the path of the file -/// it was written in, relative to `test`, which is what -/// [TestOptimization.resolveReport] needs to undo the bundling. -String? _topGroupName(Test test, Map groups) => test.groupIDs - .map((groupID) => groups[groupID]?.name) - .firstWhereOrNull((groupName) => groupName?.isNotEmpty ?? false); - -final int _lineLength = () { - try { - return stdout.terminalColumns; - } on StdoutException { - return 80; - } -}(); diff --git a/lib/src/cli/test_event_reporter.dart b/lib/src/cli/test_event_reporter.dart new file mode 100644 index 000000000..8f58912b9 --- /dev/null +++ b/lib/src/cli/test_event_reporter.dart @@ -0,0 +1,181 @@ +part of 'cli.dart'; + +/// Prints the progress of a test run as its [TestEvent]s arrive, and keeps +/// the tally of passing, skipped and failing tests the summary is made of. +class _TestEventReporter { + new({ + required this.stdout, + required this.stderr, + required this.optimization, + required this.cwd, + }); + + final void Function(String) stdout; + final void Function(String) stderr; + final TestOptimization optimization; + final String cwd; + + final _suites = {}; + final _groups = {}; + final _tests = {}; + final _failedTestErrorMessages = >{}; + + var _successCount = 0; + var _skipCount = 0; + + void report(TestEvent event) { + switch (event) { + case SuiteTestEvent(:final suite): + _suites[suite.id] = suite; + case GroupTestEvent(:final group): + _groups[group.id] = group; + case TestStartEvent(:final test): + _tests[test.id] = test; + case MessageTestEvent(): + _reportMessage(event); + case ErrorTestEvent(): + _reportError(event); + case TestDoneEvent(): + _reportTestDone(event); + case DoneTestEvent(): + _reportDone(event); + } + } + + void _reportMessage(MessageTestEvent event) { + final message = event.message; + if (message.startsWith('Skip:')) { + stdout('$_clearLine${lightYellow.wrap(message)}\n'); + } else if (message.contains('EXCEPTION')) { + stderr('$_clearLine$message'); + } else { + stdout('$_clearLine$message\n'); + } + } + + void _reportError(ErrorTestEvent event) { + stderr('$_clearLine${event.error}'); + + if (event.stackTrace.trim().isNotEmpty) { + stderr('$_clearLine${event.stackTrace}'); + } + + final report = _resolveReport(event.testID); + final prefix = event.isFailure ? '[FAILED]' : '[ERROR]'; + + final relativeTestPath = p.relative(report.path, from: cwd); + _failedTestErrorMessages[relativeTestPath] = [ + ...?_failedTestErrorMessages[relativeTestPath], + '$prefix ${report.name}', + ]; + } + + void _reportTestDone(TestDoneEvent event) { + if (event.hidden) return; + + final (:path, :name) = _resolveReport(event.testID); + _tallyResult(event, testPath: path, testName: name); + + final timeElapsed = Duration(milliseconds: event.time).formatted(); + final stats = _stats(); + final truncatedTestName = name.toSingleLine().truncated( + _lineLength - (timeElapsed.length + stats.length + 2), + ); + stdout('''$_clearLine$timeElapsed $stats: $truncatedTestName'''); + } + + void _tallyResult( + TestDoneEvent event, { + required String testPath, + required String testName, + }) { + if (event.skipped) { + stdout( + '''$_clearLine${lightYellow.wrap('$testName $testPath (SKIPPED)')}\n''', + ); + _skipCount++; + } else if (event.result == TestResult.success) { + _successCount++; + } else { + stderr('$_clearLine$testName $testPath (FAILED)'); + } + } + + void _reportDone(DoneTestEvent event) { + final timeElapsed = Duration(milliseconds: event.time).formatted(); + final stats = _stats(); + final success = event.success ?? false; + final summary = success + ? lightGreen.wrap('All tests passed!')! + : lightRed.wrap('Some tests failed.')!; + + stdout('$_clearLine${darkGray.wrap(timeElapsed)} $stats: $summary\n'); + + if (success) return; + + assert( + _failedTestErrorMessages.isNotEmpty, + 'Invalid state: test event report as failed ' + 'but no failed tests were gathered', + ); + stderr(_failingTestsSummary()); + } + + String _failingTestsSummary() { + final title = styleBold.wrap('Failing Tests:'); + + final lines = StringBuffer('$_clearLine$title\n'); + for (final MapEntry(key: testPath, value: errorMessages) + in _failedTestErrorMessages.entries) { + lines.writeln('$_clearLine - $testPath '); + + for (final errorMessage in errorMessages) { + lines.writeln('$_clearLine \t- $errorMessage'); + } + } + + return lines.toString(); + } + + /// The file and name [testID] is reported under, once the bundling of the + /// test optimizer is undone. + ({String path, String name}) _resolveReport(int testID) { + final test = _tests[testID]!; + final suite = _suites[test.suiteID]!; + + return optimization.resolveReport( + suitePath: suite.path!, + testName: test.name, + groupName: _topGroupName(test, _groups), + ); + } + + String _stats() { + final passingTests = _successCount.formatSuccess(); + final failingTests = _failedTestErrorMessages.values + .expand((e) => e) + .length + .formatFailure(); + final skippedTests = _skipCount.formatSkipped(); + final result = [passingTests, failingTests, skippedTests] + ..removeWhere((element) => element.isEmpty); + return result.join(' '); + } +} + +/// The name of the outermost non-empty group [test] belongs to. +/// +/// For a test running inside the optimized bundle this is the path of the file +/// it was written in, relative to `test`, which is what +/// [TestOptimization.resolveReport] needs to undo the bundling. +String? _topGroupName(Test test, Map groups) => test.groupIDs + .map((groupID) => groups[groupID]?.name) + .firstWhereOrNull((groupName) => groupName?.isNotEmpty ?? false); + +final int _lineLength = () { + try { + return stdout.terminalColumns; + } on StdoutException { + return 80; + } +}(); From cac5f57122708d58d885b6eeedc4b8172436ea0e Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 16:51:57 +0300 Subject: [PATCH 7/8] docs: describe exclude-coverage as space-separated globs --exclude-coverage has always accepted several space-separated globs, and the untested-files step now honors them too. The test and dart test help, the MCP test tool, the option dartdocs and the configuration docs still described it as a single glob, unlike site/docs/commands/test.md. --- lib/src/commands/dart/commands/dart_test_command.dart | 8 +++++--- lib/src/commands/test/test.dart | 8 +++++--- lib/src/mcp/mcp_server.dart | 2 +- lib/src/very_good_config/very_good_config.dart | 6 ++++-- site/docs/configuration.md | 4 ++-- test/src/commands/dart/commands/dart_test_test.dart | 2 +- test/src/commands/test/test_test.dart | 2 +- 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index 51acfa995..d238aaa32 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -160,7 +160,8 @@ class DartTestOptions { /// Run only tests associated with the specified tags. final String? tags; - /// A glob which will be used to exclude files that match from the coverage. + /// One or more space-separated globs which will be used to exclude files that + /// match from the coverage. final String? excludeFromCoverage; /// How to collect coverage. @@ -322,8 +323,9 @@ class DartTestCommand extends Command { ..addOption( 'exclude-coverage', help: - 'A glob which will be used to exclude files that match from the ' - "coverage (e.g. '**/*.g.dart').", + 'One or more space-separated globs which will be used to exclude ' + 'files that match from the coverage ' + "(e.g. '**/*.g.dart **/*.freezed.dart').", ) ..addOption( 'exclude-tags', diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 4b8cf5e29..2229c2df8 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -186,7 +186,8 @@ class FlutterTestOptions { /// Run only tests associated with the specified tags. final String? tags; - /// A glob which will be used to exclude files that match from the coverage. + /// One or more space-separated globs which will be used to exclude files that + /// match from the coverage. final String? excludeFromCoverage; /// How to collect coverage. @@ -375,8 +376,9 @@ class TestCommand extends Command { ..addOption( 'exclude-coverage', help: - 'A glob which will be used to exclude files that match from the ' - "coverage (e.g. '**/*.g.dart').", + 'One or more space-separated globs which will be used to exclude ' + 'files that match from the coverage ' + "(e.g. '**/*.g.dart **/*.freezed.dart').", ) ..addOption( 'exclude-tags', diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index e37fbaf21..de840d410 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -216,7 +216,7 @@ Automatically set to 1 when --platform is specified. '''Run only tests associated with the specified tags.''', ), 'exclude_coverage': StringSchema( - description: '''A glob which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart').''', + description: '''One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').''', ), 'exclude_tags': StringSchema( description: diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 8056e1492..8e75a171c 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -313,7 +313,8 @@ class VeryGoodTestConfig extends Equatable { /// Run only tests associated with the specified tags. final String? tags; - /// A glob which will be used to exclude files that match from the coverage. + /// One or more space-separated globs which will be used to exclude files that + /// match from the coverage. final String? excludeCoverage; /// Run only tests that do not have the specified tags. @@ -470,7 +471,8 @@ class VeryGoodDartTestConfig extends Equatable { /// Run only tests associated with the specified tags. final String? tags; - /// A glob which will be used to exclude files that match from the coverage. + /// One or more space-separated globs which will be used to exclude files that + /// match from the coverage. final String? excludeCoverage; /// Run only tests that do not have the specified tags. diff --git a/site/docs/configuration.md b/site/docs/configuration.md index 9105e5167..fdc5fbaeb 100644 --- a/site/docs/configuration.md +++ b/site/docs/configuration.md @@ -92,7 +92,7 @@ test: | `optimization` | `bool` \| `map` | Whether to apply optimizations for test performance. See [`optimization`](#optimization). | | `concurrency` | `int` | Positive integer. The number of concurrent test suites run. | | `tags` | `string` | Run only tests associated with the specified tags. | -| `exclude_coverage` | `string` | A glob that excludes matching files from coverage. | +| `exclude_coverage` | `string` | Space-separated globs that exclude matching files from coverage. | | `exclude_tags` | `string` | Run only tests that do not have the specified tags. | | `min_coverage` | `number` | Between `0` and `100`. Enforces a minimum coverage percentage. | | `show_uncovered` | `bool` | Whether to show uncovered lines when coverage is below 100%. | @@ -201,7 +201,7 @@ dart: | `optimization` | `bool` \| `map` | Whether to apply optimizations for test performance. See [`optimization`](#optimization). | | `concurrency` | `int` | Positive integer. The number of concurrent test suites run. | | `tags` | `string` | Run only tests associated with the specified tags. | -| `exclude_coverage` | `string` | A glob that excludes matching files from coverage. | +| `exclude_coverage` | `string` | Space-separated globs that exclude matching files from coverage. | | `exclude_tags` | `string` | Run only tests that do not have the specified tags. | | `min_coverage` | `number` | Between `0` and `100`. Enforces a minimum coverage percentage. | | `show_uncovered` | `bool` | Whether to show uncovered lines when coverage is below 100%. | diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 15a2ceaa7..143507807 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -37,7 +37,7 @@ const expectedTestUsage = [ '-j, --concurrency The number of concurrent test suites run. Automatically set to 1 when --platform is specified.\n' ' (defaults to "4")\n' '-t, --tags Run only tests associated with the specified tags.\n' - " --exclude-coverage A glob which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart').\n" + " --exclude-coverage One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" '-x, --exclude-tags Run only tests that do not have the specified tags.\n' ' --min-coverage Whether to enforce a minimum coverage percentage. Implicitly enables coverage collection when used alone.\n' ' --show-uncovered Whether to show uncovered lines when coverage is below 100%. Requires --coverage or --min-coverage to be set, or implicitly enables coverage collection when used alone.\n' diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 215338767..8ef249987 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -37,7 +37,7 @@ const expectedTestUsage = [ '-j, --concurrency The number of concurrent test suites run. Automatically set to 1 when --platform is specified.\n' ' (defaults to "4")\n' '-t, --tags Run only tests associated with the specified tags.\n' - " --exclude-coverage A glob which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart').\n" + " --exclude-coverage One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" '-x, --exclude-tags Run only tests that do not have the specified tags.\n' ' --min-coverage Whether to enforce a minimum coverage percentage. Implicitly enables coverage collection when used alone.\n' ' --show-uncovered Whether to show uncovered lines when coverage is below 100%. Implicitly enables coverage collection when used alone.\n' From cc91c40ac23740bcd0e7504cd1f1ef26cae089dd Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 5 Oct 2026 18:17:11 +0300 Subject: [PATCH 8/8] refactor(test): address review findings on the test runners --- lib/src/cli/cli.dart | 1 + lib/src/cli/flutter_cli.dart | 50 ----- lib/src/cli/test_cli_runner.dart | 97 +++++--- lib/src/cli/test_event_reporter.dart | 53 +++++ .../dart/commands/dart_test_command.dart | 74 ++---- lib/src/commands/test/test.dart | 74 ++---- lib/src/coverage/coverage.dart | 2 + lib/src/coverage/coverage_metrics.dart | 44 +--- lib/src/coverage/coverage_options.dart | 64 ++++++ lib/src/coverage/coverage_report.dart | 58 ++--- lib/src/coverage/exclude_globs.dart | 10 + lib/src/mcp/mcp_server.dart | 5 +- .../very_good_config/very_good_config.dart | 8 +- site/docs/commands/test.md | 2 +- site/docs/configuration.md | 4 +- test/src/cli/test_cli_runner_test.dart | 211 ++++++++++++------ .../dart/commands/dart_test_test.dart | 12 +- test/src/commands/test/test_test.dart | 12 +- test/src/coverage/coverage_metrics_test.dart | 53 +++-- test/src/coverage/coverage_options_test.dart | 36 +++ test/src/coverage/coverage_report_test.dart | 80 +++++-- 21 files changed, 566 insertions(+), 384 deletions(-) create mode 100644 lib/src/coverage/coverage_options.dart create mode 100644 lib/src/coverage/exclude_globs.dart create mode 100644 test/src/coverage/coverage_options_test.dart diff --git a/lib/src/cli/cli.dart b/lib/src/cli/cli.dart index 0c0f78024..1143a98e4 100644 --- a/lib/src/cli/cli.dart +++ b/lib/src/cli/cli.dart @@ -12,6 +12,7 @@ import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/test_optimizer/test_optimizer.dart'; import 'package:very_good_test_runner/very_good_test_runner.dart'; +export 'package:very_good_cli/src/coverage/coverage.dart'; export 'package:very_good_cli/src/test_optimizer/test_optimizer.dart'; part 'dart_cli.dart'; diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart index 9a8656e17..2027eb075 100644 --- a/lib/src/cli/flutter_cli.dart +++ b/lib/src/cli/flutter_cli.dart @@ -229,53 +229,3 @@ Future> _runCommand({ } return results; } - -// The extension is intended to be unnamed, but it's not possible due to -// an issue with Dart SDK 2.18.0. -// -// Once the min Dart SDK is bumped, this extension can be unnamed again. -extension _TestEvent on TestEvent { - bool shouldCancelTimer() { - final event = this; - if (event is MessageTestEvent) return true; - if (event is ErrorTestEvent) return true; - if (event is DoneTestEvent) return true; - if (event is TestDoneEvent) return !event.hidden; - return false; - } -} - -extension on Duration { - String formatted() { - String twoDigits(int n) => n.toString().padLeft(2, '0'); - final twoDigitMinutes = twoDigits(inMinutes.remainder(60)); - final twoDigitSeconds = twoDigits(inSeconds.remainder(60)); - return darkGray.wrap('$twoDigitMinutes:$twoDigitSeconds')!; - } -} - -extension on int { - String formatSuccess() { - return this > 0 ? lightGreen.wrap('+$this')! : ''; - } - - String formatFailure() { - return this > 0 ? lightRed.wrap('-$this')! : ''; - } - - String formatSkipped() { - return this > 0 ? lightYellow.wrap('~$this')! : ''; - } -} - -extension on String { - String truncated(int maxLength) { - if (length <= maxLength) return this; - final truncated = substring(length - maxLength, length).trim(); - return '...$truncated'; - } - - String toSingleLine() { - return replaceAll('\n', '').replaceAll(RegExp(r'\s\s+'), ' '); - } -} diff --git a/lib/src/cli/test_cli_runner.dart b/lib/src/cli/test_cli_runner.dart index 8833d33b4..3c88e8ee5 100644 --- a/lib/src/cli/test_cli_runner.dart +++ b/lib/src/cli/test_cli_runner.dart @@ -12,10 +12,37 @@ typedef VeryGoodTestRunner = Stream Function({ /// Which test runner to use for running tests. enum TestRunType { /// Run tests using `flutter test`. - flutter, + flutter('Flutter'), /// Run tests using `dart test`. - dart, + dart('Dart'); + + new(this.projectKind); + + /// The kind of project the runner tests, as named in messages. + final String projectKind; + + /// The command of `package:very_good_test_runner` that runs the tests. + VeryGoodTestRunner get runner => switch (this) { + TestRunType.flutter => flutterTest, + TestRunType.dart => dartTest, + }; + + /// The [CoverageReport] of a run of the tests of the package at + /// [packageRoot]. + CoverageReport coverageReportOf( + String packageRoot, { + required CoverageOptions options, + }) => switch (this) { + TestRunType.flutter => FlutterCoverageReport( + packageRoot: packageRoot, + options: options, + ), + TestRunType.dart => DartCoverageReport( + packageRoot: packageRoot, + options: options, + ), + }; } /// A class to run test command from a CLI command, like `flutter` or `dart`. @@ -112,13 +139,12 @@ class TestCLIRunner { /// /// Logs the problem and returns the exit code to stop with, or returns /// `null` when the run can proceed. [rest] are the positional arguments of - /// the command and [projectKind] names the project in the messages, such as - /// `Flutter` or `Dart`. + /// the command and [testType] names the project in the messages. static int? validateTarget({ required String targetPath, required bool recursive, required List rest, - required String projectKind, + required TestRunType testType, required Logger logger, }) { if (recursive && isTargettingTestFiles(rest)) { @@ -132,9 +158,11 @@ that contains them.'''); final pubspec = File(p.join(targetPath, 'pubspec.yaml')); if (!recursive && !pubspec.existsSync()) { - logger.err(''' + logger.err( + ''' Could not find a pubspec.yaml in $targetPath. -This command should be run from the root of your $projectKind project.'''); +This command should be run from the root of your ${testType.projectKind} project.''', + ); return ExitCode.noInput.code; } @@ -166,9 +194,7 @@ This command should be run from the root of your $projectKind project.'''); }) { final initialCwd = cwd; - final testRunner = - overrideTestRunner ?? - (testType == TestRunType.flutter ? flutterTest : dartTest); + final testRunner = overrideTestRunner ?? testType.runner; final coverageOptions = CoverageOptions( collect: collectCoverage, @@ -180,17 +206,6 @@ This command should be run from the root of your $projectKind project.'''); checkIgnore: checkIgnore, ); - CoverageReport coverageReportOf(String packageRoot) => switch (testType) { - TestRunType.flutter => FlutterCoverageReport( - packageRoot: packageRoot, - options: coverageOptions, - ), - TestRunType.dart => DartCoverageReport( - packageRoot: packageRoot, - options: coverageOptions, - ), - }; - return _runCommand( cmd: (cwd) => _testPackage( cwd: cwd, @@ -199,7 +214,10 @@ This command should be run from the root of your $projectKind project.'''); testType: testType, testRunner: testRunner, optimizer: optimizer, - coverageReport: coverageReportOf(cwd), + coverageReport: testType.coverageReportOf( + cwd, + options: coverageOptions, + ), randomSeed: randomSeed, forceAnsi: forceAnsi, arguments: arguments, @@ -297,13 +315,35 @@ This command should be run from the root of your $projectKind project.'''); ? body.call() : overrideAnsiOutput(enableAnsiOutput, body); + /// Awaits the exit codes of [runTests] and maps the outcome of the run to + /// the exit code of the command, logging any failure. + static Future exitCodeOf( + Future> Function() runTests, { + required Logger logger, + }) async { + try { + final results = await runTests(); + return results.every((code) => code == ExitCode.success.code) + ? ExitCode.success.code + : ExitCode.software.code; + } on MinCoverageNotMet catch (error) { + return _handleMinCoverageNotMet(error, logger: logger); + } on InvalidOptimizationGlob catch (error) { + logger.err('$error'); + return ExitCode.config.code; + } on Exception catch (error) { + logger.err('$error'); + return ExitCode.unavailable.code; + } + } + /// Logs [error], along with its uncovered lines when it carries any, and /// returns the exit code an unmet coverage threshold reports. - static int handleMinCoverageNotMet( + static int _handleMinCoverageNotMet( MinCoverageNotMet error, { required Logger logger, - double? minCoverage, }) { + final minCoverage = error.minCoverage; var decimalPlaces = 2; double round(double x) { @@ -311,7 +351,7 @@ This command should be run from the root of your $projectKind project.'''); return (x * b).roundToDouble() / b; } - if (error.coverage < minCoverage!) { + if (error.coverage < minCoverage) { var rounded = round(error.coverage); while (rounded == minCoverage) { decimalPlaces++; @@ -336,9 +376,6 @@ This command should be run from the root of your $projectKind project.'''); /// example because `--exclude-tags` filtered out every test. const _noTestsRanExitCode = 79; -/// Clears the current terminal line and moves the cursor to its start. -const _clearLine = '\u001B[2K\r'; - Future _testCommand({ required void Function(String) stdout, required void Function(String) stderr, @@ -391,7 +428,7 @@ Future _testCommand({ if (event is! ExitTestEvent || completer.isCompleted) return; unawaited(subscription.cancel()); unawaited(sigintWatchSubscription.cancel()); - completer.complete(_exitCodeOf(event, optimization)); + completer.complete(_exitCodeOfEvent(event, optimization)); }, onError: (Object error, StackTrace stackTrace) { stderr('$_clearLine$error'); @@ -403,7 +440,7 @@ Future _testCommand({ } /// The exit code a test run that ended with [event] reports. -int _exitCodeOf(ExitTestEvent event, TestOptimization optimization) { +int _exitCodeOfEvent(ExitTestEvent event, TestOptimization optimization) { // A shard can end up holding only tests that the given tags // filter out, which is expected and not a failure. final noTestsRanInShard = diff --git a/lib/src/cli/test_event_reporter.dart b/lib/src/cli/test_event_reporter.dart index 8f58912b9..182dd5614 100644 --- a/lib/src/cli/test_event_reporter.dart +++ b/lib/src/cli/test_event_reporter.dart @@ -179,3 +179,56 @@ final int _lineLength = () { return 80; } }(); + +/// Clears the current terminal line and moves the cursor to its start. +const _clearLine = '\u001B[2K\r'; + +// The extension is intended to be unnamed, but it's not possible due to +// an issue with Dart SDK 2.18.0. +// +// Once the min Dart SDK is bumped, this extension can be unnamed again. +extension _TestEvent on TestEvent { + bool shouldCancelTimer() { + final event = this; + if (event is MessageTestEvent) return true; + if (event is ErrorTestEvent) return true; + if (event is DoneTestEvent) return true; + if (event is TestDoneEvent) return !event.hidden; + return false; + } +} + +extension on Duration { + String formatted() { + String twoDigits(int n) => n.toString().padLeft(2, '0'); + final twoDigitMinutes = twoDigits(inMinutes.remainder(60)); + final twoDigitSeconds = twoDigits(inSeconds.remainder(60)); + return darkGray.wrap('$twoDigitMinutes:$twoDigitSeconds')!; + } +} + +extension on int { + String formatSuccess() { + return this > 0 ? lightGreen.wrap('+$this')! : ''; + } + + String formatFailure() { + return this > 0 ? lightRed.wrap('-$this')! : ''; + } + + String formatSkipped() { + return this > 0 ? lightYellow.wrap('~$this')! : ''; + } +} + +extension on String { + String truncated(int maxLength) { + if (length <= maxLength) return this; + final truncated = substring(length - maxLength, length).trim(); + return '...$truncated'; + } + + String toSingleLine() { + return replaceAll('\n', '').replaceAll(RegExp(r'\s\s+'), ' '); + } +} diff --git a/lib/src/commands/dart/commands/dart_test_command.dart b/lib/src/commands/dart/commands/dart_test_command.dart index d238aaa32..dfbcc8860 100644 --- a/lib/src/commands/dart/commands/dart_test_command.dart +++ b/lib/src/commands/dart/commands/dart_test_command.dart @@ -7,7 +7,6 @@ import 'package:mason/mason.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:very_good_cli/src/cli/cli.dart'; -import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Dart test command. @@ -160,8 +159,8 @@ class DartTestOptions { /// Run only tests associated with the specified tags. final String? tags; - /// One or more space-separated globs which will be used to exclude files that - /// match from the coverage. + /// One or more space-separated globs, relative to the package root, which + /// will be used to exclude files that match from the coverage. final String? excludeFromCoverage; /// How to collect coverage. @@ -225,8 +224,18 @@ class DartTestOptions { !TestCLIRunner.isTargettingTestFiles(rest) && platform == null; + /// The coverage threshold the run enforces. A threshold inherited from + /// `very_good.yaml` only applies to un-sharded runs: a single shard covers a + /// fraction of the code and would fail it. + double? get _enforcedMinCoverage => totalShards == null ? minCoverage : null; + + /// Whether the run collects coverage, which enforcing a threshold and + /// listing the uncovered lines need too. + bool get _shouldCollectCoverage => + collectCoverage || _enforcedMinCoverage != null || showUncovered; + /// The arguments forwarded verbatim to `dart test`. - List get _testArguments => [ + List get _runnerArguments => [ if (excludeTags != null) ...['-x', excludeTags!], if (tags != null) ...['-t', tags!], if (failFast) '--fail-fast', @@ -320,13 +329,7 @@ class DartTestCommand extends Command { abbr: 't', help: 'Run only tests associated with the specified tags.', ) - ..addOption( - 'exclude-coverage', - help: - 'One or more space-separated globs which will be used to exclude ' - 'files that match from the coverage ' - "(e.g. '**/*.g.dart **/*.freezed.dart').", - ) + ..addOption('exclude-coverage', help: excludeCoverageHelp) ..addOption( 'exclude-tags', abbr: 'x', @@ -450,7 +453,7 @@ class DartTestCommand extends Command { targetPath: targetPath, recursive: recursive, rest: _argResults.rest, - projectKind: 'Dart', + testType: TestRunType.dart, logger: _logger, ); if (targetError != null) return targetError; @@ -479,58 +482,29 @@ class DartTestCommand extends Command { } /// Runs `dart test` with [options] and maps its outcome to an exit code. - Future _runTests( - DartTestOptions options, { - required bool recursive, - }) async { - // A threshold inherited from very_good.yaml only applies to un-sharded - // runs: a single shard covers a fraction of the code and would fail it. - final minCoverage = options.totalShards == null - ? options.minCoverage - : null; - - try { - final results = await _dartTest( + Future _runTests(DartTestOptions options, {required bool recursive}) { + return TestCLIRunner.exitCodeOf( + () => _dartTest( optimizePerformance: options.shouldOptimize, excludeOptimization: options.excludeOptimization, recursive: recursive, logger: _logger, stdout: _logger.write, stderr: _logger.err, - collectCoverage: - options.collectCoverage || - minCoverage != null || - options.showUncovered, - minCoverage: minCoverage, + collectCoverage: options._shouldCollectCoverage, + minCoverage: options._enforcedMinCoverage, showUncovered: options.showUncovered, excludeFromCoverage: options.excludeFromCoverage, collectCoverageFrom: options.collectCoverageFrom, randomSeed: options.randomSeed, forceAnsi: options.forceAnsi, - arguments: options._testArguments, + arguments: options._runnerArguments, reportOn: options.reportOn.isEmpty ? null : options.reportOn, checkIgnore: options.checkIgnore, shardIndex: int.tryParse(options.shardIndex ?? ''), totalShards: int.tryParse(options.totalShards ?? ''), - ); - - if (results.any((code) => code != ExitCode.success.code)) { - return ExitCode.software.code; - } - } on MinCoverageNotMet catch (error) { - return TestCLIRunner.handleMinCoverageNotMet( - error, - logger: _logger, - minCoverage: minCoverage, - ); - } on InvalidOptimizationGlob catch (error) { - _logger.err('$error'); - return ExitCode.config.code; - } on Exception catch (error) { - _logger.err('$error'); - return ExitCode.unavailable.code; - } - - return ExitCode.success.code; + ), + logger: _logger, + ); } } diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 2229c2df8..1f24ca716 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -7,7 +7,6 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:universal_io/io.dart'; import 'package:very_good_cli/src/cli/cli.dart'; -import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Flutter test command. @@ -186,8 +185,8 @@ class FlutterTestOptions { /// Run only tests associated with the specified tags. final String? tags; - /// One or more space-separated globs which will be used to exclude files that - /// match from the coverage. + /// One or more space-separated globs, relative to the package root, which + /// will be used to exclude files that match from the coverage. final String? excludeFromCoverage; /// How to collect coverage. @@ -265,8 +264,18 @@ class FlutterTestOptions { !updateGoldens && platform == null; + /// The coverage threshold the run enforces. A threshold inherited from + /// `very_good.yaml` only applies to un-sharded runs: a single shard covers a + /// fraction of the code and would fail it. + double? get _enforcedMinCoverage => totalShards == null ? minCoverage : null; + + /// Whether the run collects coverage, which enforcing a threshold and + /// listing the uncovered lines need too. + bool get _shouldCollectCoverage => + collectCoverage || _enforcedMinCoverage != null || showUncovered; + /// The arguments forwarded to `flutter test`. - List get _flutterTestArguments => [ + List get _runnerArguments => [ if (excludeTags != null) ...['-x', excludeTags!], if (tags != null) ...['-t', tags!], if (updateGoldens) '--update-goldens', @@ -373,13 +382,7 @@ class TestCommand extends Command { abbr: 't', help: 'Run only tests associated with the specified tags.', ) - ..addOption( - 'exclude-coverage', - help: - 'One or more space-separated globs which will be used to exclude ' - 'files that match from the coverage ' - "(e.g. '**/*.g.dart **/*.freezed.dart').", - ) + ..addOption('exclude-coverage', help: excludeCoverageHelp) ..addOption( 'exclude-tags', abbr: 'x', @@ -541,7 +544,7 @@ class TestCommand extends Command { targetPath: targetPath, recursive: recursive, rest: _argResults.rest, - projectKind: 'Flutter', + testType: TestRunType.flutter, logger: _logger, ); if (targetError != null) return targetError; @@ -570,29 +573,17 @@ class TestCommand extends Command { } /// Runs `flutter test` with [options] and maps its outcome to an exit code. - Future _runTests( - FlutterTestOptions options, { - required bool recursive, - }) async { - // A threshold inherited from very_good.yaml only applies to un-sharded - // runs: a single shard covers a fraction of the code and would fail it. - final minCoverage = options.totalShards == null - ? options.minCoverage - : null; - - try { - final results = await _flutterTest( + Future _runTests(FlutterTestOptions options, {required bool recursive}) { + return TestCLIRunner.exitCodeOf( + () => _flutterTest( optimizePerformance: options.shouldOptimize, excludeOptimization: options.excludeOptimization, recursive: recursive, logger: _logger, stdout: _logger.write, stderr: _logger.err, - collectCoverage: - options.collectCoverage || - minCoverage != null || - options.showUncovered, - minCoverage: minCoverage, + collectCoverage: options._shouldCollectCoverage, + minCoverage: options._enforcedMinCoverage, showUncovered: options.showUncovered, excludeFromCoverage: options.excludeFromCoverage, collectCoverageFrom: options.collectCoverageFrom, @@ -601,26 +592,9 @@ class TestCommand extends Command { reportOn: options.reportOn.isEmpty ? null : options.reportOn, shardIndex: int.tryParse(options.shardIndex ?? ''), totalShards: int.tryParse(options.totalShards ?? ''), - arguments: options._flutterTestArguments, - ); - - if (results.any((code) => code != ExitCode.success.code)) { - return ExitCode.software.code; - } - } on MinCoverageNotMet catch (error) { - return TestCLIRunner.handleMinCoverageNotMet( - error, - logger: _logger, - minCoverage: minCoverage, - ); - } on InvalidOptimizationGlob catch (error) { - _logger.err('$error'); - return ExitCode.config.code; - } on Exception catch (error) { - _logger.err('$error'); - return ExitCode.unavailable.code; - } - - return ExitCode.success.code; + arguments: options._runnerArguments, + ), + logger: _logger, + ); } } diff --git a/lib/src/coverage/coverage.dart b/lib/src/coverage/coverage.dart index e4e5d4c2b..9e5fbc400 100644 --- a/lib/src/coverage/coverage.dart +++ b/lib/src/coverage/coverage.dart @@ -7,5 +7,7 @@ import 'package:path/path.dart' as p; import 'package:universal_io/io.dart'; part 'coverage_metrics.dart'; +part 'coverage_options.dart'; part 'coverage_report.dart'; +part 'exclude_globs.dart'; part 'untested_files.dart'; diff --git a/lib/src/coverage/coverage_metrics.dart b/lib/src/coverage/coverage_metrics.dart index f88b5e909..1dec773cf 100644 --- a/lib/src/coverage/coverage_metrics.dart +++ b/lib/src/coverage/coverage_metrics.dart @@ -1,39 +1,5 @@ part of 'coverage.dart'; -/// How to collect coverage. -enum CoverageCollectionMode { - /// Collect coverage from imported files only (default behavior). - imports, - - /// Collect coverage from all files in the project. - all; - - /// Parses a string value into a [CoverageCollectionMode]. - static CoverageCollectionMode fromString(String value) { - return CoverageCollectionMode.values.firstWhere( - (mode) => mode.name == value, - orElse: () => CoverageCollectionMode.imports, - ); - } -} - -/// {@template coverage_not_met} -/// Thrown when `flutter test ---coverage --min-coverage` -/// does not meet the provided minimum coverage threshold. -/// {@endtemplate} -class MinCoverageNotMet implements Exception { - /// {@macro coverage_not_met} - const new(this.coverage, {this.uncoveredLines}); - - /// The measured coverage percentage (total hits / total found * 100). - final double coverage; - - /// Lines not covered, keyed by file path, values are line numbers. - /// - /// Only populated when `--show-uncovered` is set. - final Map>? uncoveredLines; -} - /// {@template coverage_metrics} /// Aggregated coverage metrics computed from a list of LCOV records. /// {@endtemplate} @@ -94,7 +60,8 @@ class CoverageMetrics { final int totalFound; /// Lines not covered. - /// Keyed by file path, values are sorted line numbers. + /// Keyed by file path, values are line numbers in the order the records + /// list them. final Map> uncoveredLines; /// Coverage percentage: [totalHits] / [totalFound] * 100. @@ -105,13 +72,6 @@ class CoverageMetrics { } } -/// Parses the space-separated glob patterns of an `--exclude-coverage` -/// value, ignoring empty segments. -List _parseExcludeGlobs(String? excludeFromCoverage) => [ - for (final pattern in (excludeFromCoverage ?? '').trim().split(' ')) - if (pattern.isNotEmpty) Glob(pattern), -]; - /// Formats a map of uncovered lines into a human-readable string. /// /// The [uncoveredLines] map is keyed by file path, with values being lists diff --git a/lib/src/coverage/coverage_options.dart b/lib/src/coverage/coverage_options.dart new file mode 100644 index 000000000..7d17f483d --- /dev/null +++ b/lib/src/coverage/coverage_options.dart @@ -0,0 +1,64 @@ +part of 'coverage.dart'; + +/// The help of the `--exclude-coverage` option, shared by every command and +/// tool that forwards it. +const excludeCoverageHelp = + 'One or more space-separated globs, relative to the package root, which ' + 'will be used to exclude files that match from the coverage ' + "(e.g. '**/*.g.dart **/*.freezed.dart')."; + +/// How to collect coverage. +enum CoverageCollectionMode { + /// Collect coverage from imported files only (default behavior). + imports, + + /// Collect coverage from all files in the project. + all; + + /// Parses a string value into a [CoverageCollectionMode]. + static CoverageCollectionMode fromString(String value) { + return CoverageCollectionMode.values.firstWhere( + (mode) => mode.name == value, + orElse: () => CoverageCollectionMode.imports, + ); + } +} + +/// {@template coverage_options} +/// The coverage settings shared by every package of a test run. +/// {@endtemplate} +@immutable +class CoverageOptions { + /// {@macro coverage_options} + const new({ + this.collect = false, + this.collectFrom = CoverageCollectionMode.imports, + this.minCoverage, + this.showUncovered = false, + this.excludeFromCoverage, + this.reportOn = const ['lib'], + this.checkIgnore = false, + }); + + /// Whether to collect coverage into `coverage/lcov.info`. + final bool collect; + + /// Which files the lcov report accounts for. + final CoverageCollectionMode collectFrom; + + /// The minimum coverage percentage the run must reach, if any. + final double? minCoverage; + + /// Whether to list the lines left uncovered. + final bool showUncovered; + + /// Whitespace-separated globs of the files left out of the coverage, + /// matched against paths relative to the package root. + final String? excludeFromCoverage; + + /// The directories, relative to the package, the coverage reports on. + final List reportOn; + + /// Whether to honor the `coverage:ignore` comments. + final bool checkIgnore; +} diff --git a/lib/src/coverage/coverage_report.dart b/lib/src/coverage/coverage_report.dart index 1afa6f78f..3d29d0c00 100644 --- a/lib/src/coverage/coverage_report.dart +++ b/lib/src/coverage/coverage_report.dart @@ -1,41 +1,23 @@ part of 'coverage.dart'; -/// {@template coverage_options} -/// The coverage settings shared by every package of a test run. +/// {@template coverage_not_met} +/// Thrown when `flutter test ---coverage --min-coverage` +/// does not meet the provided minimum coverage threshold. /// {@endtemplate} -@immutable -class CoverageOptions { - /// {@macro coverage_options} - const new({ - this.collect = false, - this.collectFrom = CoverageCollectionMode.imports, - this.minCoverage, - this.showUncovered = false, - this.excludeFromCoverage, - this.reportOn = const ['lib'], - this.checkIgnore = false, - }); - - /// Whether to collect coverage into `coverage/lcov.info`. - final bool collect; - - /// Which files the lcov report accounts for. - final CoverageCollectionMode collectFrom; - - /// The minimum coverage percentage the run must reach, if any. - final double? minCoverage; - - /// Whether to list the lines left uncovered. - final bool showUncovered; - - /// Space-separated globs of the files left out of the coverage. - final String? excludeFromCoverage; - - /// The directories, relative to the package, the coverage reports on. - final List reportOn; - - /// Whether to honor the `coverage:ignore` comments. - final bool checkIgnore; +class MinCoverageNotMet implements Exception { + /// {@macro coverage_not_met} + const new(this.coverage, {required this.minCoverage, this.uncoveredLines}); + + /// The measured coverage percentage (total hits / total found * 100). + final double coverage; + + /// The minimum coverage percentage the run had to reach. + final double minCoverage; + + /// Lines not covered, keyed by file path, values are line numbers. + /// + /// Only populated when `--show-uncovered` is set. + final Map>? uncoveredLines; } /// {@template coverage_report} @@ -120,7 +102,11 @@ sealed class CoverageReport { final minCoverage = options.minCoverage; if (minCoverage != null && percentage < minCoverage) { - throw MinCoverageNotMet(percentage, uncoveredLines: uncoveredLines); + throw MinCoverageNotMet( + percentage, + minCoverage: minCoverage, + uncoveredLines: uncoveredLines, + ); } // When coverage passes but is below 100%, diff --git a/lib/src/coverage/exclude_globs.dart b/lib/src/coverage/exclude_globs.dart new file mode 100644 index 000000000..e4d157344 --- /dev/null +++ b/lib/src/coverage/exclude_globs.dart @@ -0,0 +1,10 @@ +part of 'coverage.dart'; + +/// Parses the whitespace-separated glob patterns of an `--exclude-coverage` +/// value, ignoring empty segments. +List _parseExcludeGlobs(String? excludeFromCoverage) => [ + for (final pattern in (excludeFromCoverage ?? '').split(_whitespace)) + if (pattern.isNotEmpty) Glob(pattern), +]; + +final _whitespace = RegExp(r'\s+'); diff --git a/lib/src/mcp/mcp_server.dart b/lib/src/mcp/mcp_server.dart index de840d410..ed753ea26 100644 --- a/lib/src/mcp/mcp_server.dart +++ b/lib/src/mcp/mcp_server.dart @@ -9,6 +9,7 @@ import 'package:mason/mason.dart' hide packageVersion; import 'package:meta/meta.dart'; import 'package:stream_channel/stream_channel.dart'; import 'package:very_good_cli/src/command_runner.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/mcp/lock.dart'; import 'package:very_good_cli/src/mcp/structured_tool_error.dart'; import 'package:very_good_cli/src/version.dart'; @@ -215,9 +216,7 @@ Automatically set to 1 when --platform is specified. description: '''Run only tests associated with the specified tags.''', ), - 'exclude_coverage': StringSchema( - description: '''One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').''', - ), + 'exclude_coverage': StringSchema(description: excludeCoverageHelp), 'exclude_tags': StringSchema( description: 'Run only tests that do not have the specified tags.', diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 8e75a171c..1b5ebdb59 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -313,8 +313,8 @@ class VeryGoodTestConfig extends Equatable { /// Run only tests associated with the specified tags. final String? tags; - /// One or more space-separated globs which will be used to exclude files that - /// match from the coverage. + /// One or more space-separated globs, relative to the package root, which + /// will be used to exclude files that match from the coverage. final String? excludeCoverage; /// Run only tests that do not have the specified tags. @@ -471,8 +471,8 @@ class VeryGoodDartTestConfig extends Equatable { /// Run only tests associated with the specified tags. final String? tags; - /// One or more space-separated globs which will be used to exclude files that - /// match from the coverage. + /// One or more space-separated globs, relative to the package root, which + /// will be used to exclude files that match from the coverage. final String? excludeCoverage; /// Run only tests that do not have the specified tags. diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index 6c9447fff..d14726601 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -23,7 +23,7 @@ very_good test [arguments] -j, --concurrency The number of concurrent test suites run. (defaults to "4") -t, --tags Run only tests associated with the specified tags. - --exclude-coverage One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart'). + --exclude-coverage One or more space-separated globs, relative to the package root, which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart'). -x, --exclude-tags Run only tests that do not have the specified tags. --min-coverage Whether to enforce a minimum coverage percentage. Implicitly enables coverage collection when used alone. diff --git a/site/docs/configuration.md b/site/docs/configuration.md index fdc5fbaeb..e25519b98 100644 --- a/site/docs/configuration.md +++ b/site/docs/configuration.md @@ -92,7 +92,7 @@ test: | `optimization` | `bool` \| `map` | Whether to apply optimizations for test performance. See [`optimization`](#optimization). | | `concurrency` | `int` | Positive integer. The number of concurrent test suites run. | | `tags` | `string` | Run only tests associated with the specified tags. | -| `exclude_coverage` | `string` | Space-separated globs that exclude matching files from coverage. | +| `exclude_coverage` | `string` | Space-separated globs, relative to each package root, that exclude files from coverage. | | `exclude_tags` | `string` | Run only tests that do not have the specified tags. | | `min_coverage` | `number` | Between `0` and `100`. Enforces a minimum coverage percentage. | | `show_uncovered` | `bool` | Whether to show uncovered lines when coverage is below 100%. | @@ -201,7 +201,7 @@ dart: | `optimization` | `bool` \| `map` | Whether to apply optimizations for test performance. See [`optimization`](#optimization). | | `concurrency` | `int` | Positive integer. The number of concurrent test suites run. | | `tags` | `string` | Run only tests associated with the specified tags. | -| `exclude_coverage` | `string` | Space-separated globs that exclude matching files from coverage. | +| `exclude_coverage` | `string` | Space-separated globs, relative to each package root, that exclude files from coverage. | | `exclude_tags` | `string` | Run only tests that do not have the specified tags. | | `min_coverage` | `number` | Between `0` and `100`. Enforces a minimum coverage percentage. | | `show_uncovered` | `bool` | Whether to show uncovered lines when coverage is below 100%. | diff --git a/test/src/cli/test_cli_runner_test.dart b/test/src/cli/test_cli_runner_test.dart index 5521abc27..2a47233b5 100644 --- a/test/src/cli/test_cli_runner_test.dart +++ b/test/src/cli/test_cli_runner_test.dart @@ -10,7 +10,6 @@ import 'package:path/path.dart' as p; import 'package:stack_trace/stack_trace.dart' as stack_trace; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; -import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_test_runner/very_good_test_runner.dart'; import '../../fixtures/fixtures.dart'; @@ -26,21 +25,23 @@ class _MockLogger extends Mock implements Logger; class _FakeGeneratorTarget extends Fake implements GeneratorTarget; void main() { - group(CoverageCollectionMode, () { - group('.fromString', () { - test('returns matching mode for known value', () { - expect( - CoverageCollectionMode.fromString('all'), - equals(CoverageCollectionMode.all), - ); - }); + group(TestRunType, () { + test('runs the tests with the command of its runner', () { + expect(TestRunType.flutter.runner, equals(flutterTest)); + expect(TestRunType.dart.runner, equals(dartTest)); + }); - test('returns imports for unrecognized value', () { - expect( - CoverageCollectionMode.fromString('unknown'), - equals(CoverageCollectionMode.imports), - ); - }); + test('keeps the coverage the way its runner writes it', () { + const options = CoverageOptions(); + + expect( + TestRunType.flutter.coverageReportOf('.', options: options), + isA(), + ); + expect( + TestRunType.dart.coverageReportOf('.', options: options), + isA(), + ); }); }); @@ -50,6 +51,132 @@ void main() { registerFallbackValue(FileConflictResolution.prompt); }); + group('.validateTarget', () { + late Logger logger; + late Directory targetDirectory; + + setUp(() { + logger = _MockLogger(); + targetDirectory = Directory.systemTemp.createTempSync( + 'validate_target_', + ); + addTearDown(() => targetDirectory.deleteSync(recursive: true)); + }); + + int? validateTarget({ + bool recursive = false, + List rest = const [], + }) => TestCLIRunner.validateTarget( + targetPath: targetDirectory.path, + recursive: recursive, + rest: rest, + testType: TestRunType.dart, + logger: logger, + ); + + test('lets the tests of a package run', () { + File(p.join(targetDirectory.path, 'pubspec.yaml')).createSync(); + + expect(validateTarget(), isNull); + verifyNever(() => logger.err(any())); + }); + + test('lets a recursive run go without a pubspec', () { + expect(validateTarget(recursive: true), isNull); + verifyNever(() => logger.err(any())); + }); + + test('rejects test targets together with --recursive', () { + expect( + validateTarget(recursive: true, rest: ['test/a_test.dart']), + equals(ExitCode.usage.code), + ); + verify( + () => logger.err( + any( + that: contains( + 'Cannot target specific test files together with --recursive.', + ), + ), + ), + ).called(1); + }); + + test('rejects a directory without a pubspec', () { + expect(validateTarget(), equals(ExitCode.noInput.code)); + verify( + () => logger.err( + any( + that: contains( + 'This command should be run from the root of your Dart ' + 'project.', + ), + ), + ), + ).called(1); + }); + }); + + group('.exitCodeOf', () { + late Logger logger; + + setUp(() => logger = _MockLogger()); + + test('succeeds when every test process succeeds', () async { + await expectLater( + TestCLIRunner.exitCodeOf( + () async => [ExitCode.success.code, ExitCode.success.code], + logger: logger, + ), + completion(equals(ExitCode.success.code)), + ); + }); + + test('fails when a test process fails', () async { + await expectLater( + TestCLIRunner.exitCodeOf( + () async => [ExitCode.success.code, ExitCode.unavailable.code], + logger: logger, + ), + completion(equals(ExitCode.software.code)), + ); + }); + + test('reports an unmet coverage threshold', () async { + await expectLater( + TestCLIRunner.exitCodeOf( + () => throw const MinCoverageNotMet(50, minCoverage: 80), + logger: logger, + ), + completion(equals(ExitCode.software.code)), + ); + verify( + () => logger.err('Expected coverage >= 80.00% but actual is 50.00%.'), + ).called(1); + }); + + test('reports an invalid optimization glob as a config error', () async { + const exception = InvalidOptimizationGlob('bad glob'); + + await expectLater( + TestCLIRunner.exitCodeOf(() => throw exception, logger: logger), + completion(equals(ExitCode.config.code)), + ); + verify(() => logger.err('$exception')).called(1); + }); + + test('reports any other exception as unavailable', () async { + await expectLater( + TestCLIRunner.exitCodeOf( + () => throw Exception('oops'), + logger: logger, + ), + completion(equals(ExitCode.unavailable.code)), + ); + verify(() => logger.err('Exception: oops')).called(1); + }); + }); + group('.test', () { late Progress progress; late Logger logger; @@ -2019,60 +2146,6 @@ void main() { }, ); - test('respects exclude-coverage pattern when enhancing lcov', () async { - final tempDirectory = Directory.systemTemp.createTempSync(); - addTearDown(() => tempDirectory.deleteSync(recursive: true)); - - final libDir = Directory(p.join(tempDirectory.path, 'lib')) - ..createSync(recursive: true); - File(p.join(libDir.path, 'main.dart')) - .writeAsStringSync('void main() {}'); - File(p.join(libDir.path, 'main.g.dart')) - .writeAsStringSync('// Generated code'); - - File(p.join(tempDirectory.path, 'pubspec.yaml')).createSync(); - Directory(p.join(tempDirectory.path, 'test')).createSync(); - - final lcovFile = File( - p.join(tempDirectory.path, 'coverage', 'lcov.info'), - ); - - await expectLater( - TestCLIRunner.test( - testType: TestRunType.dart, - cwd: tempDirectory.path, - logger: logger, - collectCoverage: true, - collectCoverageFrom: CoverageCollectionMode.all, - excludeFromCoverage: '**/*.g.dart', - stdout: stdoutLogs.add, - stderr: stderrLogs.add, - overrideTestRunner: testRunner( - Stream.fromIterable([ - const DoneTestEvent(success: true, time: 0), - const ExitTestEvent(exitCode: 0, time: 0), - ]), - onStart: () { - // Create LCOV with covered file for main.dart only - lcovFile - ..createSync(recursive: true) - ..writeAsStringSync( - 'TN:test\n' - 'SF:lib/main.dart\n' - 'DA:1,1\n' - 'LH:1\n' - 'LF:1\n' - 'end_of_record\n', - ); - }, - ), - ), - completion(equals([ExitCode.success.code])), - ); - - expect(lcovFile.existsSync(), isTrue); - }); - test('respects every space-separated exclude-coverage pattern ' 'when enhancing lcov', () async { final tempDirectory = Directory.systemTemp.createTempSync(); diff --git a/test/src/commands/dart/commands/dart_test_test.dart b/test/src/commands/dart/commands/dart_test_test.dart index 143507807..9c0686f29 100644 --- a/test/src/commands/dart/commands/dart_test_test.dart +++ b/test/src/commands/dart/commands/dart_test_test.dart @@ -11,7 +11,6 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; import 'package:very_good_cli/src/commands/dart/commands/commands.dart'; -import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; import '../../../../helpers/helpers.dart'; @@ -37,7 +36,7 @@ const expectedTestUsage = [ '-j, --concurrency The number of concurrent test suites run. Automatically set to 1 when --platform is specified.\n' ' (defaults to "4")\n' '-t, --tags Run only tests associated with the specified tags.\n' - " --exclude-coverage One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" + " --exclude-coverage One or more space-separated globs, relative to the package root, which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" '-x, --exclude-tags Run only tests that do not have the specified tags.\n' ' --min-coverage Whether to enforce a minimum coverage percentage. Implicitly enables coverage collection when used alone.\n' ' --show-uncovered Whether to show uncovered lines when coverage is below 100%. Requires --coverage or --min-coverage to be set, or implicitly enables coverage collection when used alone.\n' @@ -570,7 +569,7 @@ void main() { test('fails when coverage not met', () async { when(() => argResults['coverage']).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('100'); - const exception = MinCoverageNotMet(0); + const exception = MinCoverageNotMet(0, minCoverage: 100); when( () => dartTest( cwd: any(named: 'cwd'), @@ -612,6 +611,7 @@ void main() { when(() => argResults['show-uncovered']).thenReturn(true); const exception = MinCoverageNotMet( 95, + minCoverage: 100, uncoveredLines: { 'lib/src/foo.dart': [10, 20, 30], }, @@ -647,7 +647,7 @@ void main() { test('displays required precision see why coverage was not met', () async { when(() => argResults['coverage']).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('100'); - const exception = MinCoverageNotMet(99.999995); + const exception = MinCoverageNotMet(99.999995, minCoverage: 100); when( () => dartTest( cwd: any(named: 'cwd'), @@ -808,6 +808,8 @@ void main() { test( '''disables optimizePerformance when rest arguement is not an option''', () async { + // Reading argResults.rest inside verify() would make mocktail verify + // that getter instead of the test runner call. final rest = ['my-test.dart']; when(() => argResults.rest).thenReturn(rest); @@ -860,6 +862,8 @@ void main() { test( 'enables optimizePerformance when rest arguement is an option', () async { + // Reading argResults.rest inside verify() would make mocktail verify + // that getter instead of the test runner call. final rest = ['--track-wdiget-creation']; when(() => argResults.rest).thenReturn(rest); diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 8ef249987..ee48020f7 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -11,7 +11,6 @@ import 'package:path/path.dart' as path; import 'package:test/test.dart'; import 'package:very_good_cli/src/cli/cli.dart'; import 'package:very_good_cli/src/commands/test/test.dart'; -import 'package:very_good_cli/src/coverage/coverage.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; import '../../../helpers/helpers.dart'; @@ -37,7 +36,7 @@ const expectedTestUsage = [ '-j, --concurrency The number of concurrent test suites run. Automatically set to 1 when --platform is specified.\n' ' (defaults to "4")\n' '-t, --tags Run only tests associated with the specified tags.\n' - " --exclude-coverage One or more space-separated globs which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" + " --exclude-coverage One or more space-separated globs, relative to the package root, which will be used to exclude files that match from the coverage (e.g. '**/*.g.dart **/*.freezed.dart').\n" '-x, --exclude-tags Run only tests that do not have the specified tags.\n' ' --min-coverage Whether to enforce a minimum coverage percentage. Implicitly enables coverage collection when used alone.\n' ' --show-uncovered Whether to show uncovered lines when coverage is below 100%. Implicitly enables coverage collection when used alone.\n' @@ -581,6 +580,8 @@ void main() { test( '''disables optimizePerformance when rest arguement is not an option''', () async { + // Reading argResults.rest inside verify() would make mocktail verify + // that getter instead of the test runner call. final rest = ['my-test.dart']; when(() => argResults.rest).thenReturn(rest); @@ -634,6 +635,8 @@ void main() { test( 'enables optimizePerformance when rest arguement is an option', () async { + // Reading argResults.rest inside verify() would make mocktail verify + // that getter instead of the test runner call. final rest = ['--track-wdiget-creation']; when(() => argResults.rest).thenReturn(rest); @@ -778,7 +781,7 @@ void main() { test('fails when coverage not met', () async { when(() => argResults['coverage']).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('100'); - const exception = MinCoverageNotMet(0); + const exception = MinCoverageNotMet(0, minCoverage: 100); when( () => flutterTest( cwd: any(named: 'cwd'), @@ -818,6 +821,7 @@ void main() { when(() => argResults['show-uncovered']).thenReturn(true); const exception = MinCoverageNotMet( 95, + minCoverage: 100, uncoveredLines: { 'lib/src/foo.dart': [10, 20, 30], }, @@ -855,7 +859,7 @@ void main() { () async { when(() => argResults['coverage']).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('100'); - const exception = MinCoverageNotMet(99.999995); + const exception = MinCoverageNotMet(99.999995, minCoverage: 100); when( () => flutterTest( cwd: any(named: 'cwd'), diff --git a/test/src/coverage/coverage_metrics_test.dart b/test/src/coverage/coverage_metrics_test.dart index a9ff70b55..9c393e0fe 100644 --- a/test/src/coverage/coverage_metrics_test.dart +++ b/test/src/coverage/coverage_metrics_test.dart @@ -193,30 +193,35 @@ void main() { expect(metrics.totalHits, equals(8)); }); - test('handles multiple consecutive spaces between globs', () { - final records = parseRecords([ - 'SF:lib/a.dart', - 'LF:10', - 'LH:8', - 'end_of_record', - 'SF:lib/generated/b.g.dart', - 'LF:5', - 'LH:5', - 'end_of_record', - 'SF:lib/mocks/mock_c.dart', - 'LF:4', - 'LH:4', - 'end_of_record', - ]); - - final metrics = CoverageMetrics.fromLcovRecords( - records, - excludeFromCoverage: 'lib/generated/** lib/mocks/**', - ); - - expect(metrics.totalFound, equals(10)); - expect(metrics.totalHits, equals(8)); - }); + for (final (description, separator) in [ + ('multiple consecutive spaces', ' '), + ('tabs and newlines', '\t\n'), + ]) { + test('handles $description between globs', () { + final records = parseRecords([ + 'SF:lib/a.dart', + 'LF:10', + 'LH:8', + 'end_of_record', + 'SF:lib/generated/b.g.dart', + 'LF:5', + 'LH:5', + 'end_of_record', + 'SF:lib/mocks/mock_c.dart', + 'LF:4', + 'LH:4', + 'end_of_record', + ]); + + final metrics = CoverageMetrics.fromLcovRecords( + records, + excludeFromCoverage: 'lib/generated/**${separator}lib/mocks/**', + ); + + expect(metrics.totalFound, equals(10)); + expect(metrics.totalHits, equals(8)); + }); + } test('does not exclude file when glob does not match', () { final records = parseRecords([ diff --git a/test/src/coverage/coverage_options_test.dart b/test/src/coverage/coverage_options_test.dart new file mode 100644 index 000000000..079aa3651 --- /dev/null +++ b/test/src/coverage/coverage_options_test.dart @@ -0,0 +1,36 @@ +import 'package:test/test.dart'; +import 'package:very_good_cli/src/coverage/coverage.dart'; + +void main() { + group(CoverageCollectionMode, () { + group('.fromString', () { + test('returns matching mode for known value', () { + expect( + CoverageCollectionMode.fromString('all'), + equals(CoverageCollectionMode.all), + ); + }); + + test('returns imports for unrecognized value', () { + expect( + CoverageCollectionMode.fromString('unknown'), + equals(CoverageCollectionMode.imports), + ); + }); + }); + }); + + group(CoverageOptions, () { + test('has defaults that collect nothing', () { + const options = CoverageOptions(); + + expect(options.collect, isFalse); + expect(options.collectFrom, equals(CoverageCollectionMode.imports)); + expect(options.minCoverage, isNull); + expect(options.showUncovered, isFalse); + expect(options.excludeFromCoverage, isNull); + expect(options.reportOn, equals(['lib'])); + expect(options.checkIgnore, isFalse); + }); + }); +} diff --git a/test/src/coverage/coverage_report_test.dart b/test/src/coverage/coverage_report_test.dart index ddd1d679b..3fa30612e 100644 --- a/test/src/coverage/coverage_report_test.dart +++ b/test/src/coverage/coverage_report_test.dart @@ -6,20 +6,6 @@ import 'package:universal_io/io.dart'; import 'package:very_good_cli/src/coverage/coverage.dart'; void main() { - group(CoverageOptions, () { - test('has defaults that collect nothing', () { - const options = CoverageOptions(); - - expect(options.collect, isFalse); - expect(options.collectFrom, equals(CoverageCollectionMode.imports)); - expect(options.minCoverage, isNull); - expect(options.showUncovered, isFalse); - expect(options.excludeFromCoverage, isNull); - expect(options.reportOn, equals(['lib'])); - expect(options.checkIgnore, isFalse); - }); - }); - group(CoverageReport, () { late Directory packageRoot; late File lcovFile; @@ -172,6 +158,63 @@ void main() { ); }); + test('matches the exclude globs relative to the package root', () async { + writeLcov(coveredLcov); + writeSource('lib/generated/c.dart', 'void c() {}\n'); + + await flutterReport( + const CoverageOptions( + collect: true, + collectFrom: CoverageCollectionMode.all, + excludeFromCoverage: 'lib/generated/**', + ), + ).finalize(); + + expect(lcovFile.readAsStringSync(), equals(coveredLcov)); + }); + + test('marks every line but directives as untested', () async { + writeLcov(coveredLcov); + writeSource( + 'lib/b.dart', + "export 'a.dart';\n" + "part 'b.part.dart';\n" + 'void b() {}\n', + ); + + await flutterReport( + const CoverageOptions( + collect: true, + collectFrom: CoverageCollectionMode.all, + ), + ).finalize(); + + expect( + lcovFile.readAsStringSync(), + equals( + '${coveredLcov}SF:lib/b.dart\n' + 'DA:3,0\n' + 'LF:1\n' + 'LH:0\n' + 'end_of_record\n', + ), + ); + }); + + test('adds nothing for a report-on directory that is missing', () async { + writeLcov(coveredLcov); + + await flutterReport( + const CoverageOptions( + collect: true, + collectFrom: CoverageCollectionMode.all, + reportOn: ['missing'], + ), + ).finalize(); + + expect(lcovFile.readAsStringSync(), equals(coveredLcov)); + }); + test('throws $MinCoverageNotMet when below the threshold', () async { writeLcov(coveredLcov); @@ -186,6 +229,11 @@ void main() { throwsA( isA() .having((error) => error.coverage, 'coverage', equals(50)) + .having( + (error) => error.minCoverage, + 'minCoverage', + equals(100), + ) .having( (error) => error.uncoveredLines, 'uncoveredLines', @@ -265,7 +313,9 @@ void main() { expect( lcovFile.readAsStringSync(), equals( - 'SF:lib/a.dart\n' + // The coverage package writes the paths with the separator of the + // platform. + 'SF:${p.join('lib', 'a.dart')}\n' 'DA:1,1\n' 'DA:2,0\n' 'LF:2\n'