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

[marcossevilla]

Status

READY

Description

• TestCLIRunner.test moves its per-package body into _testPackage. Coverage settings are grouped in a private _CoverageOptions value and handled by _reportCoverage, _writeLcov, _convertDartCoverageToLcov and _checkCoverage.
• _testCommand hands each test event to a private _TestEventReporter that switches on the event type and keeps the tallies and failure list.
• _enhanceLcovWithUntestedFiles builds each record through _untestedFileRecord and _isCoverableLine.
• CoverageMetrics.fromLcovRecords filters excluded records and folds the rest through small helpers.
• TestCommand.run and DartTestCommand.run move their target checks into _validateTarget and their runner call into a helper, and the forwarded argument lists become private getters on the options classes.

One ordering change: for dart runs with collectCoverageFrom: all, the lcov.info existence assert now runs before the untested-files enhancement. The file already exists at that point, and asserts are off in release builds.

A follow-up could share the near-identical _validateTarget between TestCommand and DartTestCommand.

Found by the full cognitive complexity scan in #1755, which tests VeryGoodOpenSource/very_good_workflows#520. Every function in the touched files now scores 15 or under. Behavior, log output, exit codes and public APIs are unchanged, and no tests were needed because the existing ones already reach every new branch.

Function	Before	After
TestCLIRunner.test	66	1
_testCommand	66	8
TestCLIRunner._enhanceLcovWithUntestedFiles	17	4
TestCommand.run	30	4
CoverageMetrics.fromLcovRecords	28	0
DartTestCommand.run	21	4

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.