refactor(test): reduce cognitive complexity of the test runners

[marcossevilla]

Status

READY

Description

Reduces the cognitive complexity of the test runners, and moves coverage handling into its own library along the way.

• Test runner. TestCLIRunner.test hands each package to _testPackage. Test events go to _TestEventReporter, which switches on the event type and keeps the tallies and the list of failures. It now lives in its own test_event_reporter.dart part, so test_cli_runner.dart only orchestrates the run (719 → 415 lines).
• Coverage library (lib/src/coverage/). CoverageOptions holds the settings of a run. A sealed CoverageReport per package owns coverage/lcov.info:
  • FlutterCoverageReport keeps the file flutter test writes.
  • DartCoverageReport converts the json coverage of dart test into lcov.
  • The runner picks the report once and no longer branches on the test type for coverage, including the --coverage argument.
  • CoverageMetrics, CoverageCollectionMode, MinCoverageNotMet and formatUncoveredLines move there too, so lib/src/cli depends on lib/src/coverage and not the other way around.
  • .gitignore re-includes lib/src/coverage/ and test/src/coverage/, which its coverage/ rule for reports also matched.
• Commands. TestCommand and DartTestCommand share TestCLIRunner.validateTarget, and both run their tests through _runTests. The arguments they forward become private getters on their options classes.

Bug fix

With --collect-coverage-from all, the step that adds untested files to lcov.info didn't honor --exclude-coverage:

• It parsed the value as a single glob, so the documented '**/*.g.dart **/*.freezed.dart' excluded nothing.
• It matched globs against absolute paths, which package:glob resolves against the process's working directory instead of the package root. Recursive runs never excluded anything.

It now parses the value the same way CoverageMetrics does and matches paths relative to the package, like the lcov SF: entries. The threshold check was already correct, so only the contents of lcov.info change.

Other changes

• The lcov.info existence assert for dart runs with collectCoverageFrom: all now runs before the untested-files step. The file already exists at that point, and asserts are off in release builds.
• 4 command tests read argResults.rest inside verify(), which made mocktail verify that getter instead of the runner call. They now verify the runner.

Validation

Function	Before	After
TestCLIRunner.test	66	3
_testCommand	66	7
TestCommand.run	30	4
CoverageMetrics.fromLcovRecords	28	0
DartTestCommand.run	21	4
_enhanceLcovWithUntestedFiles (now _addUntestedFiles)	17	4

• cognitive_complexity 0.2.4: compared with main, nothing increased and nothing breaks the 15 ceiling (net delta -118). The highest score in any touched file is 11.
• undead 0.1.1 (closed-app): no new dead declarations or privatization candidates.
• dedupe 0.1.0: duplicate lines between TestCommand and DartTestCommand drop from 784 to 755. The rest of that duplication predates this PR, and a shared base for the two commands would be a follow-up.
• The public API (api_summary) is unchanged. Every changed line is covered by tests. New tests cover CoverageReport and the exclude-coverage fix, and the CoverageMetrics tests moved to test/src/coverage.

The commits are split so they can be stacked as separate PRs if needed.

Found by the full cognitive complexity scan in #1755, which tests VeryGoodOpenSource/very_good_workflows#520. This is part of a three-PR cleanup. Once all three merge, the full scan on #1755 passes:

• refactor(packages): reduce cognitive complexity of check licenses #1757 check licenses and pubspec workspace
• refactor(mcp): reduce cognitive complexity of the test tool #1758 MCP test tool

Type of Change

• ✨ New feature (non-breaking change which adds functionality)
• 🛠️ Bug fix (non-breaking change which fixes an issue)
• ❌ Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 Code refactor
• ✅ Build configuration change
• 📝 Documentation
• 🗑️ Chore

🤖 Generated with Claude Code

[marcossevilla]

@vgvbot /review-pr

[unicoderbot]

I'll review the code and share my feedback shortly.

• Preparing environment
  • Cloning repository
  • Setting up dev environment
• Reviewing code
• Posting review

Code review submitted — PR approved.

[unicoderbot]

Approve. This is a clean, careful refactor that significantly reduces cognitive complexity across the test runners while preserving behavior, log output, exit codes, and public APIs.

Verified correctness

• CoverageMetrics.fromLcovRecords — the _add + _uncoveredLinesWith decomposition preserves per-file accumulation (via [...?uncoveredLines[file], ...newLines]) and still filters by exclude globs the same way.
• _TestEventReporter switch — I confirmed TestEvent in very_good_test_runner 0.4.0 is abstract class TestEvent (not sealed), with subtypes StartTestEvent, AllSuitesTestEvent, DebugTestEvent, ExitTestEvent not matched in the new switch. In Dart, a non-exhaustive switch statement (not expression) over a non-sealed type simply falls through on no-match — matching the original if-chain's silent ignore of those types. ExitTestEvent continues to be handled explicitly in _testCommand.
• _checkCoverage — the simplified if (uncoveredLines != null) is sound because uncoveredLines is assigned non-null only when options.showUncovered && coverageMetrics.uncoveredLines.isNotEmpty.
• _reportDone — success = event.success ?? false; if (success) return; is equivalent to the original if (event.success != true) (continues on null/false, returns on true).
• _writeLcov ordering change for Dart+all — assert runs before enhance now; _convertDartCoverageToLcov creates the file beforehand, so the invariant holds. The PR description already calls this out.
• _enhanceLcovWithUntestedFiles — .map((r) => r.file).nonNulls.toSet() is equivalent to where((r) => r.file != null).map((r) => r.file!).toSet(); the _untestedFileRecord LF/LH/DA/SF format and the (pre-existing) prefix-based line filter ('part' still matching partition) are preserved verbatim.
• _runTests / _validateTarget extractions in both commands are mechanical and preserve exit-code semantics.

Observations (no action required)

• _runTests in DartTestCommand/TestCommand takes recursive even though it's only forwarded to TestCLIRunner.test — fine, keeps the extraction honest.
• The near-identical _validateTarget duplication between TestCommand and DartTestCommand is already called out in the PR description as a follow-up.

No correctness, security, or lifecycle issues found. The reasoning that existing tests already reach every new branch is believable given the refactor is purely structural, but CI should confirm.

[unicoderbot]

Heads up — I'm auto-updating this PR by merging the latest main into this branch. No action needed; I'll comment again only if this update hits a conflict or an unexpected error.