From fd7e16efe82ff74266585c48fb2f82b2012fbf69 Mon Sep 17 00:00:00 2001 From: "unicoderbot[bot]" <269805761+unicoderbot[bot]@users.noreply.github.com> Date: Thu, 2 Jul 2026 10:36:14 +0000 Subject: [PATCH 01/14] feat: support CLI parameter configuration via very_good.yaml Closes #360 Co-authored-by: marcossevilla --- lib/src/commands/test/test.dart | 110 +++++-- .../very_good_config/very_good_config.dart | 305 ++++++++++++++++++ site/docs/commands/test.md | 17 + test/src/commands/test/test_test.dart | 64 ++++ .../very_good_config_test.dart | 223 +++++++++++++ 5 files changed, 696 insertions(+), 23 deletions(-) create mode 100644 lib/src/very_good_config/very_good_config.dart create mode 100644 test/src/very_good_config/very_good_config_test.dart diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index cfef17a63..4c78cc452 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/very_good_config/very_good_config.dart'; /// Options for configuring the Flutter test command. class FlutterTestOptions { @@ -35,18 +36,50 @@ class FlutterTestOptions { }); /// Parses [ArgResults] into a [FlutterTestOptions] instance. - factory FlutterTestOptions.parse(ArgResults argResults) { - final concurrency = argResults['concurrency'] as String; - final collectCoverage = argResults['coverage'] as bool; - final minCoverage = double.tryParse( - argResults['min-coverage'] as String? ?? '', - ); - final showUncovered = argResults['show-uncovered'] as bool; - final excludeTags = argResults['exclude-tags'] as String?; - final tags = argResults['tags'] as String?; - final excludeFromCoverage = argResults['exclude-coverage'] as String?; + /// + /// When [config] is provided, its values are used as defaults for any + /// option that was not explicitly parsed on the command line. + factory FlutterTestOptions.parse( + ArgResults argResults, { + VeryGoodConfig config = VeryGoodConfig.empty, + }) { + final testConfig = config.test; + + T? configOverride(String name, T? value) { + if (value == null) return null; + if (argResults.wasParsed(name)) return null; + return value; + } + + final concurrency = + configOverride('concurrency', testConfig.concurrency) ?? + argResults['concurrency'] as String; + final collectCoverage = + configOverride('coverage', testConfig.coverage) ?? + argResults['coverage'] as bool; + final minCoverageString = + configOverride('min-coverage', testConfig.minCoverage) ?? + argResults['min-coverage'] as String?; + final minCoverage = double.tryParse(minCoverageString ?? ''); + final showUncovered = + configOverride('show-uncovered', testConfig.showUncovered) ?? + argResults['show-uncovered'] as bool; + final excludeTags = + configOverride('exclude-tags', testConfig.excludeTags) ?? + argResults['exclude-tags'] as String?; + final tags = + configOverride('tags', testConfig.tags) ?? + argResults['tags'] as String?; + final excludeFromCoverage = + configOverride('exclude-coverage', testConfig.excludeCoverage) ?? + argResults['exclude-coverage'] as String?; final collectCoverageFromString = - argResults['collect-coverage-from'] as String? ?? 'imports'; + configOverride( + 'collect-coverage-from', + testConfig.collectCoverageFrom, + ) ?? + argResults['collect-coverage-from'] as String? ?? + 'imports'; final collectCoverageFrom = CoverageCollectionMode.fromString( collectCoverageFromString, ); @@ -55,23 +88,44 @@ class FlutterTestOptions { final randomSeed = randomOrderingSeed == 'random' ? Random().nextInt(4294967295).toString() : randomOrderingSeed; - final optimizePerformance = argResults['optimization'] as bool; - final updateGoldens = argResults['update-goldens'] as bool; - final failFast = argResults['fail-fast'] as bool; + final optimizePerformance = + configOverride('optimization', testConfig.optimization) ?? + argResults['optimization'] as bool; + final updateGoldens = + configOverride('update-goldens', testConfig.updateGoldens) ?? + argResults['update-goldens'] as bool; + final failFast = + configOverride('fail-fast', testConfig.failFast) ?? + argResults['fail-fast'] as bool; final forceAnsi = argResults['force-ansi'] as bool?; - final dartDefine = argResults['dart-define'] as List?; + final dartDefine = + configOverride('dart-define', testConfig.dartDefine) ?? + argResults['dart-define'] as List?; final dartDefineFromFile = + configOverride( + 'dart-define-from-file', + testConfig.dartDefineFromFile, + ) ?? argResults['dart-define-from-file'] as List?; - final platform = argResults['platform'] as String?; - final reportOn = (argResults['report-on'] as List) + final platform = + configOverride('platform', testConfig.platform) ?? + argResults['platform'] as String?; + final reportOnFromArgs = argResults['report-on'] as List; + final reportOnFromConfig = configOverride('report-on', testConfig.reportOn); + final reportOn = (reportOnFromConfig ?? reportOnFromArgs) .expand((e) => e.split(RegExp(r'[,\s]+'))) .where((e) => e.isNotEmpty) .toList(); - final runSkipped = argResults['run-skipped'] as bool; - final flavor = argResults['flavor'] as String?; - final timeoutSeconds = int.tryParse( - argResults['timeout'] as String? ?? '', - ); + final runSkipped = + configOverride('run-skipped', testConfig.runSkipped) ?? + argResults['run-skipped'] as bool; + final flavor = + configOverride('flavor', testConfig.flavor) ?? + argResults['flavor'] as String?; + final timeoutString = + configOverride('timeout', testConfig.timeout) ?? + argResults['timeout'] as String?; + final timeoutSeconds = int.tryParse(timeoutString ?? ''); final timeout = timeoutSeconds != null ? Duration(seconds: timeoutSeconds) : null; @@ -382,9 +436,19 @@ This command should be run from the root of your Flutter project.'''); return ExitCode.noInput.code; } + final VeryGoodConfig config; + try { + config = VeryGoodConfig.loadFromDirectory(Directory(targetPath)); + } on VeryGoodConfigParseException catch (e) { + _logger.err( + 'Could not read `$veryGoodConfigFileName` in $targetPath.\n${e.message}', + ); + return ExitCode.config.code; + } + final isFlutterInstalled = await _flutterInstalled(logger: _logger); - final options = FlutterTestOptions.parse(_argResults); + final options = FlutterTestOptions.parse(_argResults, config: config); if (isFlutterInstalled) { try { diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart new file mode 100644 index 000000000..7c24b3e3e --- /dev/null +++ b/lib/src/very_good_config/very_good_config.dart @@ -0,0 +1,305 @@ +/// Support for loading Very Good CLI configuration from a +/// `very_good.yaml` file. +/// +/// The configuration file lives at the root of a project and allows +/// developers to persist frequently used CLI parameters (for example +/// `test` coverage excludes) so that running the CLI locally produces the +/// same results as running it on CI. +library; + +import 'package:equatable/equatable.dart'; +import 'package:path/path.dart' as p; +import 'package:universal_io/io.dart'; +import 'package:yaml/yaml.dart'; + +/// The default name of the Very Good CLI configuration file. +const veryGoodConfigFileName = 'very_good.yaml'; + +/// {@template very_good_config_parse_exception} +/// Thrown when a [VeryGoodConfig] fails to parse. +/// {@endtemplate} +class VeryGoodConfigParseException implements Exception { + /// {@macro very_good_config_parse_exception} + const VeryGoodConfigParseException(this.message); + + /// A human readable description of the parse failure. + final String message; + + @override + String toString() => 'VeryGoodConfigParseException: $message'; +} + +/// {@template very_good_config} +/// A representation of a `very_good.yaml` configuration file. +/// +/// The configuration file may declare per-command sections whose values +/// are used as defaults whenever the corresponding CLI flag is not +/// explicitly passed at the command line. +/// {@endtemplate} +class VeryGoodConfig extends Equatable { + /// {@macro very_good_config} + const VeryGoodConfig({this.test = const VeryGoodTestConfig()}); + + /// An empty [VeryGoodConfig] with no values set. + static const VeryGoodConfig empty = VeryGoodConfig(); + + /// Parses a [VeryGoodConfig] from a YAML [content] string. + /// + /// An empty or `null` YAML document yields [VeryGoodConfig.empty]. + /// + /// Throws a [VeryGoodConfigParseException] if [content] is not a valid + /// YAML map or if any known section is malformed. + factory VeryGoodConfig.fromString(String content) { + final dynamic loaded; + try { + loaded = loadYaml(content); + } on YamlException catch (e) { + throw VeryGoodConfigParseException('Failed to parse YAML: ${e.message}'); + } + + if (loaded == null) return VeryGoodConfig.empty; + if (loaded is! YamlMap) { + throw const VeryGoodConfigParseException( + 'The root of `very_good.yaml` must be a map.', + ); + } + + final testSection = loaded['test']; + final testConfig = testSection == null + ? const VeryGoodTestConfig() + : VeryGoodTestConfig.fromYaml(testSection); + + return VeryGoodConfig(test: testConfig); + } + + /// Loads a [VeryGoodConfig] from the given [directory]. + /// + /// Returns [VeryGoodConfig.empty] when the configuration file does not + /// exist. Throws a [VeryGoodConfigParseException] when the file exists + /// but cannot be parsed. + static VeryGoodConfig loadFromDirectory(Directory directory) { + final file = File(p.join(directory.path, veryGoodConfigFileName)); + if (!file.existsSync()) return VeryGoodConfig.empty; + return VeryGoodConfig.fromString(file.readAsStringSync()); + } + + /// Configuration values for the `very_good test` command. + final VeryGoodTestConfig test; + + @override + List get props => [test]; +} + +/// {@template very_good_test_config} +/// Configuration values that customize the defaults of the +/// `very_good test` command. +/// +/// Any field that is left as `null` retains its CLI default. +/// {@endtemplate} +class VeryGoodTestConfig extends Equatable { + /// {@macro very_good_test_config} + const VeryGoodTestConfig({ + this.coverage, + this.optimization, + this.concurrency, + this.tags, + this.excludeCoverage, + this.excludeTags, + this.minCoverage, + this.showUncovered, + this.collectCoverageFrom, + this.updateGoldens, + this.failFast, + this.dartDefine, + this.dartDefineFromFile, + this.platform, + this.reportOn, + this.runSkipped, + this.flavor, + this.timeout, + }); + + /// Parses a [VeryGoodTestConfig] from a YAML node. + /// + /// Throws a [VeryGoodConfigParseException] if the [yaml] node is not a + /// map or if any known field has an unsupported type. + factory VeryGoodTestConfig.fromYaml(dynamic yaml) { + if (yaml is! YamlMap) { + throw const VeryGoodConfigParseException( + 'The `test` section of `very_good.yaml` must be a map.', + ); + } + + return VeryGoodTestConfig( + coverage: _readBool(yaml, 'coverage'), + optimization: _readBool(yaml, 'optimization'), + concurrency: _readIntAsString(yaml, 'concurrency'), + tags: _readString(yaml, 'tags'), + excludeCoverage: _readString(yaml, 'exclude-coverage'), + excludeTags: _readString(yaml, 'exclude-tags'), + minCoverage: _readNumAsString(yaml, 'min-coverage'), + showUncovered: _readBool(yaml, 'show-uncovered'), + collectCoverageFrom: _readCollectCoverageFrom(yaml), + updateGoldens: _readBool(yaml, 'update-goldens'), + failFast: _readBool(yaml, 'fail-fast'), + dartDefine: _readStringList(yaml, 'dart-define'), + dartDefineFromFile: _readStringList(yaml, 'dart-define-from-file'), + platform: _readString(yaml, 'platform'), + reportOn: _readStringList(yaml, 'report-on'), + runSkipped: _readBool(yaml, 'run-skipped'), + flavor: _readString(yaml, 'flavor'), + timeout: _readIntAsString(yaml, 'timeout'), + ); + } + + /// Whether to collect coverage information. + final bool? coverage; + + /// Whether to apply optimizations for test performance. + final bool? optimization; + + /// The number of concurrent test suites run. + final String? concurrency; + + /// 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. + final String? excludeCoverage; + + /// Run only tests that do not have the specified tags. + final String? excludeTags; + + /// The minimum coverage percentage enforced. + final String? minCoverage; + + /// Whether to show uncovered lines when coverage is below 100%. + final bool? showUncovered; + + /// Whether to collect coverage from imported files only or all files. + final String? collectCoverageFrom; + + /// Whether `matchesGoldenFile()` calls should update the golden files. + final bool? updateGoldens; + + /// Whether to stop running tests after the first failure. + final bool? failFast; + + /// Additional `--dart-define` values. + final List? dartDefine; + + /// Paths of `.json` or `.env` files with `--dart-define-from-file` values. + final List? dartDefineFromFile; + + /// The platform to run tests on (e.g. `chrome`, `vm`, `android`, `ios`). + final String? platform; + + /// Optional file paths to report coverage information to. + final List? reportOn; + + /// Whether to run skipped tests instead of skipping them. + final bool? runSkipped; + + /// The flavor to build for testing. + final String? flavor; + + /// Maximum seconds to let tests run before killing the process. + final String? timeout; + + @override + List get props => [ + coverage, + optimization, + concurrency, + tags, + excludeCoverage, + excludeTags, + minCoverage, + showUncovered, + collectCoverageFrom, + updateGoldens, + failFast, + dartDefine, + dartDefineFromFile, + platform, + reportOn, + runSkipped, + flavor, + timeout, + ]; +} + +bool? _readBool(YamlMap yaml, String key) { + if (!yaml.containsKey(key)) return null; + final value = yaml[key]; + if (value is! bool) { + throw VeryGoodConfigParseException( + 'Expected a boolean value for `$key` but got `$value`.', + ); + } + return value; +} + +String? _readString(YamlMap yaml, String key) { + if (!yaml.containsKey(key)) return null; + final value = yaml[key]; + if (value is! String) { + throw VeryGoodConfigParseException( + 'Expected a string value for `$key` but got `$value`.', + ); + } + return value; +} + +String? _readIntAsString(YamlMap yaml, String key) { + if (!yaml.containsKey(key)) return null; + final value = yaml[key]; + if (value is int) return value.toString(); + if (value is String) return value; + throw VeryGoodConfigParseException( + 'Expected an integer or string value for `$key` but got `$value`.', + ); +} + +String? _readNumAsString(YamlMap yaml, String key) { + if (!yaml.containsKey(key)) return null; + final value = yaml[key]; + if (value is num) return value.toString(); + if (value is String) return value; + throw VeryGoodConfigParseException( + 'Expected a number or string value for `$key` but got `$value`.', + ); +} + +String? _readCollectCoverageFrom(YamlMap yaml) { + const key = 'collect-coverage-from'; + final value = _readString(yaml, key); + if (value == null) return null; + if (value != 'imports' && value != 'all') { + throw VeryGoodConfigParseException( + 'Expected `$key` to be `imports` or `all` but got `$value`.', + ); + } + return value; +} + +List? _readStringList(YamlMap yaml, String key) { + if (!yaml.containsKey(key)) return null; + final value = yaml[key]; + if (value is String) return [value]; + if (value is YamlList) { + return value + .map((dynamic e) { + if (e is! String) { + throw VeryGoodConfigParseException( + 'Expected every entry of `$key` to be a string but got `$e`.', + ); + } + return e; + }) + .toList(growable: false); + } + throw VeryGoodConfigParseException( + 'Expected a string or list of strings for `$key` but got `$value`.', + ); +} diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index 80a90e897..b24f629e9 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -92,3 +92,20 @@ By default, all tests run with optimizations enabled; use the `--no-optimization import 'package:test/test.dart'; ``` + +### Configuring defaults with `very_good.yaml` + +To avoid repeating flags every time you run `very_good test` locally or on CI, you may create a `very_good.yaml` file at the root of your project. The `test` section accepts the same names as the CLI flags (kebab-case). Values from `very_good.yaml` are used as defaults; anything you pass on the command line takes precedence. + +```yaml +# very_good.yaml +test: + min-coverage: 100 + exclude-coverage: "**/*.g.dart" + report-on: + - lib/ + dart-define: + - FLAVOR=development +``` + +With the file above, running `very_good test` behaves the same as running `very_good test --min-coverage 100 --exclude-coverage '**/*.g.dart' --report-on lib/ --dart-define=FLAVOR=development`. You can still override any of these values on the command line, for example `very_good test --min-coverage 90` to lower the coverage threshold for a single run. diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 5e6e466ea..9bb10036c 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/very_good_config/very_good_config.dart'; import '../../../helpers/helpers.dart'; @@ -910,5 +911,68 @@ void main() { }, ); }); + + group('very_good.yaml configuration', () { + test( + 'fails with exit code ${ExitCode.config.code} ' + 'when very_good.yaml is malformed', + withRunner((commandRunner, logger, pubUpdater, printLogs) async { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() { + Directory.current = cwd; + tempDirectory.deleteSync(recursive: true); + }); + + Directory.current = tempDirectory.path; + File(path.join(tempDirectory.path, 'pubspec.yaml')).createSync(); + File( + path.join(tempDirectory.path, 'very_good.yaml'), + ).writeAsStringSync('- not\n- a\n- map'); + + final result = await commandRunner.run(['test']); + expect(result, equals(ExitCode.config.code)); + verify( + () => logger.err( + any(that: contains('Could not read `very_good.yaml`')), + ), + ).called(1); + }), + ); + + test('applies config value when arg was not parsed', () async { + when(() => argResults.wasParsed(any())).thenReturn(false); + final result = await testCommand.run(); + expect(result, equals(ExitCode.success.code)); + // Provide the config directly via FlutterTestOptions.parse to + // verify precedence rules without touching the filesystem. + final options = FlutterTestOptions.parse( + argResults, + config: const VeryGoodConfig( + test: VeryGoodTestConfig( + minCoverage: '90', + excludeCoverage: '**/*.g.dart', + reportOn: ['lib/'], + ), + ), + ); + expect(options.minCoverage, 90); + expect(options.excludeFromCoverage, '**/*.g.dart'); + expect(options.reportOn, equals(['lib/'])); + }); + + test('CLI argument takes precedence over config value', () async { + when(() => argResults.wasParsed('min-coverage')).thenReturn(true); + when(() => argResults.wasParsed(any())).thenReturn(false); + when(() => argResults['min-coverage']).thenReturn('50'); + + final options = FlutterTestOptions.parse( + argResults, + config: const VeryGoodConfig( + test: VeryGoodTestConfig(minCoverage: '90'), + ), + ); + expect(options.minCoverage, 50); + }); + }); }); } diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart new file mode 100644 index 000000000..720fbd194 --- /dev/null +++ b/test/src/very_good_config/very_good_config_test.dart @@ -0,0 +1,223 @@ +// Ensures we don't have to use const constructors +// and instances are created at runtime. +// ignore_for_file: prefer_const_constructors + +import 'dart:io'; + +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; +import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; + +void main() { + group('$VeryGoodConfig', () { + group('fromString', () { + test('returns empty config when content is empty', () { + expect(VeryGoodConfig.fromString(''), equals(VeryGoodConfig.empty)); + }); + + test('returns empty config when content is null-only', () { + expect(VeryGoodConfig.fromString('~'), equals(VeryGoodConfig.empty)); + }); + + test('throws $VeryGoodConfigParseException when root is not a map', () { + expect( + () => VeryGoodConfig.fromString('- foo\n- bar'), + throwsA(isA()), + ); + }); + + test('throws $VeryGoodConfigParseException when yaml is malformed', () { + expect( + () => VeryGoodConfig.fromString(':\n:'), + throwsA(isA()), + ); + }); + + test('parses all supported test options', () { + final config = VeryGoodConfig.fromString(''' +test: + coverage: true + optimization: false + concurrency: 8 + tags: my-tag + exclude-coverage: "**/*.g.dart" + exclude-tags: skip + min-coverage: 95 + show-uncovered: true + collect-coverage-from: all + update-goldens: true + fail-fast: true + dart-define: + - FOO=bar + - X=42 + dart-define-from-file: defines.env + platform: chrome + report-on: + - lib/ + - packages/foo/lib/ + run-skipped: true + flavor: staging + timeout: 30 +'''); + + expect(config.test.coverage, isTrue); + expect(config.test.optimization, isFalse); + expect(config.test.concurrency, '8'); + expect(config.test.tags, 'my-tag'); + expect(config.test.excludeCoverage, '**/*.g.dart'); + expect(config.test.excludeTags, 'skip'); + expect(config.test.minCoverage, '95'); + expect(config.test.showUncovered, isTrue); + expect(config.test.collectCoverageFrom, 'all'); + expect(config.test.updateGoldens, isTrue); + expect(config.test.failFast, isTrue); + expect(config.test.dartDefine, equals(['FOO=bar', 'X=42'])); + expect(config.test.dartDefineFromFile, equals(['defines.env'])); + expect(config.test.platform, 'chrome'); + expect(config.test.reportOn, equals(['lib/', 'packages/foo/lib/'])); + expect(config.test.runSkipped, isTrue); + expect(config.test.flavor, 'staging'); + expect(config.test.timeout, '30'); + }); + + test('parses min-coverage as decimal string', () { + final config = VeryGoodConfig.fromString(''' +test: + min-coverage: 95.5 +'''); + expect(config.test.minCoverage, '95.5'); + }); + + test('throws when test section is not a map', () { + expect( + () => VeryGoodConfig.fromString('test: foo'), + throwsA(isA()), + ); + }); + + test('throws when bool option has wrong type', () { + expect( + () => VeryGoodConfig.fromString('test:\n coverage: yes-please'), + throwsA(isA()), + ); + }); + + test('throws when string option has wrong type', () { + expect( + () => VeryGoodConfig.fromString('test:\n tags: [a, b]'), + throwsA(isA()), + ); + }); + + test('throws when integer option has wrong type', () { + expect( + () => VeryGoodConfig.fromString('test:\n concurrency: [1]'), + throwsA(isA()), + ); + }); + + test('throws when number option has wrong type', () { + expect( + () => VeryGoodConfig.fromString('test:\n min-coverage: [95]'), + throwsA(isA()), + ); + }); + + test('throws when collect-coverage-from has invalid value', () { + expect( + () => + VeryGoodConfig.fromString('test:\n collect-coverage-from: bad'), + throwsA(isA()), + ); + }); + + test('throws when string list has non-string entries', () { + expect( + () => VeryGoodConfig.fromString('test:\n dart-define:\n - 42'), + throwsA(isA()), + ); + }); + + test('throws when string list has wrong type', () { + expect( + () => VeryGoodConfig.fromString('test:\n report-on: 42'), + throwsA(isA()), + ); + }); + }); + + group('loadFromDirectory', () { + late Directory tempDir; + + setUp(() { + tempDir = Directory.systemTemp.createTempSync('very_good_config_'); + }); + + tearDown(() { + if (tempDir.existsSync()) { + tempDir.deleteSync(recursive: true); + } + }); + + test('returns empty config when file is missing', () { + expect( + VeryGoodConfig.loadFromDirectory(tempDir), + equals(VeryGoodConfig.empty), + ); + }); + + test('reads config file when present', () { + File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' +test: + min-coverage: 90 +'''); + final config = VeryGoodConfig.loadFromDirectory(tempDir); + expect(config.test.minCoverage, '90'); + }); + + test('rethrows parse exception when file is malformed', () { + File( + p.join(tempDir.path, veryGoodConfigFileName), + ).writeAsStringSync('- not\n- a\n- map'); + expect( + () => VeryGoodConfig.loadFromDirectory(tempDir), + throwsA(isA()), + ); + }); + }); + + test('supports value equality', () { + expect(VeryGoodConfig(), equals(VeryGoodConfig())); + expect( + VeryGoodConfig(test: VeryGoodTestConfig(coverage: true)), + equals(VeryGoodConfig(test: VeryGoodTestConfig(coverage: true))), + ); + expect( + VeryGoodConfig(test: VeryGoodTestConfig(coverage: true)), + isNot( + equals(VeryGoodConfig(test: VeryGoodTestConfig(coverage: false))), + ), + ); + }); + }); + + group('$VeryGoodTestConfig', () { + test('supports value equality', () { + expect( + VeryGoodTestConfig(coverage: true, minCoverage: '95'), + equals(VeryGoodTestConfig(coverage: true, minCoverage: '95')), + ); + expect( + VeryGoodTestConfig(coverage: true), + isNot(equals(VeryGoodTestConfig(coverage: false))), + ); + }); + }); + + group('$VeryGoodConfigParseException', () { + test('provides message via toString', () { + const exception = VeryGoodConfigParseException('bad thing'); + expect(exception.toString(), contains('bad thing')); + }); + }); +} From b548f2311863f2bc1e23ed85042293ac2e6194f5 Mon Sep 17 00:00:00 2001 From: "unicoderbot[bot]" <269805761+unicoderbot[bot]@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:16:51 +0000 Subject: [PATCH 02/14] fix: address review feedback on PR #1636 Co-authored-by: marcossevilla --- .github/workflows/e2e.yaml | 1 + .../fixture/lib/uncovered.dart | 1 + .../very_good_config/fixture/pubspec.yaml | 10 ++ .../fixture/test/covered_test.dart | 9 ++ .../very_good_config/fixture/very_good.yaml | 2 + .../very_good_config_test.dart | 127 ++++++++++++++++++ test/src/commands/test/test_test.dart | 2 +- .../very_good_config_test.dart | 26 ++++ 8 files changed, 177 insertions(+), 1 deletion(-) create mode 100644 e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart create mode 100644 e2e/test/commands/test/very_good_config/fixture/pubspec.yaml create mode 100644 e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart create mode 100644 e2e/test/commands/test/very_good_config/fixture/very_good.yaml create mode 100644 e2e/test/commands/test/very_good_config/very_good_config_test.dart diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index cc7bba843..f77584dea 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -33,6 +33,7 @@ jobs: - test/commands/test/compilation_error/compilation_error_test.dart - test/commands/test/no_project/no_project_test.dart - test/commands/test/spaced_golden_file_name/spaced_golden_file_name_test.dart + - test/commands/test/very_good_config/very_good_config_test.dart # E2E tests for the create command - test/commands/create/flutter_app/core_test.dart diff --git a/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart b/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart new file mode 100644 index 000000000..36c51a5b7 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart @@ -0,0 +1 @@ +int untestedFunction(int value) => value * 2; diff --git a/e2e/test/commands/test/very_good_config/fixture/pubspec.yaml b/e2e/test/commands/test/very_good_config/fixture/pubspec.yaml new file mode 100644 index 000000000..d98f01c24 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/pubspec.yaml @@ -0,0 +1,10 @@ +name: very_good_config_fixture +description: Fixture for testing very_good.yaml configuration. +version: 0.1.0+1 +publish_to: none + +environment: + sdk: ^3.12.0 + +dev_dependencies: + test: ^1.24.3 diff --git a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart new file mode 100644 index 000000000..150a80be2 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart @@ -0,0 +1,9 @@ +import 'package:test/test.dart'; + +void main() { + group('very_good_config_fixture', () { + test('trivially succeeds', () { + expect(true, isTrue); + }); + }); +} diff --git a/e2e/test/commands/test/very_good_config/fixture/very_good.yaml b/e2e/test/commands/test/very_good_config/fixture/very_good.yaml new file mode 100644 index 000000000..550b2df50 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/very_good.yaml @@ -0,0 +1,2 @@ +test: + min-coverage: 100 diff --git a/e2e/test/commands/test/very_good_config/very_good_config_test.dart b/e2e/test/commands/test/very_good_config/very_good_config_test.dart new file mode 100644 index 000000000..a2219e9e2 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/very_good_config_test.dart @@ -0,0 +1,127 @@ +import 'package:mason/mason.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; +import 'package:universal_io/io.dart'; + +import '../../../../helpers/helpers.dart'; + +void main() { + group('very_good.yaml', () { + test( + 'enforces min-coverage from very_good.yaml when the flag is not passed', + timeout: const Timeout(Duration(minutes: 2)), + withRunner((commandRunner, logger, updater, logs, progressLogs) async { + final tempDirectory = Directory.systemTemp.createTempSync( + 'very_good_config', + ); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final fixture = Directory( + path.join( + Directory.current.path, + 'test/commands/test/very_good_config/fixture', + ), + ); + + await copyDirectory(fixture, tempDirectory); + + await expectSuccessfulProcessResult('flutter', [ + 'pub', + 'get', + ], workingDirectory: tempDirectory.path); + + final cwd = Directory.current; + Directory.current = tempDirectory; + addTearDown(() => Directory.current = cwd); + + final result = await commandRunner.run(['test', '--coverage']); + + expect(result, equals(ExitCode.unavailable.code)); + verify( + () => logger.err(any(that: contains('Expected coverage >= 100.00%'))), + ).called(1); + }), + ); + + test( + 'CLI --min-coverage takes precedence over very_good.yaml', + timeout: const Timeout(Duration(minutes: 2)), + withRunner((commandRunner, logger, updater, logs, progressLogs) async { + final tempDirectory = Directory.systemTemp.createTempSync( + 'very_good_config_override', + ); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final fixture = Directory( + path.join( + Directory.current.path, + 'test/commands/test/very_good_config/fixture', + ), + ); + + await copyDirectory(fixture, tempDirectory); + + await expectSuccessfulProcessResult('flutter', [ + 'pub', + 'get', + ], workingDirectory: tempDirectory.path); + + final cwd = Directory.current; + Directory.current = tempDirectory; + addTearDown(() => Directory.current = cwd); + + final result = await commandRunner.run([ + 'test', + '--coverage', + '--min-coverage', + '0', + ]); + + expect(result, equals(ExitCode.success.code)); + }), + ); + + test( + 'fails with config exit code when very_good.yaml is malformed', + timeout: const Timeout(Duration(minutes: 2)), + withRunner((commandRunner, logger, updater, logs, progressLogs) async { + final tempDirectory = Directory.systemTemp.createTempSync( + 'very_good_config_malformed', + ); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + File(path.join(tempDirectory.path, 'pubspec.yaml')).writeAsStringSync( + ''' +name: malformed_fixture +description: Fixture for testing malformed very_good.yaml. +version: 0.1.0+1 +publish_to: none + +environment: + sdk: ^3.12.0 + +dev_dependencies: + test: ^1.24.3 +''', + ); + File( + path.join(tempDirectory.path, 'very_good.yaml'), + ).writeAsStringSync('- not\n- a\n- map'); + + final cwd = Directory.current; + Directory.current = tempDirectory; + addTearDown(() => Directory.current = cwd); + + final result = await commandRunner.run(['test']); + + expect(result, equals(ExitCode.config.code)); + verify( + () => logger.err( + any(that: contains('Could not read `very_good.yaml`')), + ), + ).called(1); + }), + ); + }); +} diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index 9bb10036c..eb3f2aa71 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -961,8 +961,8 @@ void main() { }); test('CLI argument takes precedence over config value', () async { - when(() => argResults.wasParsed('min-coverage')).thenReturn(true); when(() => argResults.wasParsed(any())).thenReturn(false); + when(() => argResults.wasParsed('min-coverage')).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('50'); final options = FlutterTestOptions.parse( diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart index 720fbd194..32bea5ae3 100644 --- a/test/src/very_good_config/very_good_config_test.dart +++ b/test/src/very_good_config/very_good_config_test.dart @@ -88,6 +88,32 @@ test: expect(config.test.minCoverage, '95.5'); }); + test('parses integer options provided as quoted strings', () { + final config = VeryGoodConfig.fromString(''' +test: + concurrency: "8" + timeout: "60" +'''); + expect(config.test.concurrency, '8'); + expect(config.test.timeout, '60'); + }); + + test('parses min-coverage provided as a quoted string', () { + final config = VeryGoodConfig.fromString(''' +test: + min-coverage: "95" +'''); + expect(config.test.minCoverage, '95'); + }); + + test('parses collect-coverage-from with value `imports`', () { + final config = VeryGoodConfig.fromString(''' +test: + collect-coverage-from: imports +'''); + expect(config.test.collectCoverageFrom, 'imports'); + }); + test('throws when test section is not a map', () { expect( () => VeryGoodConfig.fromString('test: foo'), From e8cabe6a7d408e64ccdc165b085b6b516a3f32bf Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Thu, 2 Jul 2026 16:17:44 +0200 Subject: [PATCH 03/14] chore: use checked_yaml and json_serializable --- .github/workflows/very_good_cli.yaml | 6 +- analysis_options.yaml | 1 + build.yaml | 8 + lib/src/commands/test/test.dart | 117 +++++------ .../very_good_config/very_good_config.dart | 191 ++++++++---------- .../very_good_config/very_good_config.g.dart | 67 ++++++ pubspec.yaml | 3 + 7 files changed, 211 insertions(+), 182 deletions(-) create mode 100644 build.yaml create mode 100644 lib/src/very_good_config/very_good_config.g.dart diff --git a/.github/workflows/very_good_cli.yaml b/.github/workflows/very_good_cli.yaml index 493837649..3ed8b1fc6 100644 --- a/.github/workflows/very_good_cli.yaml +++ b/.github/workflows/very_good_cli.yaml @@ -29,12 +29,12 @@ jobs: os: [ubuntu-latest, windows-latest] uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/flutter_package.yml@v1 with: + runs_on: ${{ matrix.os }} flutter_version: "3.44.x" + coverage_excludes: "**/*.{gen,g}.dart" concurrency: 1 - coverage_excludes: "**/*.gen.dart" - run_bloc_lint: false run_skipped: true - runs_on: ${{ matrix.os }} + run_bloc_lint: false test_optimization: false pana: diff --git a/analysis_options.yaml b/analysis_options.yaml index 9834b46d4..363528c1b 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -1,5 +1,6 @@ include: package:very_good_analysis/analysis_options.yaml analyzer: exclude: + - "**/*.g.dart" - "**/version.dart" - "bricks/**/__brick__" diff --git a/build.yaml b/build.yaml new file mode 100644 index 000000000..9186cc335 --- /dev/null +++ b/build.yaml @@ -0,0 +1,8 @@ +targets: + $default: + sources: + - $package$ + - lib/** + - bin/** + - test/** + - pubspec.yaml diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 4c78cc452..eb24bbc21 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -45,87 +45,65 @@ class FlutterTestOptions { }) { final testConfig = config.test; - T? configOverride(String name, T? value) { - if (value == null) return null; - if (argResults.wasParsed(name)) return null; - return value; + T resolve(String name, T? configValue, {T? fallbackValue}) { + final value = configValue != null && !argResults.wasParsed(name) + ? configValue + : argResults[name] as T?; + return (value ?? fallbackValue) as T; } - final concurrency = - configOverride('concurrency', testConfig.concurrency) ?? - argResults['concurrency'] as String; - final collectCoverage = - configOverride('coverage', testConfig.coverage) ?? - argResults['coverage'] as bool; - final minCoverageString = - configOverride('min-coverage', testConfig.minCoverage) ?? - argResults['min-coverage'] as String?; - final minCoverage = double.tryParse(minCoverageString ?? ''); - final showUncovered = - configOverride('show-uncovered', testConfig.showUncovered) ?? - argResults['show-uncovered'] as bool; - final excludeTags = - configOverride('exclude-tags', testConfig.excludeTags) ?? - argResults['exclude-tags'] as String?; - final tags = - configOverride('tags', testConfig.tags) ?? - argResults['tags'] as String?; - final excludeFromCoverage = - configOverride('exclude-coverage', testConfig.excludeCoverage) ?? - argResults['exclude-coverage'] as String?; - final collectCoverageFromString = - configOverride( - 'collect-coverage-from', - testConfig.collectCoverageFrom, - ) ?? - argResults['collect-coverage-from'] as String? ?? - 'imports'; + final concurrency = resolve('concurrency', testConfig.concurrency); + final collectCoverage = resolve('coverage', testConfig.coverage); + final minCoverage = double.tryParse( + resolve('min-coverage', testConfig.minCoverage) ?? '', + ); + final showUncovered = resolve('show-uncovered', testConfig.showUncovered); + final excludeTags = resolve( + 'exclude-tags', + testConfig.excludeTags, + ); + final tags = resolve('tags', testConfig.tags); + final excludeFromCoverage = resolve( + 'exclude-coverage', + testConfig.excludeCoverage, + ); final collectCoverageFrom = CoverageCollectionMode.fromString( - collectCoverageFromString, + resolve( + 'collect-coverage-from', + testConfig.collectCoverageFrom, + fallbackValue: 'imports', + ), ); final randomOrderingSeed = argResults['test-randomize-ordering-seed'] as String?; final randomSeed = randomOrderingSeed == 'random' ? Random().nextInt(4294967295).toString() : randomOrderingSeed; - final optimizePerformance = - configOverride('optimization', testConfig.optimization) ?? - argResults['optimization'] as bool; - final updateGoldens = - configOverride('update-goldens', testConfig.updateGoldens) ?? - argResults['update-goldens'] as bool; - final failFast = - configOverride('fail-fast', testConfig.failFast) ?? - argResults['fail-fast'] as bool; + final optimizePerformance = resolve( + 'optimization', + testConfig.optimization, + ); + final updateGoldens = resolve('update-goldens', testConfig.updateGoldens); + final failFast = resolve('fail-fast', testConfig.failFast); final forceAnsi = argResults['force-ansi'] as bool?; - final dartDefine = - configOverride('dart-define', testConfig.dartDefine) ?? - argResults['dart-define'] as List?; - final dartDefineFromFile = - configOverride( - 'dart-define-from-file', - testConfig.dartDefineFromFile, - ) ?? - argResults['dart-define-from-file'] as List?; - final platform = - configOverride('platform', testConfig.platform) ?? - argResults['platform'] as String?; - final reportOnFromArgs = argResults['report-on'] as List; - final reportOnFromConfig = configOverride('report-on', testConfig.reportOn); - final reportOn = (reportOnFromConfig ?? reportOnFromArgs) + final dartDefine = resolve?>( + 'dart-define', + testConfig.dartDefine, + ); + final dartDefineFromFile = resolve?>( + 'dart-define-from-file', + testConfig.dartDefineFromFile, + ); + final platform = resolve('platform', testConfig.platform); + final reportOn = resolve>('report-on', testConfig.reportOn) .expand((e) => e.split(RegExp(r'[,\s]+'))) .where((e) => e.isNotEmpty) .toList(); - final runSkipped = - configOverride('run-skipped', testConfig.runSkipped) ?? - argResults['run-skipped'] as bool; - final flavor = - configOverride('flavor', testConfig.flavor) ?? - argResults['flavor'] as String?; - final timeoutString = - configOverride('timeout', testConfig.timeout) ?? - argResults['timeout'] as String?; - final timeoutSeconds = int.tryParse(timeoutString ?? ''); + final runSkipped = resolve('run-skipped', testConfig.runSkipped); + final flavor = resolve('flavor', testConfig.flavor); + final timeoutSeconds = int.tryParse( + resolve('timeout', testConfig.timeout, fallbackValue: ''), + ); final timeout = timeoutSeconds != null ? Duration(seconds: timeoutSeconds) : null; @@ -441,7 +419,8 @@ This command should be run from the root of your Flutter project.'''); config = VeryGoodConfig.loadFromDirectory(Directory(targetPath)); } on VeryGoodConfigParseException catch (e) { _logger.err( - 'Could not read `$veryGoodConfigFileName` in $targetPath.\n${e.message}', + 'Could not read `$veryGoodConfigFileName` in $targetPath.\n' + '${e.message}', ); return ExitCode.config.code; } diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 7c24b3e3e..ebeac5100 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -1,16 +1,19 @@ /// Support for loading Very Good CLI configuration from a /// `very_good.yaml` file. /// -/// The configuration file lives at the root of a project and allows -/// developers to persist frequently used CLI parameters (for example -/// `test` coverage excludes) so that running the CLI locally produces the -/// same results as running it on CI. +/// The configuration file lives at the root of a project and allows developers +/// to persist frequently used CLI parameters so that running the CLI locally +/// produces the same results as running it on CI. library; +import 'dart:io'; + +import 'package:checked_yaml/checked_yaml.dart'; import 'package:equatable/equatable.dart'; -import 'package:path/path.dart' as p; -import 'package:universal_io/io.dart'; -import 'package:yaml/yaml.dart'; +import 'package:json_annotation/json_annotation.dart'; +import 'package:path/path.dart' as path; + +part 'very_good_config.g.dart'; /// The default name of the Very Good CLI configuration file. const veryGoodConfigFileName = 'very_good.yaml'; @@ -36,40 +39,41 @@ class VeryGoodConfigParseException implements Exception { /// are used as defaults whenever the corresponding CLI flag is not /// explicitly passed at the command line. /// {@endtemplate} +@JsonSerializable( + anyMap: true, + checked: true, + createToJson: false, + fieldRename: FieldRename.kebab, +) class VeryGoodConfig extends Equatable { /// {@macro very_good_config} const VeryGoodConfig({this.test = const VeryGoodTestConfig()}); - /// An empty [VeryGoodConfig] with no values set. - static const VeryGoodConfig empty = VeryGoodConfig(); + /// Creates a [VeryGoodConfig] from a decoded YAML/JSON [json] map. + factory VeryGoodConfig.fromJson(Map json) { + return _$VeryGoodConfigFromJson(json); + } /// Parses a [VeryGoodConfig] from a YAML [content] string. /// /// An empty or `null` YAML document yields [VeryGoodConfig.empty]. /// + /// When provided, [sourceUrl] is used to enrich error messages with the + /// location the [content] originated from. + /// /// Throws a [VeryGoodConfigParseException] if [content] is not a valid /// YAML map or if any known section is malformed. - factory VeryGoodConfig.fromString(String content) { - final dynamic loaded; + factory VeryGoodConfig.fromString(String content, {Uri? sourceUrl}) { try { - loaded = loadYaml(content); - } on YamlException catch (e) { - throw VeryGoodConfigParseException('Failed to parse YAML: ${e.message}'); - } - - if (loaded == null) return VeryGoodConfig.empty; - if (loaded is! YamlMap) { - throw const VeryGoodConfigParseException( - 'The root of `very_good.yaml` must be a map.', + return checkedYamlDecode( + content, + (json) => VeryGoodConfig.fromJson(json ?? const {}), + allowNull: true, + sourceUrl: sourceUrl, ); + } on ParsedYamlException catch (e) { + throw VeryGoodConfigParseException(e.formattedMessage ?? e.message); } - - final testSection = loaded['test']; - final testConfig = testSection == null - ? const VeryGoodTestConfig() - : VeryGoodTestConfig.fromYaml(testSection); - - return VeryGoodConfig(test: testConfig); } /// Loads a [VeryGoodConfig] from the given [directory]. @@ -77,12 +81,18 @@ class VeryGoodConfig extends Equatable { /// Returns [VeryGoodConfig.empty] when the configuration file does not /// exist. Throws a [VeryGoodConfigParseException] when the file exists /// but cannot be parsed. - static VeryGoodConfig loadFromDirectory(Directory directory) { - final file = File(p.join(directory.path, veryGoodConfigFileName)); + factory VeryGoodConfig.loadFromDirectory(Directory directory) { + final file = File(path.join(directory.path, veryGoodConfigFileName)); if (!file.existsSync()) return VeryGoodConfig.empty; - return VeryGoodConfig.fromString(file.readAsStringSync()); + return VeryGoodConfig.fromString( + file.readAsStringSync(), + sourceUrl: file.uri, + ); } + /// An empty [VeryGoodConfig] with no values set. + static const VeryGoodConfig empty = VeryGoodConfig(); + /// Configuration values for the `very_good test` command. final VeryGoodTestConfig test; @@ -96,6 +106,12 @@ class VeryGoodConfig extends Equatable { /// /// Any field that is left as `null` retains its CLI default. /// {@endtemplate} +@JsonSerializable( + anyMap: true, + checked: true, + createToJson: false, + fieldRename: FieldRename.kebab, +) class VeryGoodTestConfig extends Equatable { /// {@macro very_good_test_config} const VeryGoodTestConfig({ @@ -119,37 +135,9 @@ class VeryGoodTestConfig extends Equatable { this.timeout, }); - /// Parses a [VeryGoodTestConfig] from a YAML node. - /// - /// Throws a [VeryGoodConfigParseException] if the [yaml] node is not a - /// map or if any known field has an unsupported type. - factory VeryGoodTestConfig.fromYaml(dynamic yaml) { - if (yaml is! YamlMap) { - throw const VeryGoodConfigParseException( - 'The `test` section of `very_good.yaml` must be a map.', - ); - } - - return VeryGoodTestConfig( - coverage: _readBool(yaml, 'coverage'), - optimization: _readBool(yaml, 'optimization'), - concurrency: _readIntAsString(yaml, 'concurrency'), - tags: _readString(yaml, 'tags'), - excludeCoverage: _readString(yaml, 'exclude-coverage'), - excludeTags: _readString(yaml, 'exclude-tags'), - minCoverage: _readNumAsString(yaml, 'min-coverage'), - showUncovered: _readBool(yaml, 'show-uncovered'), - collectCoverageFrom: _readCollectCoverageFrom(yaml), - updateGoldens: _readBool(yaml, 'update-goldens'), - failFast: _readBool(yaml, 'fail-fast'), - dartDefine: _readStringList(yaml, 'dart-define'), - dartDefineFromFile: _readStringList(yaml, 'dart-define-from-file'), - platform: _readString(yaml, 'platform'), - reportOn: _readStringList(yaml, 'report-on'), - runSkipped: _readBool(yaml, 'run-skipped'), - flavor: _readString(yaml, 'flavor'), - timeout: _readIntAsString(yaml, 'timeout'), - ); + /// Creates a [VeryGoodTestConfig] from a decoded YAML/JSON [json] map. + factory VeryGoodTestConfig.fromJson(Map json) { + return _$VeryGoodTestConfigFromJson(json); } /// Whether to collect coverage information. @@ -159,6 +147,7 @@ class VeryGoodTestConfig extends Equatable { final bool? optimization; /// The number of concurrent test suites run. + @JsonKey(fromJson: _intAsString) final String? concurrency; /// Run only tests associated with the specified tags. @@ -171,12 +160,14 @@ class VeryGoodTestConfig extends Equatable { final String? excludeTags; /// The minimum coverage percentage enforced. + @JsonKey(fromJson: _numAsString) final String? minCoverage; /// Whether to show uncovered lines when coverage is below 100%. final bool? showUncovered; /// Whether to collect coverage from imported files only or all files. + @JsonKey(fromJson: _collectCoverageFrom) final String? collectCoverageFrom; /// Whether `matchesGoldenFile()` calls should update the golden files. @@ -186,15 +177,18 @@ class VeryGoodTestConfig extends Equatable { final bool? failFast; /// Additional `--dart-define` values. + @JsonKey(fromJson: _stringList) final List? dartDefine; /// Paths of `.json` or `.env` files with `--dart-define-from-file` values. + @JsonKey(fromJson: _stringList) final List? dartDefineFromFile; /// The platform to run tests on (e.g. `chrome`, `vm`, `android`, `ios`). final String? platform; /// Optional file paths to report coverage information to. + @JsonKey(fromJson: _stringList) final List? reportOn; /// Whether to run skipped tests instead of skipping them. @@ -204,6 +198,7 @@ class VeryGoodTestConfig extends Equatable { final String? flavor; /// Maximum seconds to let tests run before killing the process. + @JsonKey(fromJson: _intAsString) final String? timeout; @override @@ -229,77 +224,53 @@ class VeryGoodTestConfig extends Equatable { ]; } -bool? _readBool(YamlMap yaml, String key) { - if (!yaml.containsKey(key)) return null; - final value = yaml[key]; - if (value is! bool) { - throw VeryGoodConfigParseException( - 'Expected a boolean value for `$key` but got `$value`.', - ); - } - return value; -} - -String? _readString(YamlMap yaml, String key) { - if (!yaml.containsKey(key)) return null; - final value = yaml[key]; - if (value is! String) { - throw VeryGoodConfigParseException( - 'Expected a string value for `$key` but got `$value`.', - ); - } - return value; -} - -String? _readIntAsString(YamlMap yaml, String key) { - if (!yaml.containsKey(key)) return null; - final value = yaml[key]; +/// Coerces an `int` or `String` value into a `String`. +/// +/// Options are stored as strings to match the CLI's argument parsing (which +/// always yields strings) but are naturally written as integers in YAML. +String? _intAsString(Object? value) { + if (value == null) return null; if (value is int) return value.toString(); if (value is String) return value; - throw VeryGoodConfigParseException( - 'Expected an integer or string value for `$key` but got `$value`.', - ); + throw FormatException('Expected an integer or string but got `$value`.'); } -String? _readNumAsString(YamlMap yaml, String key) { - if (!yaml.containsKey(key)) return null; - final value = yaml[key]; +/// Coerces a `num` or `String` value into a `String`. +String? _numAsString(Object? value) { + if (value == null) return null; if (value is num) return value.toString(); if (value is String) return value; - throw VeryGoodConfigParseException( - 'Expected a number or string value for `$key` but got `$value`.', - ); + throw FormatException('Expected a number or string but got `$value`.'); } -String? _readCollectCoverageFrom(YamlMap yaml) { - const key = 'collect-coverage-from'; - final value = _readString(yaml, key); +/// Validates and returns the `collect-coverage-from` value. +/// +/// Accepts only `imports` or `all`. +String? _collectCoverageFrom(Object? value) { if (value == null) return null; if (value != 'imports' && value != 'all') { - throw VeryGoodConfigParseException( - 'Expected `$key` to be `imports` or `all` but got `$value`.', - ); + throw FormatException('Expected `imports` or `all` but got `$value`.'); } - return value; + return value as String; } -List? _readStringList(YamlMap yaml, String key) { - if (!yaml.containsKey(key)) return null; - final value = yaml[key]; +/// Coerces a single string or a list of strings into a `List`. +List? _stringList(Object? value) { + if (value == null) return null; if (value is String) return [value]; - if (value is YamlList) { + if (value is List) { return value .map((dynamic e) { if (e is! String) { - throw VeryGoodConfigParseException( - 'Expected every entry of `$key` to be a string but got `$e`.', + throw FormatException( + 'Expected every entry to be a string but got `$e`.', ); } return e; }) .toList(growable: false); } - throw VeryGoodConfigParseException( - 'Expected a string or list of strings for `$key` but got `$value`.', + throw FormatException( + 'Expected a string or list of strings but got `$value`.', ); } diff --git a/lib/src/very_good_config/very_good_config.g.dart b/lib/src/very_good_config/very_good_config.g.dart new file mode 100644 index 000000000..7210a5b6c --- /dev/null +++ b/lib/src/very_good_config/very_good_config.g.dart @@ -0,0 +1,67 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +part of 'very_good_config.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +VeryGoodConfig _$VeryGoodConfigFromJson(Map json) => + $checkedCreate('VeryGoodConfig', json, ($checkedConvert) { + final val = VeryGoodConfig( + test: $checkedConvert( + 'test', + (v) => v == null + ? const VeryGoodTestConfig() + : VeryGoodTestConfig.fromJson(v as Map), + ), + ); + return val; + }); + +VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( + 'VeryGoodTestConfig', + json, + ($checkedConvert) { + final val = VeryGoodTestConfig( + coverage: $checkedConvert('coverage', (v) => v as bool?), + optimization: $checkedConvert('optimization', (v) => v as bool?), + concurrency: $checkedConvert('concurrency', (v) => _intAsString(v)), + tags: $checkedConvert('tags', (v) => v as String?), + excludeCoverage: $checkedConvert('exclude-coverage', (v) => v as String?), + excludeTags: $checkedConvert('exclude-tags', (v) => v as String?), + minCoverage: $checkedConvert('min-coverage', (v) => _numAsString(v)), + showUncovered: $checkedConvert('show-uncovered', (v) => v as bool?), + collectCoverageFrom: $checkedConvert( + 'collect-coverage-from', + (v) => _collectCoverageFrom(v), + ), + updateGoldens: $checkedConvert('update-goldens', (v) => v as bool?), + failFast: $checkedConvert('fail-fast', (v) => v as bool?), + dartDefine: $checkedConvert('dart-define', (v) => _stringList(v)), + dartDefineFromFile: $checkedConvert( + 'dart-define-from-file', + (v) => _stringList(v), + ), + platform: $checkedConvert('platform', (v) => v as String?), + reportOn: $checkedConvert('report-on', (v) => _stringList(v)), + runSkipped: $checkedConvert('run-skipped', (v) => v as bool?), + flavor: $checkedConvert('flavor', (v) => v as String?), + timeout: $checkedConvert('timeout', (v) => _intAsString(v)), + ); + return val; + }, + fieldKeyMap: const { + 'excludeCoverage': 'exclude-coverage', + 'excludeTags': 'exclude-tags', + 'minCoverage': 'min-coverage', + 'showUncovered': 'show-uncovered', + 'collectCoverageFrom': 'collect-coverage-from', + 'updateGoldens': 'update-goldens', + 'failFast': 'fail-fast', + 'dartDefine': 'dart-define', + 'dartDefineFromFile': 'dart-define-from-file', + 'reportOn': 'report-on', + 'runSkipped': 'run-skipped', + }, +); diff --git a/pubspec.yaml b/pubspec.yaml index 5883f57b0..6d535e6ef 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -12,12 +12,14 @@ environment: dependencies: args: ^2.6.0 + checked_yaml: ^2.0.3 cli_completion: ^0.5.1 collection: ^1.19.0 coverage: ^1.15.0 dart_mcp: ^0.5.0 equatable: ^2.0.5 glob: ^2.1.2 + json_annotation: ^4.9.0 lcov_parser: ^0.1.3 mason: ^0.1.0 mason_logger: ^0.3.0 @@ -36,6 +38,7 @@ dependencies: dev_dependencies: build_runner: ^2.4.12 build_verify: ^3.1.0 + json_serializable: ^6.8.0 mocktail: ^1.0.4 test: ^1.25.8 very_good_analysis: ^10.3.0 From ae1829837a588755d69f15fa6ad7ded1b4dda32e Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Thu, 2 Jul 2026 16:22:32 +0200 Subject: [PATCH 04/14] chore: simplify --- lib/src/commands/test/test.dart | 2 +- lib/src/very_good_config/very_good_config.dart | 16 ++++------------ lib/src/very_good_config/very_good_config.g.dart | 4 ++-- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index eb24bbc21..215ffbd27 100644 --- a/lib/src/commands/test/test.dart +++ b/lib/src/commands/test/test.dart @@ -102,7 +102,7 @@ class FlutterTestOptions { final runSkipped = resolve('run-skipped', testConfig.runSkipped); final flavor = resolve('flavor', testConfig.flavor); final timeoutSeconds = int.tryParse( - resolve('timeout', testConfig.timeout, fallbackValue: ''), + resolve('timeout', testConfig.timeout) ?? '', ); final timeout = timeoutSeconds != null ? Duration(seconds: timeoutSeconds) diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index ebeac5100..2095ad9d9 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -147,7 +147,7 @@ class VeryGoodTestConfig extends Equatable { final bool? optimization; /// The number of concurrent test suites run. - @JsonKey(fromJson: _intAsString) + @JsonKey(fromJson: _numAsString) final String? concurrency; /// Run only tests associated with the specified tags. @@ -198,7 +198,7 @@ class VeryGoodTestConfig extends Equatable { final String? flavor; /// Maximum seconds to let tests run before killing the process. - @JsonKey(fromJson: _intAsString) + @JsonKey(fromJson: _numAsString) final String? timeout; @override @@ -224,18 +224,10 @@ class VeryGoodTestConfig extends Equatable { ]; } -/// Coerces an `int` or `String` value into a `String`. +/// Coerces a `num` or `String` value into a `String`. /// /// Options are stored as strings to match the CLI's argument parsing (which -/// always yields strings) but are naturally written as integers in YAML. -String? _intAsString(Object? value) { - if (value == null) return null; - if (value is int) return value.toString(); - if (value is String) return value; - throw FormatException('Expected an integer or string but got `$value`.'); -} - -/// Coerces a `num` or `String` value into a `String`. +/// always yields strings) but are naturally written as numbers in YAML. String? _numAsString(Object? value) { if (value == null) return null; if (value is num) return value.toString(); diff --git a/lib/src/very_good_config/very_good_config.g.dart b/lib/src/very_good_config/very_good_config.g.dart index 7210a5b6c..276036b72 100644 --- a/lib/src/very_good_config/very_good_config.g.dart +++ b/lib/src/very_good_config/very_good_config.g.dart @@ -26,7 +26,7 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( final val = VeryGoodTestConfig( coverage: $checkedConvert('coverage', (v) => v as bool?), optimization: $checkedConvert('optimization', (v) => v as bool?), - concurrency: $checkedConvert('concurrency', (v) => _intAsString(v)), + concurrency: $checkedConvert('concurrency', (v) => _numAsString(v)), tags: $checkedConvert('tags', (v) => v as String?), excludeCoverage: $checkedConvert('exclude-coverage', (v) => v as String?), excludeTags: $checkedConvert('exclude-tags', (v) => v as String?), @@ -47,7 +47,7 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( reportOn: $checkedConvert('report-on', (v) => _stringList(v)), runSkipped: $checkedConvert('run-skipped', (v) => v as bool?), flavor: $checkedConvert('flavor', (v) => v as String?), - timeout: $checkedConvert('timeout', (v) => _intAsString(v)), + timeout: $checkedConvert('timeout', (v) => _numAsString(v)), ); return val; }, From deff17e90e904cc56b801d6daef4219d226d6736 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Thu, 2 Jul 2026 16:49:06 +0200 Subject: [PATCH 05/14] refactor: add config resolver --- lib/src/commands/test/test.dart | 56 +++---- lib/src/very_good_config/config_resolver.dart | 33 +++++ .../very_good_config/very_good_config.dart | 75 +++++++++- .../very_good_config/very_good_config.g.dart | 30 +++- .../config_resolver_test.dart | 58 ++++++++ .../very_good_config_test.dart | 138 ++++++++++++++++++ 6 files changed, 357 insertions(+), 33 deletions(-) create mode 100644 lib/src/very_good_config/config_resolver.dart create mode 100644 test/src/very_good_config/config_resolver_test.dart diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index 215ffbd27..c615e96c7 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/very_good_config/config_resolver.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Flutter test command. @@ -44,31 +45,28 @@ class FlutterTestOptions { VeryGoodConfig config = VeryGoodConfig.empty, }) { final testConfig = config.test; + final resolver = ConfigResolver(argResults); - T resolve(String name, T? configValue, {T? fallbackValue}) { - final value = configValue != null && !argResults.wasParsed(name) - ? configValue - : argResults[name] as T?; - return (value ?? fallbackValue) as T; - } - - final concurrency = resolve('concurrency', testConfig.concurrency); - final collectCoverage = resolve('coverage', testConfig.coverage); + final concurrency = resolver.resolve('concurrency', testConfig.concurrency); + final collectCoverage = resolver.resolve('coverage', testConfig.coverage); final minCoverage = double.tryParse( - resolve('min-coverage', testConfig.minCoverage) ?? '', + resolver.resolve('min-coverage', testConfig.minCoverage) ?? '', ); - final showUncovered = resolve('show-uncovered', testConfig.showUncovered); - final excludeTags = resolve( + final showUncovered = resolver.resolve( + 'show-uncovered', + testConfig.showUncovered, + ); + final excludeTags = resolver.resolve( 'exclude-tags', testConfig.excludeTags, ); - final tags = resolve('tags', testConfig.tags); - final excludeFromCoverage = resolve( + final tags = resolver.resolve('tags', testConfig.tags); + final excludeFromCoverage = resolver.resolve( 'exclude-coverage', testConfig.excludeCoverage, ); final collectCoverageFrom = CoverageCollectionMode.fromString( - resolve( + resolver.resolve( 'collect-coverage-from', testConfig.collectCoverageFrom, fallbackValue: 'imports', @@ -79,30 +77,34 @@ class FlutterTestOptions { final randomSeed = randomOrderingSeed == 'random' ? Random().nextInt(4294967295).toString() : randomOrderingSeed; - final optimizePerformance = resolve( + final optimizePerformance = resolver.resolve( 'optimization', testConfig.optimization, ); - final updateGoldens = resolve('update-goldens', testConfig.updateGoldens); - final failFast = resolve('fail-fast', testConfig.failFast); + final updateGoldens = resolver.resolve( + 'update-goldens', + testConfig.updateGoldens, + ); + final failFast = resolver.resolve('fail-fast', testConfig.failFast); final forceAnsi = argResults['force-ansi'] as bool?; - final dartDefine = resolve?>( + final dartDefine = resolver.resolve?>( 'dart-define', testConfig.dartDefine, ); - final dartDefineFromFile = resolve?>( + final dartDefineFromFile = resolver.resolve?>( 'dart-define-from-file', testConfig.dartDefineFromFile, ); - final platform = resolve('platform', testConfig.platform); - final reportOn = resolve>('report-on', testConfig.reportOn) + final platform = resolver.resolve('platform', testConfig.platform); + final reportOn = resolver + .resolve>('report-on', testConfig.reportOn) .expand((e) => e.split(RegExp(r'[,\s]+'))) .where((e) => e.isNotEmpty) .toList(); - final runSkipped = resolve('run-skipped', testConfig.runSkipped); - final flavor = resolve('flavor', testConfig.flavor); + final runSkipped = resolver.resolve('run-skipped', testConfig.runSkipped); + final flavor = resolver.resolve('flavor', testConfig.flavor); final timeoutSeconds = int.tryParse( - resolve('timeout', testConfig.timeout) ?? '', + resolver.resolve('timeout', testConfig.timeout) ?? '', ); final timeout = timeoutSeconds != null ? Duration(seconds: timeoutSeconds) @@ -416,10 +418,10 @@ This command should be run from the root of your Flutter project.'''); final VeryGoodConfig config; try { - config = VeryGoodConfig.loadFromDirectory(Directory(targetPath)); + config = VeryGoodConfig.loadFromClosestAncestor(Directory(targetPath)); } on VeryGoodConfigParseException catch (e) { _logger.err( - 'Could not read `$veryGoodConfigFileName` in $targetPath.\n' + 'Could not read `$veryGoodConfigFileName`.\n' '${e.message}', ); return ExitCode.config.code; diff --git a/lib/src/very_good_config/config_resolver.dart b/lib/src/very_good_config/config_resolver.dart new file mode 100644 index 000000000..cf9df4f19 --- /dev/null +++ b/lib/src/very_good_config/config_resolver.dart @@ -0,0 +1,33 @@ +import 'package:args/args.dart'; + +/// {@template config_resolver} +/// Merges command line [ArgResults] with `very_good.yaml` configuration values. +/// +/// Resolution follows a fixed precedence, from highest to lowest: +/// +/// 1. A command line argument that was explicitly parsed. +/// 2. A value declared in the configuration file. +/// 3. A fallback value (typically the argument's command line default). +/// +/// Centralizing the rule here keeps every command's argument/configuration +/// merge consistent and independently testable. +/// {@endtemplate} +class ConfigResolver { + /// {@macro config_resolver} + const ConfigResolver(this.argResults); + + /// The parsed command line arguments. + final ArgResults argResults; + + /// Resolves the value for the argument named [name]. + /// + /// [configValue] is the corresponding value from the configuration file, if + /// any. [fallbackValue] is used when neither the command line nor the + /// configuration provide a value. + T resolve(String name, T? configValue, {T? fallbackValue}) { + final value = configValue != null && !argResults.wasParsed(name) + ? configValue + : argResults[name] as T?; + return (value ?? fallbackValue) as T; + } +} diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 2095ad9d9..19961e23d 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -43,6 +43,7 @@ class VeryGoodConfigParseException implements Exception { anyMap: true, checked: true, createToJson: false, + disallowUnrecognizedKeys: true, fieldRename: FieldRename.kebab, ) class VeryGoodConfig extends Equatable { @@ -90,6 +91,32 @@ class VeryGoodConfig extends Equatable { ); } + /// Loads the closest [VeryGoodConfig] by searching [directory] and each of + /// its ancestors, from the innermost directory outward. + /// + /// This allows a single repository-wide `very_good.yaml` at the project root + /// to apply to commands run from any nested package directory. The first + /// configuration file encountered wins; ancestors are not merged. + /// + /// Returns [VeryGoodConfig.empty] when no configuration file is found. + /// Throws a [VeryGoodConfigParseException] when the closest file exists but + /// cannot be parsed. + factory VeryGoodConfig.loadFromClosestAncestor(Directory directory) { + var current = directory.absolute; + while (true) { + final file = File(path.join(current.path, veryGoodConfigFileName)); + if (file.existsSync()) { + return VeryGoodConfig.fromString( + file.readAsStringSync(), + sourceUrl: file.uri, + ); + } + final parent = current.parent; + if (parent.path == current.path) return VeryGoodConfig.empty; + current = parent; + } + } + /// An empty [VeryGoodConfig] with no values set. static const VeryGoodConfig empty = VeryGoodConfig(); @@ -110,6 +137,7 @@ class VeryGoodConfig extends Equatable { anyMap: true, checked: true, createToJson: false, + disallowUnrecognizedKeys: true, fieldRename: FieldRename.kebab, ) class VeryGoodTestConfig extends Equatable { @@ -147,7 +175,7 @@ class VeryGoodTestConfig extends Equatable { final bool? optimization; /// The number of concurrent test suites run. - @JsonKey(fromJson: _numAsString) + @JsonKey(fromJson: _concurrency) final String? concurrency; /// Run only tests associated with the specified tags. @@ -160,7 +188,7 @@ class VeryGoodTestConfig extends Equatable { final String? excludeTags; /// The minimum coverage percentage enforced. - @JsonKey(fromJson: _numAsString) + @JsonKey(fromJson: _minCoverage) final String? minCoverage; /// Whether to show uncovered lines when coverage is below 100%. @@ -198,7 +226,7 @@ class VeryGoodTestConfig extends Equatable { final String? flavor; /// Maximum seconds to let tests run before killing the process. - @JsonKey(fromJson: _numAsString) + @JsonKey(fromJson: _timeout) final String? timeout; @override @@ -235,6 +263,47 @@ String? _numAsString(Object? value) { throw FormatException('Expected a number or string but got `$value`.'); } +/// Coerces and validates a positive integer option stored as a string. +/// +/// [key] is the option name used to enrich the error message. +String? _positiveInt(Object? value, String key) { + final asString = _numAsString(value); + if (asString == null) return null; + final parsed = int.tryParse(asString); + if (parsed == null || parsed < 1) { + throw FormatException( + 'Expected `$key` to be a positive integer but got `$asString`.', + ); + } + return asString; +} + +/// Validates and returns the `concurrency` value. +/// +/// Accepts only positive integers. +String? _concurrency(Object? value) => _positiveInt(value, 'concurrency'); + +/// Validates and returns the `timeout` value. +/// +/// Accepts only positive integers (seconds). +String? _timeout(Object? value) => _positiveInt(value, 'timeout'); + +/// Validates and returns the `min-coverage` value. +/// +/// Accepts only a number between 0 and 100 (inclusive). +String? _minCoverage(Object? value) { + final asString = _numAsString(value); + if (asString == null) return null; + final parsed = double.tryParse(asString); + if (parsed == null || parsed < 0 || parsed > 100) { + throw FormatException( + 'Expected `min-coverage` to be a number between 0 and 100 ' + 'but got `$asString`.', + ); + } + return asString; +} + /// Validates and returns the `collect-coverage-from` value. /// /// Accepts only `imports` or `all`. diff --git a/lib/src/very_good_config/very_good_config.g.dart b/lib/src/very_good_config/very_good_config.g.dart index 276036b72..ea8437d06 100644 --- a/lib/src/very_good_config/very_good_config.g.dart +++ b/lib/src/very_good_config/very_good_config.g.dart @@ -8,6 +8,7 @@ part of 'very_good_config.dart'; VeryGoodConfig _$VeryGoodConfigFromJson(Map json) => $checkedCreate('VeryGoodConfig', json, ($checkedConvert) { + $checkKeys(json, allowedKeys: const ['test']); final val = VeryGoodConfig( test: $checkedConvert( 'test', @@ -23,14 +24,37 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( 'VeryGoodTestConfig', json, ($checkedConvert) { + $checkKeys( + json, + allowedKeys: const [ + 'coverage', + 'optimization', + 'concurrency', + 'tags', + 'exclude-coverage', + 'exclude-tags', + 'min-coverage', + 'show-uncovered', + 'collect-coverage-from', + 'update-goldens', + 'fail-fast', + 'dart-define', + 'dart-define-from-file', + 'platform', + 'report-on', + 'run-skipped', + 'flavor', + 'timeout', + ], + ); final val = VeryGoodTestConfig( coverage: $checkedConvert('coverage', (v) => v as bool?), optimization: $checkedConvert('optimization', (v) => v as bool?), - concurrency: $checkedConvert('concurrency', (v) => _numAsString(v)), + concurrency: $checkedConvert('concurrency', (v) => _concurrency(v)), tags: $checkedConvert('tags', (v) => v as String?), excludeCoverage: $checkedConvert('exclude-coverage', (v) => v as String?), excludeTags: $checkedConvert('exclude-tags', (v) => v as String?), - minCoverage: $checkedConvert('min-coverage', (v) => _numAsString(v)), + minCoverage: $checkedConvert('min-coverage', (v) => _minCoverage(v)), showUncovered: $checkedConvert('show-uncovered', (v) => v as bool?), collectCoverageFrom: $checkedConvert( 'collect-coverage-from', @@ -47,7 +71,7 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( reportOn: $checkedConvert('report-on', (v) => _stringList(v)), runSkipped: $checkedConvert('run-skipped', (v) => v as bool?), flavor: $checkedConvert('flavor', (v) => v as String?), - timeout: $checkedConvert('timeout', (v) => _numAsString(v)), + timeout: $checkedConvert('timeout', (v) => _timeout(v)), ); return val; }, diff --git a/test/src/very_good_config/config_resolver_test.dart b/test/src/very_good_config/config_resolver_test.dart new file mode 100644 index 000000000..5c57d207f --- /dev/null +++ b/test/src/very_good_config/config_resolver_test.dart @@ -0,0 +1,58 @@ +// Ensures we don't have to use const constructors +// and instances are created at runtime. +// ignore_for_file: prefer_const_constructors + +import 'package:args/args.dart'; +import 'package:mocktail/mocktail.dart'; +import 'package:test/test.dart'; +import 'package:very_good_cli/src/very_good_config/config_resolver.dart'; + +class _MockArgResults extends Mock implements ArgResults {} + +void main() { + group('$ConfigResolver', () { + late ArgResults argResults; + late ConfigResolver resolver; + + setUp(() { + argResults = _MockArgResults(); + resolver = ConfigResolver(argResults); + }); + + test('uses the config value when the argument was not parsed', () { + when(() => argResults.wasParsed('min-coverage')).thenReturn(false); + + expect(resolver.resolve('min-coverage', '90'), '90'); + }); + + test('uses the parsed argument over the config value', () { + when(() => argResults.wasParsed('min-coverage')).thenReturn(true); + when(() => argResults['min-coverage']).thenReturn('50'); + + expect(resolver.resolve('min-coverage', '90'), '50'); + }); + + test('uses the argument value when there is no config value', () { + when(() => argResults.wasParsed('tags')).thenReturn(false); + when(() => argResults['tags']).thenReturn('unit'); + + expect(resolver.resolve('tags', null), 'unit'); + }); + + test('falls back when neither argument nor config provide a value', () { + when( + () => argResults.wasParsed('collect-coverage-from'), + ).thenReturn(false); + when(() => argResults['collect-coverage-from']).thenReturn(null); + + expect( + resolver.resolve( + 'collect-coverage-from', + null, + fallbackValue: 'imports', + ), + 'imports', + ); + }); + }); +} diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart index 32bea5ae3..30d9ecf2f 100644 --- a/test/src/very_good_config/very_good_config_test.dart +++ b/test/src/very_good_config/very_good_config_test.dart @@ -170,6 +170,75 @@ test: throwsA(isA()), ); }); + + test('throws when an unrecognized root key is present', () { + expect( + () => VeryGoodConfig.fromString('unknown: true'), + throwsA(isA()), + ); + }); + + test('throws when an unrecognized test key is present', () { + expect( + () => VeryGoodConfig.fromString('test:\n min-coverag: 80'), + throwsA(isA()), + ); + }); + + test('throws when min-coverage is below 0', () { + expect( + () => VeryGoodConfig.fromString('test:\n min-coverage: -1'), + throwsA(isA()), + ); + }); + + test('throws when min-coverage is above 100', () { + expect( + () => VeryGoodConfig.fromString('test:\n min-coverage: 101'), + throwsA(isA()), + ); + }); + + test('parses min-coverage at the boundaries', () { + expect( + VeryGoodConfig.fromString( + 'test:\n min-coverage: 0', + ).test.minCoverage, + '0', + ); + expect( + VeryGoodConfig.fromString( + 'test:\n min-coverage: 100', + ).test.minCoverage, + '100', + ); + }); + + test('throws when concurrency is not a positive integer', () { + expect( + () => VeryGoodConfig.fromString('test:\n concurrency: 0'), + throwsA(isA()), + ); + expect( + () => VeryGoodConfig.fromString('test:\n concurrency: -3'), + throwsA(isA()), + ); + expect( + () => VeryGoodConfig.fromString('test:\n concurrency: 1.5'), + throwsA(isA()), + ); + }); + + test('throws when timeout is not a positive integer', () { + expect( + () => VeryGoodConfig.fromString('test:\n timeout: 0'), + throwsA(isA()), + ); + expect( + () => VeryGoodConfig.fromString('test:\n timeout: -30'), + throwsA(isA()), + ); + }); }); group('loadFromDirectory', () { @@ -212,6 +281,75 @@ test: }); }); + group('loadFromClosestAncestor', () { + late Directory tempDir; + late Directory nestedDir; + + setUp(() { + tempDir = Directory.systemTemp.createTempSync('very_good_config_'); + nestedDir = Directory(p.join(tempDir.path, 'packages', 'foo')) + ..createSync(recursive: true); + }); + + tearDown(() { + if (tempDir.existsSync()) { + tempDir.deleteSync(recursive: true); + } + }); + + test('reads config from the starting directory', () { + File(p.join(nestedDir.path, veryGoodConfigFileName)).writeAsStringSync( + ''' +test: + min-coverage: 80 +''', + ); + final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); + expect(config.test.minCoverage, '80'); + }); + + test('reads config from an ancestor directory', () { + File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' +test: + min-coverage: 90 +'''); + final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); + expect(config.test.minCoverage, '90'); + }); + + test('prefers the closest config over an ancestor', () { + File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' +test: + min-coverage: 90 +'''); + File(p.join(nestedDir.path, veryGoodConfigFileName)).writeAsStringSync( + ''' +test: + min-coverage: 80 +''', + ); + final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); + expect(config.test.minCoverage, '80'); + }); + + test('returns empty config when no file is found in any ancestor', () { + expect( + VeryGoodConfig.loadFromClosestAncestor(nestedDir), + equals(VeryGoodConfig.empty), + ); + }); + + test('rethrows parse exception when the closest file is malformed', () { + File( + p.join(nestedDir.path, veryGoodConfigFileName), + ).writeAsStringSync('- not\n- a\n- map'); + expect( + () => VeryGoodConfig.loadFromClosestAncestor(nestedDir), + throwsA(isA()), + ); + }); + }); + test('supports value equality', () { expect(VeryGoodConfig(), equals(VeryGoodConfig())); expect( From c72f0186c47fbdb7462fbaca50c18768321f00d8 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Fri, 3 Jul 2026 12:07:21 +0200 Subject: [PATCH 06/14] refactor: rename from snake_case --- .../very_good_config/very_good_config.dart | 10 +-- .../very_good_config/very_good_config.g.dart | 66 +++++++++---------- .../very_good_config_test.dart | 56 ++++++++-------- 3 files changed, 66 insertions(+), 66 deletions(-) diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 19961e23d..2b6ff49b4 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -44,7 +44,7 @@ class VeryGoodConfigParseException implements Exception { checked: true, createToJson: false, disallowUnrecognizedKeys: true, - fieldRename: FieldRename.kebab, + fieldRename: FieldRename.snake, ) class VeryGoodConfig extends Equatable { /// {@macro very_good_config} @@ -138,7 +138,7 @@ class VeryGoodConfig extends Equatable { checked: true, createToJson: false, disallowUnrecognizedKeys: true, - fieldRename: FieldRename.kebab, + fieldRename: FieldRename.snake, ) class VeryGoodTestConfig extends Equatable { /// {@macro very_good_test_config} @@ -288,7 +288,7 @@ String? _concurrency(Object? value) => _positiveInt(value, 'concurrency'); /// Accepts only positive integers (seconds). String? _timeout(Object? value) => _positiveInt(value, 'timeout'); -/// Validates and returns the `min-coverage` value. +/// Validates and returns the `min_coverage` value. /// /// Accepts only a number between 0 and 100 (inclusive). String? _minCoverage(Object? value) { @@ -297,14 +297,14 @@ String? _minCoverage(Object? value) { final parsed = double.tryParse(asString); if (parsed == null || parsed < 0 || parsed > 100) { throw FormatException( - 'Expected `min-coverage` to be a number between 0 and 100 ' + 'Expected `min_coverage` to be a number between 0 and 100 ' 'but got `$asString`.', ); } return asString; } -/// Validates and returns the `collect-coverage-from` value. +/// Validates and returns the `collect_coverage_from` value. /// /// Accepts only `imports` or `all`. String? _collectCoverageFrom(Object? value) { diff --git a/lib/src/very_good_config/very_good_config.g.dart b/lib/src/very_good_config/very_good_config.g.dart index ea8437d06..a772995e7 100644 --- a/lib/src/very_good_config/very_good_config.g.dart +++ b/lib/src/very_good_config/very_good_config.g.dart @@ -31,18 +31,18 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( 'optimization', 'concurrency', 'tags', - 'exclude-coverage', - 'exclude-tags', - 'min-coverage', - 'show-uncovered', - 'collect-coverage-from', - 'update-goldens', - 'fail-fast', - 'dart-define', - 'dart-define-from-file', + 'exclude_coverage', + 'exclude_tags', + 'min_coverage', + 'show_uncovered', + 'collect_coverage_from', + 'update_goldens', + 'fail_fast', + 'dart_define', + 'dart_define_from_file', 'platform', - 'report-on', - 'run-skipped', + 'report_on', + 'run_skipped', 'flavor', 'timeout', ], @@ -52,40 +52,40 @@ VeryGoodTestConfig _$VeryGoodTestConfigFromJson(Map json) => $checkedCreate( optimization: $checkedConvert('optimization', (v) => v as bool?), concurrency: $checkedConvert('concurrency', (v) => _concurrency(v)), tags: $checkedConvert('tags', (v) => v as String?), - excludeCoverage: $checkedConvert('exclude-coverage', (v) => v as String?), - excludeTags: $checkedConvert('exclude-tags', (v) => v as String?), - minCoverage: $checkedConvert('min-coverage', (v) => _minCoverage(v)), - showUncovered: $checkedConvert('show-uncovered', (v) => v as bool?), + excludeCoverage: $checkedConvert('exclude_coverage', (v) => v as String?), + excludeTags: $checkedConvert('exclude_tags', (v) => v as String?), + minCoverage: $checkedConvert('min_coverage', (v) => _minCoverage(v)), + showUncovered: $checkedConvert('show_uncovered', (v) => v as bool?), collectCoverageFrom: $checkedConvert( - 'collect-coverage-from', + 'collect_coverage_from', (v) => _collectCoverageFrom(v), ), - updateGoldens: $checkedConvert('update-goldens', (v) => v as bool?), - failFast: $checkedConvert('fail-fast', (v) => v as bool?), - dartDefine: $checkedConvert('dart-define', (v) => _stringList(v)), + updateGoldens: $checkedConvert('update_goldens', (v) => v as bool?), + failFast: $checkedConvert('fail_fast', (v) => v as bool?), + dartDefine: $checkedConvert('dart_define', (v) => _stringList(v)), dartDefineFromFile: $checkedConvert( - 'dart-define-from-file', + 'dart_define_from_file', (v) => _stringList(v), ), platform: $checkedConvert('platform', (v) => v as String?), - reportOn: $checkedConvert('report-on', (v) => _stringList(v)), - runSkipped: $checkedConvert('run-skipped', (v) => v as bool?), + reportOn: $checkedConvert('report_on', (v) => _stringList(v)), + runSkipped: $checkedConvert('run_skipped', (v) => v as bool?), flavor: $checkedConvert('flavor', (v) => v as String?), timeout: $checkedConvert('timeout', (v) => _timeout(v)), ); return val; }, fieldKeyMap: const { - 'excludeCoverage': 'exclude-coverage', - 'excludeTags': 'exclude-tags', - 'minCoverage': 'min-coverage', - 'showUncovered': 'show-uncovered', - 'collectCoverageFrom': 'collect-coverage-from', - 'updateGoldens': 'update-goldens', - 'failFast': 'fail-fast', - 'dartDefine': 'dart-define', - 'dartDefineFromFile': 'dart-define-from-file', - 'reportOn': 'report-on', - 'runSkipped': 'run-skipped', + 'excludeCoverage': 'exclude_coverage', + 'excludeTags': 'exclude_tags', + 'minCoverage': 'min_coverage', + 'showUncovered': 'show_uncovered', + 'collectCoverageFrom': 'collect_coverage_from', + 'updateGoldens': 'update_goldens', + 'failFast': 'fail_fast', + 'dartDefine': 'dart_define', + 'dartDefineFromFile': 'dart_define_from_file', + 'reportOn': 'report_on', + 'runSkipped': 'run_skipped', }, ); diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart index 30d9ecf2f..ede147c31 100644 --- a/test/src/very_good_config/very_good_config_test.dart +++ b/test/src/very_good_config/very_good_config_test.dart @@ -40,22 +40,22 @@ test: optimization: false concurrency: 8 tags: my-tag - exclude-coverage: "**/*.g.dart" - exclude-tags: skip - min-coverage: 95 - show-uncovered: true - collect-coverage-from: all - update-goldens: true - fail-fast: true - dart-define: + exclude_coverage: "**/*.g.dart" + exclude_tags: skip + min_coverage: 95 + show_uncovered: true + collect_coverage_from: all + update_goldens: true + fail_fast: true + dart_define: - FOO=bar - X=42 - dart-define-from-file: defines.env + dart_define_from_file: defines.env platform: chrome - report-on: + report_on: - lib/ - packages/foo/lib/ - run-skipped: true + run_skipped: true flavor: staging timeout: 30 '''); @@ -83,7 +83,7 @@ test: test('parses min-coverage as decimal string', () { final config = VeryGoodConfig.fromString(''' test: - min-coverage: 95.5 + min_coverage: 95.5 '''); expect(config.test.minCoverage, '95.5'); }); @@ -101,7 +101,7 @@ test: test('parses min-coverage provided as a quoted string', () { final config = VeryGoodConfig.fromString(''' test: - min-coverage: "95" + min_coverage: "95" '''); expect(config.test.minCoverage, '95'); }); @@ -109,7 +109,7 @@ test: test('parses collect-coverage-from with value `imports`', () { final config = VeryGoodConfig.fromString(''' test: - collect-coverage-from: imports + collect_coverage_from: imports '''); expect(config.test.collectCoverageFrom, 'imports'); }); @@ -144,7 +144,7 @@ test: test('throws when number option has wrong type', () { expect( - () => VeryGoodConfig.fromString('test:\n min-coverage: [95]'), + () => VeryGoodConfig.fromString('test:\n min_coverage: [95]'), throwsA(isA()), ); }); @@ -152,21 +152,21 @@ test: test('throws when collect-coverage-from has invalid value', () { expect( () => - VeryGoodConfig.fromString('test:\n collect-coverage-from: bad'), + VeryGoodConfig.fromString('test:\n collect_coverage_from: bad'), throwsA(isA()), ); }); test('throws when string list has non-string entries', () { expect( - () => VeryGoodConfig.fromString('test:\n dart-define:\n - 42'), + () => VeryGoodConfig.fromString('test:\n dart_define:\n - 42'), throwsA(isA()), ); }); test('throws when string list has wrong type', () { expect( - () => VeryGoodConfig.fromString('test:\n report-on: 42'), + () => VeryGoodConfig.fromString('test:\n report_on: 42'), throwsA(isA()), ); }); @@ -180,21 +180,21 @@ test: test('throws when an unrecognized test key is present', () { expect( - () => VeryGoodConfig.fromString('test:\n min-coverag: 80'), + () => VeryGoodConfig.fromString('test:\n min_coverag: 80'), throwsA(isA()), ); }); test('throws when min-coverage is below 0', () { expect( - () => VeryGoodConfig.fromString('test:\n min-coverage: -1'), + () => VeryGoodConfig.fromString('test:\n min_coverage: -1'), throwsA(isA()), ); }); test('throws when min-coverage is above 100', () { expect( - () => VeryGoodConfig.fromString('test:\n min-coverage: 101'), + () => VeryGoodConfig.fromString('test:\n min_coverage: 101'), throwsA(isA()), ); }); @@ -202,13 +202,13 @@ test: test('parses min-coverage at the boundaries', () { expect( VeryGoodConfig.fromString( - 'test:\n min-coverage: 0', + 'test:\n min_coverage: 0', ).test.minCoverage, '0', ); expect( VeryGoodConfig.fromString( - 'test:\n min-coverage: 100', + 'test:\n min_coverage: 100', ).test.minCoverage, '100', ); @@ -264,7 +264,7 @@ test: test('reads config file when present', () { File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' test: - min-coverage: 90 + min_coverage: 90 '''); final config = VeryGoodConfig.loadFromDirectory(tempDir); expect(config.test.minCoverage, '90'); @@ -301,7 +301,7 @@ test: File(p.join(nestedDir.path, veryGoodConfigFileName)).writeAsStringSync( ''' test: - min-coverage: 80 + min_coverage: 80 ''', ); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); @@ -311,7 +311,7 @@ test: test('reads config from an ancestor directory', () { File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' test: - min-coverage: 90 + min_coverage: 90 '''); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); expect(config.test.minCoverage, '90'); @@ -320,12 +320,12 @@ test: test('prefers the closest config over an ancestor', () { File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' test: - min-coverage: 90 + min_coverage: 90 '''); File(p.join(nestedDir.path, veryGoodConfigFileName)).writeAsStringSync( ''' test: - min-coverage: 80 + min_coverage: 80 ''', ); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); From 52fe50afbde181789b3f7cc118f796c43353ec56 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla <31174242+marcossevilla@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:28:38 +0200 Subject: [PATCH 07/14] Delete e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart --- .../commands/test/very_good_config/fixture/lib/uncovered.dart | 1 - 1 file changed, 1 deletion(-) delete mode 100644 e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart diff --git a/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart b/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart deleted file mode 100644 index 36c51a5b7..000000000 --- a/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart +++ /dev/null @@ -1 +0,0 @@ -int untestedFunction(int value) => value * 2; From 9bd47a27ebd3a5e52ecfdd8f73c0356d91a9baef Mon Sep 17 00:00:00 2001 From: Marcos Sevilla <31174242+marcossevilla@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:28:57 +0200 Subject: [PATCH 08/14] Delete e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart --- .../test/very_good_config/fixture/test/covered_test.dart | 9 --------- 1 file changed, 9 deletions(-) delete mode 100644 e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart diff --git a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart deleted file mode 100644 index 150a80be2..000000000 --- a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart +++ /dev/null @@ -1,9 +0,0 @@ -import 'package:test/test.dart'; - -void main() { - group('very_good_config_fixture', () { - test('trivially succeeds', () { - expect(true, isTrue); - }); - }); -} From 943d1c2bedebc373832bfae932c19ad2c059c59b Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Fri, 3 Jul 2026 12:48:15 +0200 Subject: [PATCH 09/14] fix: ci --- test/src/very_good_config/config_resolver_test.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/src/very_good_config/config_resolver_test.dart b/test/src/very_good_config/config_resolver_test.dart index 5c57d207f..d459c62d3 100644 --- a/test/src/very_good_config/config_resolver_test.dart +++ b/test/src/very_good_config/config_resolver_test.dart @@ -1,7 +1,3 @@ -// Ensures we don't have to use const constructors -// and instances are created at runtime. -// ignore_for_file: prefer_const_constructors - import 'package:args/args.dart'; import 'package:mocktail/mocktail.dart'; import 'package:test/test.dart'; From fa27488e713e174c7073670fbd12b13997baabef Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Fri, 3 Jul 2026 15:29:16 +0200 Subject: [PATCH 10/14] docs: update to use snake_case --- .../test/very_good_config/fixture/lib/uncovered.dart | 1 + .../very_good_config/fixture/test/covered_test.dart | 9 +++++++++ .../test/very_good_config/fixture/very_good.yaml | 2 +- site/docs/commands/test.md | 10 +++++----- 4 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart create mode 100644 e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart diff --git a/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart b/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart new file mode 100644 index 000000000..36c51a5b7 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/lib/uncovered.dart @@ -0,0 +1 @@ +int untestedFunction(int value) => value * 2; diff --git a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart new file mode 100644 index 000000000..150a80be2 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart @@ -0,0 +1,9 @@ +import 'package:test/test.dart'; + +void main() { + group('very_good_config_fixture', () { + test('trivially succeeds', () { + expect(true, isTrue); + }); + }); +} diff --git a/e2e/test/commands/test/very_good_config/fixture/very_good.yaml b/e2e/test/commands/test/very_good_config/fixture/very_good.yaml index 550b2df50..796c69d5c 100644 --- a/e2e/test/commands/test/very_good_config/fixture/very_good.yaml +++ b/e2e/test/commands/test/very_good_config/fixture/very_good.yaml @@ -1,2 +1,2 @@ test: - min-coverage: 100 + min_coverage: 100 diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index b24f629e9..f0f95b028 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -95,16 +95,16 @@ import 'package:test/test.dart'; ### Configuring defaults with `very_good.yaml` -To avoid repeating flags every time you run `very_good test` locally or on CI, you may create a `very_good.yaml` file at the root of your project. The `test` section accepts the same names as the CLI flags (kebab-case). Values from `very_good.yaml` are used as defaults; anything you pass on the command line takes precedence. +To avoid repeating flags every time you run `very_good test` locally or on CI, you may create a `very_good.yaml` file at the root of your project. The `test` section accepts the same names as the CLI flags in snake_case (e.g. `--min-coverage` becomes `min_coverage`). Values from `very_good.yaml` are used as defaults; anything you pass on the command line takes precedence. ```yaml # very_good.yaml test: - min-coverage: 100 - exclude-coverage: "**/*.g.dart" - report-on: + min_coverage: 100 + exclude_coverage: "**/*.g.dart" + report_on: - lib/ - dart-define: + dart_define: - FLAVOR=development ``` From 0a7fd5d8ee8baa2c545efd8ba0e93efe6e4c0ab6 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Fri, 3 Jul 2026 18:08:51 +0200 Subject: [PATCH 11/14] chore: code review --- .github/workflows/very_good_cli.yaml | 2 + .../fixture/test/covered_test.dart | 3 + lib/src/commands/test/test.dart | 129 +++++++++++++----- lib/src/very_good_config/config_resolver.dart | 33 ----- .../very_good_config/very_good_config.dart | 45 +++--- site/docs/commands/test.md | 2 + test/src/commands/test/test_test.dart | 23 +++- .../config_resolver_test.dart | 54 -------- .../very_good_config_test.dart | 94 ++++--------- 9 files changed, 172 insertions(+), 213 deletions(-) delete mode 100644 lib/src/very_good_config/config_resolver.dart delete mode 100644 test/src/very_good_config/config_resolver_test.dart diff --git a/.github/workflows/very_good_cli.yaml b/.github/workflows/very_good_cli.yaml index 3ed8b1fc6..1e93a39df 100644 --- a/.github/workflows/very_good_cli.yaml +++ b/.github/workflows/very_good_cli.yaml @@ -35,6 +35,8 @@ jobs: concurrency: 1 run_skipped: true run_bloc_lint: false + # The CLI's own test suite is run directly rather than through the test optimizer, + # matching the CI behavior prior to adopting this shared workflow. test_optimization: false pana: diff --git a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart index 150a80be2..638cba389 100644 --- a/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart +++ b/e2e/test/commands/test/very_good_config/fixture/test/covered_test.dart @@ -2,6 +2,9 @@ import 'package:test/test.dart'; void main() { group('very_good_config_fixture', () { + // This test exists only to produce a passing coverage run so the + // `min_coverage: 100` config can be evaluated against the deliberately + // uncovered `lib/uncovered.dart`. test('trivially succeeds', () { expect(true, isTrue); }); diff --git a/lib/src/commands/test/test.dart b/lib/src/commands/test/test.dart index c615e96c7..e47c64e69 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/very_good_config/config_resolver.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; /// Options for configuring the Flutter test command. @@ -45,68 +44,111 @@ class FlutterTestOptions { VeryGoodConfig config = VeryGoodConfig.empty, }) { final testConfig = config.test; - final resolver = ConfigResolver(argResults); - final concurrency = resolver.resolve('concurrency', testConfig.concurrency); - final collectCoverage = resolver.resolve('coverage', testConfig.coverage); - final minCoverage = double.tryParse( - resolver.resolve('min-coverage', testConfig.minCoverage) ?? '', + final concurrency = _resolveArg( + argResults, + 'concurrency', + testConfig.concurrency, ); - final showUncovered = resolver.resolve( + final collectCoverage = _resolveArg( + argResults, + 'coverage', + testConfig.coverage, + ); + final minCoverage = _resolveArg( + argResults, + 'min-coverage', + testConfig.minCoverage, + ); + final effectiveMinCoverage = double.tryParse(minCoverage ?? ''); + final showUncovered = _resolveArg( + argResults, 'show-uncovered', testConfig.showUncovered, ); - final excludeTags = resolver.resolve( + final excludeTags = _resolveArg( + argResults, 'exclude-tags', testConfig.excludeTags, ); - final tags = resolver.resolve('tags', testConfig.tags); - final excludeFromCoverage = resolver.resolve( + final tags = _resolveArg(argResults, 'tags', testConfig.tags); + final excludeFromCoverage = _resolveArg( + argResults, 'exclude-coverage', testConfig.excludeCoverage, ); - final collectCoverageFrom = CoverageCollectionMode.fromString( - resolver.resolve( - 'collect-coverage-from', - testConfig.collectCoverageFrom, - fallbackValue: 'imports', - ), + final collectCoverageFrom = _resolveArg( + argResults, + 'collect-coverage-from', + testConfig.collectCoverageFrom, + fallbackValue: 'imports', + ); + final effectiveCollectCoverageFrom = CoverageCollectionMode.fromString( + collectCoverageFrom, ); final randomOrderingSeed = argResults['test-randomize-ordering-seed'] as String?; final randomSeed = randomOrderingSeed == 'random' ? Random().nextInt(4294967295).toString() : randomOrderingSeed; - final optimizePerformance = resolver.resolve( + final optimizePerformance = _resolveArg( + argResults, 'optimization', testConfig.optimization, ); - final updateGoldens = resolver.resolve( + final updateGoldens = _resolveArg( + argResults, 'update-goldens', testConfig.updateGoldens, ); - final failFast = resolver.resolve('fail-fast', testConfig.failFast); + final failFast = _resolveArg( + argResults, + 'fail-fast', + testConfig.failFast, + ); final forceAnsi = argResults['force-ansi'] as bool?; - final dartDefine = resolver.resolve?>( + final dartDefine = _resolveArg?>( + argResults, 'dart-define', testConfig.dartDefine, ); - final dartDefineFromFile = resolver.resolve?>( + final dartDefineFromFile = _resolveArg?>( + argResults, 'dart-define-from-file', testConfig.dartDefineFromFile, ); - final platform = resolver.resolve('platform', testConfig.platform); - final reportOn = resolver - .resolve>('report-on', testConfig.reportOn) + final platform = _resolveArg( + argResults, + 'platform', + testConfig.platform, + ); + final reportOn = _resolveArg>( + argResults, + 'report-on', + testConfig.reportOn, + ); + final effectiveReportOn = reportOn .expand((e) => e.split(RegExp(r'[,\s]+'))) .where((e) => e.isNotEmpty) .toList(); - final runSkipped = resolver.resolve('run-skipped', testConfig.runSkipped); - final flavor = resolver.resolve('flavor', testConfig.flavor); - final timeoutSeconds = int.tryParse( - resolver.resolve('timeout', testConfig.timeout) ?? '', + + final runSkipped = _resolveArg( + argResults, + 'run-skipped', + testConfig.runSkipped, + ); + final flavor = _resolveArg( + argResults, + 'flavor', + testConfig.flavor, ); - final timeout = timeoutSeconds != null + final timeout = _resolveArg( + argResults, + 'timeout', + testConfig.timeout, + ); + final timeoutSeconds = int.tryParse(timeout ?? ''); + final effectiveTimeout = timeoutSeconds != null ? Duration(seconds: timeoutSeconds) : null; final rest = argResults.rest; @@ -114,12 +156,12 @@ class FlutterTestOptions { return FlutterTestOptions._( concurrency: concurrency, collectCoverage: collectCoverage, - minCoverage: minCoverage, + minCoverage: effectiveMinCoverage, showUncovered: showUncovered, excludeTags: excludeTags, tags: tags, excludeFromCoverage: excludeFromCoverage, - collectCoverageFrom: collectCoverageFrom, + collectCoverageFrom: effectiveCollectCoverageFrom, randomSeed: randomSeed, optimizePerformance: optimizePerformance, updateGoldens: updateGoldens, @@ -128,10 +170,10 @@ class FlutterTestOptions { dartDefine: dartDefine, dartDefineFromFile: dartDefineFromFile, platform: platform, - reportOn: reportOn, + reportOn: effectiveReportOn, runSkipped: runSkipped, flavor: flavor, - timeout: timeout, + timeout: effectiveTimeout, rest: rest, ); } @@ -202,6 +244,27 @@ class FlutterTestOptions { final List rest; } +/// Resolves the value for the argument named [name] against a `very_good.yaml` +/// configuration value. +/// +/// Resolution follows a fixed precedence, from highest to lowest: +/// +/// 1. A command line argument that was explicitly parsed. +/// 2. [configValue], the corresponding value from the configuration file. +/// 3. [fallbackValue], used when neither the command line nor the configuration +/// provide a value (typically the argument's command line default). +T _resolveArg( + ArgResults argResults, + String name, + T? configValue, { + T? fallbackValue, +}) { + final value = configValue != null && !argResults.wasParsed(name) + ? configValue + : argResults[name] as T?; + return (value ?? fallbackValue) as T; +} + /// Signature for the [Flutter.installed] method. typedef FlutterInstalledCommand = Future Function({required Logger logger}); diff --git a/lib/src/very_good_config/config_resolver.dart b/lib/src/very_good_config/config_resolver.dart deleted file mode 100644 index cf9df4f19..000000000 --- a/lib/src/very_good_config/config_resolver.dart +++ /dev/null @@ -1,33 +0,0 @@ -import 'package:args/args.dart'; - -/// {@template config_resolver} -/// Merges command line [ArgResults] with `very_good.yaml` configuration values. -/// -/// Resolution follows a fixed precedence, from highest to lowest: -/// -/// 1. A command line argument that was explicitly parsed. -/// 2. A value declared in the configuration file. -/// 3. A fallback value (typically the argument's command line default). -/// -/// Centralizing the rule here keeps every command's argument/configuration -/// merge consistent and independently testable. -/// {@endtemplate} -class ConfigResolver { - /// {@macro config_resolver} - const ConfigResolver(this.argResults); - - /// The parsed command line arguments. - final ArgResults argResults; - - /// Resolves the value for the argument named [name]. - /// - /// [configValue] is the corresponding value from the configuration file, if - /// any. [fallbackValue] is used when neither the command line nor the - /// configuration provide a value. - T resolve(String name, T? configValue, {T? fallbackValue}) { - final value = configValue != null && !argResults.wasParsed(name) - ? configValue - : argResults[name] as T?; - return (value ?? fallbackValue) as T; - } -} diff --git a/lib/src/very_good_config/very_good_config.dart b/lib/src/very_good_config/very_good_config.dart index 2b6ff49b4..e7fdbd97c 100644 --- a/lib/src/very_good_config/very_good_config.dart +++ b/lib/src/very_good_config/very_good_config.dart @@ -77,23 +77,12 @@ class VeryGoodConfig extends Equatable { } } - /// Loads a [VeryGoodConfig] from the given [directory]. - /// - /// Returns [VeryGoodConfig.empty] when the configuration file does not - /// exist. Throws a [VeryGoodConfigParseException] when the file exists - /// but cannot be parsed. - factory VeryGoodConfig.loadFromDirectory(Directory directory) { - final file = File(path.join(directory.path, veryGoodConfigFileName)); - if (!file.existsSync()) return VeryGoodConfig.empty; - return VeryGoodConfig.fromString( - file.readAsStringSync(), - sourceUrl: file.uri, - ); - } - /// Loads the closest [VeryGoodConfig] by searching [directory] and each of /// its ancestors, from the innermost directory outward. /// + /// [directory] is resolved to an absolute path before the walk, so a relative + /// [directory] is searched relative to the current working directory. + /// /// This allows a single repository-wide `very_good.yaml` at the project root /// to apply to commands run from any nested package directory. The first /// configuration file encountered wins; ancestors are not merged. @@ -104,19 +93,28 @@ class VeryGoodConfig extends Equatable { factory VeryGoodConfig.loadFromClosestAncestor(Directory directory) { var current = directory.absolute; while (true) { - final file = File(path.join(current.path, veryGoodConfigFileName)); - if (file.existsSync()) { - return VeryGoodConfig.fromString( - file.readAsStringSync(), - sourceUrl: file.uri, - ); - } + final config = _loadFromDirectory(current); + if (config != null) return config; final parent = current.parent; if (parent.path == current.path) return VeryGoodConfig.empty; current = parent; } } + /// Loads a [VeryGoodConfig] from the configuration file directly inside + /// [directory], or `null` when the file does not exist. + /// + /// Throws a [VeryGoodConfigParseException] when the file exists but cannot be + /// parsed. + static VeryGoodConfig? _loadFromDirectory(Directory directory) { + final file = File(path.join(directory.path, veryGoodConfigFileName)); + if (!file.existsSync()) return null; + return VeryGoodConfig.fromString( + file.readAsStringSync(), + sourceUrl: file.uri, + ); + } + /// An empty [VeryGoodConfig] with no values set. static const VeryGoodConfig empty = VeryGoodConfig(); @@ -252,6 +250,11 @@ class VeryGoodTestConfig extends Equatable { ]; } +// The coercers below intentionally validate more strictly than the CLI flag +// parser. A value such as `min_coverage: 150` is rejected here at config load +// time even though `--min-coverage 150` is accepted by the flag parser, so +// misconfigured `very_good.yaml` files fail fast with a clear message. + /// Coerces a `num` or `String` value into a `String`. /// /// Options are stored as strings to match the CLI's argument parsing (which diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index f0f95b028..8ab43ad3b 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -109,3 +109,5 @@ test: ``` With the file above, running `very_good test` behaves the same as running `very_good test --min-coverage 100 --exclude-coverage '**/*.g.dart' --report-on lib/ --dart-define=FLAVOR=development`. You can still override any of these values on the command line, for example `very_good test --min-coverage 90` to lower the coverage threshold for a single run. + +The `very_good.yaml` file is looked up starting from the directory where the command runs and walking up through its ancestors. The closest file wins; configuration from ancestor directories is not merged. This lets a single `very_good.yaml` at the repository root apply to commands run from any nested package. diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index eb3f2aa71..ef9f7551e 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -939,12 +939,8 @@ void main() { }), ); - test('applies config value when arg was not parsed', () async { + test('applies config value when arg was not parsed', () { when(() => argResults.wasParsed(any())).thenReturn(false); - final result = await testCommand.run(); - expect(result, equals(ExitCode.success.code)); - // Provide the config directly via FlutterTestOptions.parse to - // verify precedence rules without touching the filesystem. final options = FlutterTestOptions.parse( argResults, config: const VeryGoodConfig( @@ -960,7 +956,7 @@ void main() { expect(options.reportOn, equals(['lib/'])); }); - test('CLI argument takes precedence over config value', () async { + test('CLI argument takes precedence over config value', () { when(() => argResults.wasParsed(any())).thenReturn(false); when(() => argResults.wasParsed('min-coverage')).thenReturn(true); when(() => argResults['min-coverage']).thenReturn('50'); @@ -973,6 +969,21 @@ void main() { ); expect(options.minCoverage, 50); }); + + test( + 'falls back to the CLI default when the parsed arg is null ' + 'and the config is unset', + () { + when(() => argResults.wasParsed(any())).thenReturn(true); + when( + () => argResults['collect-coverage-from'], + ).thenReturn(null); + + final options = FlutterTestOptions.parse(argResults); + + expect(options.collectCoverageFrom, CoverageCollectionMode.imports); + }, + ); }); }); } diff --git a/test/src/very_good_config/config_resolver_test.dart b/test/src/very_good_config/config_resolver_test.dart deleted file mode 100644 index d459c62d3..000000000 --- a/test/src/very_good_config/config_resolver_test.dart +++ /dev/null @@ -1,54 +0,0 @@ -import 'package:args/args.dart'; -import 'package:mocktail/mocktail.dart'; -import 'package:test/test.dart'; -import 'package:very_good_cli/src/very_good_config/config_resolver.dart'; - -class _MockArgResults extends Mock implements ArgResults {} - -void main() { - group('$ConfigResolver', () { - late ArgResults argResults; - late ConfigResolver resolver; - - setUp(() { - argResults = _MockArgResults(); - resolver = ConfigResolver(argResults); - }); - - test('uses the config value when the argument was not parsed', () { - when(() => argResults.wasParsed('min-coverage')).thenReturn(false); - - expect(resolver.resolve('min-coverage', '90'), '90'); - }); - - test('uses the parsed argument over the config value', () { - when(() => argResults.wasParsed('min-coverage')).thenReturn(true); - when(() => argResults['min-coverage']).thenReturn('50'); - - expect(resolver.resolve('min-coverage', '90'), '50'); - }); - - test('uses the argument value when there is no config value', () { - when(() => argResults.wasParsed('tags')).thenReturn(false); - when(() => argResults['tags']).thenReturn('unit'); - - expect(resolver.resolve('tags', null), 'unit'); - }); - - test('falls back when neither argument nor config provide a value', () { - when( - () => argResults.wasParsed('collect-coverage-from'), - ).thenReturn(false); - when(() => argResults['collect-coverage-from']).thenReturn(null); - - expect( - resolver.resolve( - 'collect-coverage-from', - null, - fallbackValue: 'imports', - ), - 'imports', - ); - }); - }); -} diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart index ede147c31..61cbfa410 100644 --- a/test/src/very_good_config/very_good_config_test.dart +++ b/test/src/very_good_config/very_good_config_test.dart @@ -9,7 +9,7 @@ import 'package:test/test.dart'; import 'package:very_good_cli/src/very_good_config/very_good_config.dart'; void main() { - group('$VeryGoodConfig', () { + group(VeryGoodConfig, () { group('fromString', () { test('returns empty config when content is empty', () { expect(VeryGoodConfig.fromString(''), equals(VeryGoodConfig.empty)); @@ -62,22 +62,22 @@ test: expect(config.test.coverage, isTrue); expect(config.test.optimization, isFalse); - expect(config.test.concurrency, '8'); - expect(config.test.tags, 'my-tag'); - expect(config.test.excludeCoverage, '**/*.g.dart'); - expect(config.test.excludeTags, 'skip'); - expect(config.test.minCoverage, '95'); + expect(config.test.concurrency, equals('8')); + expect(config.test.tags, equals('my-tag')); + expect(config.test.excludeCoverage, equals('**/*.g.dart')); + expect(config.test.excludeTags, equals('skip')); + expect(config.test.minCoverage, equals('95')); expect(config.test.showUncovered, isTrue); - expect(config.test.collectCoverageFrom, 'all'); + expect(config.test.collectCoverageFrom, equals('all')); expect(config.test.updateGoldens, isTrue); expect(config.test.failFast, isTrue); expect(config.test.dartDefine, equals(['FOO=bar', 'X=42'])); expect(config.test.dartDefineFromFile, equals(['defines.env'])); - expect(config.test.platform, 'chrome'); + expect(config.test.platform, equals('chrome')); expect(config.test.reportOn, equals(['lib/', 'packages/foo/lib/'])); expect(config.test.runSkipped, isTrue); - expect(config.test.flavor, 'staging'); - expect(config.test.timeout, '30'); + expect(config.test.flavor, equals('staging')); + expect(config.test.timeout, equals('30')); }); test('parses min-coverage as decimal string', () { @@ -85,7 +85,7 @@ test: test: min_coverage: 95.5 '''); - expect(config.test.minCoverage, '95.5'); + expect(config.test.minCoverage, equals('95.5')); }); test('parses integer options provided as quoted strings', () { @@ -94,8 +94,8 @@ test: concurrency: "8" timeout: "60" '''); - expect(config.test.concurrency, '8'); - expect(config.test.timeout, '60'); + expect(config.test.concurrency, equals('8')); + expect(config.test.timeout, equals('60')); }); test('parses min-coverage provided as a quoted string', () { @@ -103,7 +103,7 @@ test: test: min_coverage: "95" '''); - expect(config.test.minCoverage, '95'); + expect(config.test.minCoverage, equals('95')); }); test('parses collect-coverage-from with value `imports`', () { @@ -111,7 +111,7 @@ test: test: collect_coverage_from: imports '''); - expect(config.test.collectCoverageFrom, 'imports'); + expect(config.test.collectCoverageFrom, equals('imports')); }); test('throws when test section is not a map', () { @@ -151,8 +151,9 @@ test: test('throws when collect-coverage-from has invalid value', () { expect( - () => - VeryGoodConfig.fromString('test:\n collect_coverage_from: bad'), + () => VeryGoodConfig.fromString( + 'test:\n collect_coverage_from: bad', + ), throwsA(isA()), ); }); @@ -204,13 +205,13 @@ test: VeryGoodConfig.fromString( 'test:\n min_coverage: 0', ).test.minCoverage, - '0', + equals('0'), ); expect( VeryGoodConfig.fromString( 'test:\n min_coverage: 100', ).test.minCoverage, - '100', + equals('100'), ); }); @@ -241,54 +242,15 @@ test: }); }); - group('loadFromDirectory', () { - late Directory tempDir; - - setUp(() { - tempDir = Directory.systemTemp.createTempSync('very_good_config_'); - }); - - tearDown(() { - if (tempDir.existsSync()) { - tempDir.deleteSync(recursive: true); - } - }); - - test('returns empty config when file is missing', () { - expect( - VeryGoodConfig.loadFromDirectory(tempDir), - equals(VeryGoodConfig.empty), - ); - }); - - test('reads config file when present', () { - File(p.join(tempDir.path, veryGoodConfigFileName)).writeAsStringSync(''' -test: - min_coverage: 90 -'''); - final config = VeryGoodConfig.loadFromDirectory(tempDir); - expect(config.test.minCoverage, '90'); - }); - - test('rethrows parse exception when file is malformed', () { - File( - p.join(tempDir.path, veryGoodConfigFileName), - ).writeAsStringSync('- not\n- a\n- map'); - expect( - () => VeryGoodConfig.loadFromDirectory(tempDir), - throwsA(isA()), - ); - }); - }); - group('loadFromClosestAncestor', () { late Directory tempDir; late Directory nestedDir; setUp(() { tempDir = Directory.systemTemp.createTempSync('very_good_config_'); - nestedDir = Directory(p.join(tempDir.path, 'packages', 'foo')) - ..createSync(recursive: true); + nestedDir = Directory( + p.join(tempDir.path, 'packages', 'foo'), + )..createSync(recursive: true); }); tearDown(() { @@ -305,7 +267,7 @@ test: ''', ); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); - expect(config.test.minCoverage, '80'); + expect(config.test.minCoverage, equals('80')); }); test('reads config from an ancestor directory', () { @@ -314,7 +276,7 @@ test: min_coverage: 90 '''); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); - expect(config.test.minCoverage, '90'); + expect(config.test.minCoverage, equals('90')); }); test('prefers the closest config over an ancestor', () { @@ -329,7 +291,7 @@ test: ''', ); final config = VeryGoodConfig.loadFromClosestAncestor(nestedDir); - expect(config.test.minCoverage, '80'); + expect(config.test.minCoverage, equals('80')); }); test('returns empty config when no file is found in any ancestor', () { @@ -365,7 +327,7 @@ test: }); }); - group('$VeryGoodTestConfig', () { + group(VeryGoodTestConfig, () { test('supports value equality', () { expect( VeryGoodTestConfig(coverage: true, minCoverage: '95'), @@ -378,7 +340,7 @@ test: }); }); - group('$VeryGoodConfigParseException', () { + group(VeryGoodConfigParseException, () { test('provides message via toString', () { const exception = VeryGoodConfigParseException('bad thing'); expect(exception.toString(), contains('bad thing')); From 6e4f29105ae9c0285d5683522de1c372eace8f79 Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 6 Jul 2026 12:50:26 +0200 Subject: [PATCH 12/14] fix: extend ensure_build timeout and format docs - Add @Timeout to ensure_build_test to avoid 30s AOT compilation timeout in CI - Format site/docs/commands/test.md with prettier - Add act scripts for local CI verification --- .gitignore | 4 ++ scripts/act/install-act.sh | 65 +++++++++++++++++++++++ scripts/act/run-act.sh | 103 ++++++++++++++++++++++++++++++++++++ site/docs/commands/test.md | 2 +- test/ensure_build_test.dart | 3 ++ 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100755 scripts/act/install-act.sh create mode 100755 scripts/act/run-act.sh diff --git a/.gitignore b/.gitignore index 4ea56d1fb..2890b4b29 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,7 @@ CLAUDE.local.md .codex/ .cursor/ + +# act artifacts +act_output.log +.secrets diff --git a/scripts/act/install-act.sh b/scripts/act/install-act.sh new file mode 100755 index 000000000..e2e7701c8 --- /dev/null +++ b/scripts/act/install-act.sh @@ -0,0 +1,65 @@ +#!/bin/bash +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Installs 'act' (https://github.com/nektos/act) for running GitHub Actions locally. +# Always installs the latest release. +# Usage: ./install-act.sh + +set -euo pipefail + +INSTALL_DIR="${HOME}/.local/bin" + +echo "🔧 Installing act..." + +# Detect platform +OS=$(uname -s | tr '[:upper:]' '[:lower:]') +ARCH=$(uname -m) + +case "$ARCH" in + x86_64) ARCH="x86_64" ;; + aarch64) ARCH="arm64" ;; + arm64) ARCH="arm64" ;; + *) + echo "❌ Unsupported architecture: $ARCH" + exit 1 + ;; +esac + +# Create install directory if it doesn't exist +mkdir -p "$INSTALL_DIR" + +# Try system-wide install first, fall back to user-local +if command -v sudo &> /dev/null && sudo -n true 2>/dev/null; then + echo " Installing to /usr/local/bin (system-wide)..." + curl -sL https://raw.githubusercontent.com/nektos/act/master/install.sh | sudo bash -s -- -b /usr/local/bin +else + echo " No sudo access. Installing to ${INSTALL_DIR} (user-local)..." + curl -sL https://raw.githubusercontent.com/nektos/act/master/install.sh | bash -s -- -b "$INSTALL_DIR" + + # Ensure install dir is on PATH + if [[ ":$PATH:" != *":${INSTALL_DIR}:"* ]]; then + echo " ⚠️ ${INSTALL_DIR} is not on your PATH." + echo " Add this to your shell profile: export PATH=\"${INSTALL_DIR}:\$PATH\"" + export PATH="${INSTALL_DIR}:$PATH" + fi +fi + +# Verify installation +if command -v act &> /dev/null; then + echo "✅ act installed successfully: $(act --version)" +else + echo "❌ Installation failed. act not found on PATH." + exit 1 +fi diff --git a/scripts/act/run-act.sh b/scripts/act/run-act.sh new file mode 100755 index 000000000..de72d8639 --- /dev/null +++ b/scripts/act/run-act.sh @@ -0,0 +1,103 @@ +#!/bin/bash +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Runs 'act' in the background with log polling to prevent agent timeouts. +# Usage: ./run-act.sh "" +# Example: ./run-act.sh "push -j build --matrix node-version:20.x" + +set -euo pipefail + +ACT_ARGS="${1:-}" +LOG_FILE="act_output.log" +TIMEOUT="${ACT_TIMEOUT:-600}" # Default: 10 minutes +POLL_INTERVAL="${ACT_POLL:-10}" # Default: 10 seconds + +if [ -z "$ACT_ARGS" ]; then + echo "Error: No arguments provided." + echo "Usage: $0 \"\"" + echo "Example: $0 \"push -j build --matrix node-version:20.x\"" + exit 1 +fi + +# Check Docker is running +if ! docker info > /dev/null 2>&1; then + echo "❌ Docker is not running. Start Docker and try again." + exit 1 +fi + +# Check act is available +if ! command -v act &> /dev/null; then + echo "❌ 'act' is not installed. Run install-act.sh first." + exit 1 +fi + +echo "🚀 Starting: act ${ACT_ARGS}" +echo "📄 Logging to: ${LOG_FILE}" +echo "⏱️ Timeout: ${TIMEOUT}s | Poll: ${POLL_INTERVAL}s" +echo "" + +# Run act in background +# Add default runner image only if the user didn't specify one via -P +if echo "$ACT_ARGS" | grep -q -- '-P '; then + act ${ACT_ARGS} > "$LOG_FILE" 2>&1 & +else + act ${ACT_ARGS} -P ubuntu-latest=catthehacker/ubuntu:act-latest > "$LOG_FILE" 2>&1 & +fi + +ACT_PID=$! +echo "Process started (PID: ${ACT_PID})" + +ELAPSED=0 + +# Poll log file while process is running +while kill -0 "$ACT_PID" 2>/dev/null; do + if [ $ELAPSED -ge $TIMEOUT ]; then + echo "" + echo "⏰ Timeout reached (${TIMEOUT}s). Killing act process..." + kill "$ACT_PID" 2>/dev/null || true + wait "$ACT_PID" 2>/dev/null || true + echo "" + echo "--- Full Log ---" + cat "$LOG_FILE" 2>/dev/null || true + echo "--- End Log ---" + exit 1 + fi + + sleep "$POLL_INTERVAL" + ELAPSED=$((ELAPSED + POLL_INTERVAL)) + + # Show last few lines as progress + echo "⏳ Running... (${ELAPSED}s/${TIMEOUT}s)" + tail -n 5 "$LOG_FILE" 2>/dev/null || true + echo "" +done + +# Capture exit code +wait "$ACT_PID" +EXIT_CODE=$? + +echo "" +echo "--- Full Execution Log ---" +cat "$LOG_FILE" +echo "--- End Log ---" +echo "" + +if [ $EXIT_CODE -eq 0 ]; then + echo "✅ Local GitHub Actions passed." + exit 0 +else + echo "❌ Local GitHub Actions failed (exit code: ${EXIT_CODE})." + exit 1 +fi diff --git a/site/docs/commands/test.md b/site/docs/commands/test.md index 8ab43ad3b..3606d2b33 100644 --- a/site/docs/commands/test.md +++ b/site/docs/commands/test.md @@ -101,7 +101,7 @@ To avoid repeating flags every time you run `very_good test` locally or on CI, y # very_good.yaml test: min_coverage: 100 - exclude_coverage: "**/*.g.dart" + exclude_coverage: '**/*.g.dart' report_on: - lib/ dart_define: diff --git a/test/ensure_build_test.dart b/test/ensure_build_test.dart index 8627c2968..743e7d167 100644 --- a/test/ensure_build_test.dart +++ b/test/ensure_build_test.dart @@ -1,4 +1,7 @@ @Tags(['pull-request-only']) +// Building the code generators with AOT compilation can take longer than the +// default 30 second test timeout on CI, so allow additional time. +@Timeout(Duration(minutes: 5)) library; import 'package:build_verify/build_verify.dart'; From 5389f13d598c5ff782736fa0873f02b14e8a9b8c Mon Sep 17 00:00:00 2001 From: Marcos Sevilla Date: Mon, 6 Jul 2026 13:01:38 +0200 Subject: [PATCH 13/14] chore: remove act scripts --- scripts/act/install-act.sh | 65 ----------------------- scripts/act/run-act.sh | 103 ------------------------------------- 2 files changed, 168 deletions(-) delete mode 100755 scripts/act/install-act.sh delete mode 100755 scripts/act/run-act.sh diff --git a/scripts/act/install-act.sh b/scripts/act/install-act.sh deleted file mode 100755 index e2e7701c8..000000000 --- a/scripts/act/install-act.sh +++ /dev/null @@ -1,65 +0,0 @@ -#!/bin/bash -# Copyright 2026 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Installs 'act' (https://github.com/nektos/act) for running GitHub Actions locally. -# Always installs the latest release. -# Usage: ./install-act.sh - -set -euo pipefail - -INSTALL_DIR="${HOME}/.local/bin" - -echo "🔧 Installing act..." - -# Detect platform -OS=$(uname -s | tr '[:upper:]' '[:lower:]') -ARCH=$(uname -m) - -case "$ARCH" in - x86_64) ARCH="x86_64" ;; - aarch64) ARCH="arm64" ;; - arm64) ARCH="arm64" ;; - *) - echo "❌ Unsupported architecture: $ARCH" - exit 1 - ;; -esac - -# Create install directory if it doesn't exist -mkdir -p "$INSTALL_DIR" - -# Try system-wide install first, fall back to user-local -if command -v sudo &> /dev/null && sudo -n true 2>/dev/null; then - echo " Installing to /usr/local/bin (system-wide)..." - curl -sL https://raw.githubusercontent.com/nektos/act/master/install.sh | sudo bash -s -- -b /usr/local/bin -else - echo " No sudo access. Installing to ${INSTALL_DIR} (user-local)..." - curl -sL https://raw.githubusercontent.com/nektos/act/master/install.sh | bash -s -- -b "$INSTALL_DIR" - - # Ensure install dir is on PATH - if [[ ":$PATH:" != *":${INSTALL_DIR}:"* ]]; then - echo " ⚠️ ${INSTALL_DIR} is not on your PATH." - echo " Add this to your shell profile: export PATH=\"${INSTALL_DIR}:\$PATH\"" - export PATH="${INSTALL_DIR}:$PATH" - fi -fi - -# Verify installation -if command -v act &> /dev/null; then - echo "✅ act installed successfully: $(act --version)" -else - echo "❌ Installation failed. act not found on PATH." - exit 1 -fi diff --git a/scripts/act/run-act.sh b/scripts/act/run-act.sh deleted file mode 100755 index de72d8639..000000000 --- a/scripts/act/run-act.sh +++ /dev/null @@ -1,103 +0,0 @@ -#!/bin/bash -# Copyright 2026 Google LLC -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Runs 'act' in the background with log polling to prevent agent timeouts. -# Usage: ./run-act.sh "" -# Example: ./run-act.sh "push -j build --matrix node-version:20.x" - -set -euo pipefail - -ACT_ARGS="${1:-}" -LOG_FILE="act_output.log" -TIMEOUT="${ACT_TIMEOUT:-600}" # Default: 10 minutes -POLL_INTERVAL="${ACT_POLL:-10}" # Default: 10 seconds - -if [ -z "$ACT_ARGS" ]; then - echo "Error: No arguments provided." - echo "Usage: $0 \"\"" - echo "Example: $0 \"push -j build --matrix node-version:20.x\"" - exit 1 -fi - -# Check Docker is running -if ! docker info > /dev/null 2>&1; then - echo "❌ Docker is not running. Start Docker and try again." - exit 1 -fi - -# Check act is available -if ! command -v act &> /dev/null; then - echo "❌ 'act' is not installed. Run install-act.sh first." - exit 1 -fi - -echo "🚀 Starting: act ${ACT_ARGS}" -echo "📄 Logging to: ${LOG_FILE}" -echo "⏱️ Timeout: ${TIMEOUT}s | Poll: ${POLL_INTERVAL}s" -echo "" - -# Run act in background -# Add default runner image only if the user didn't specify one via -P -if echo "$ACT_ARGS" | grep -q -- '-P '; then - act ${ACT_ARGS} > "$LOG_FILE" 2>&1 & -else - act ${ACT_ARGS} -P ubuntu-latest=catthehacker/ubuntu:act-latest > "$LOG_FILE" 2>&1 & -fi - -ACT_PID=$! -echo "Process started (PID: ${ACT_PID})" - -ELAPSED=0 - -# Poll log file while process is running -while kill -0 "$ACT_PID" 2>/dev/null; do - if [ $ELAPSED -ge $TIMEOUT ]; then - echo "" - echo "⏰ Timeout reached (${TIMEOUT}s). Killing act process..." - kill "$ACT_PID" 2>/dev/null || true - wait "$ACT_PID" 2>/dev/null || true - echo "" - echo "--- Full Log ---" - cat "$LOG_FILE" 2>/dev/null || true - echo "--- End Log ---" - exit 1 - fi - - sleep "$POLL_INTERVAL" - ELAPSED=$((ELAPSED + POLL_INTERVAL)) - - # Show last few lines as progress - echo "⏳ Running... (${ELAPSED}s/${TIMEOUT}s)" - tail -n 5 "$LOG_FILE" 2>/dev/null || true - echo "" -done - -# Capture exit code -wait "$ACT_PID" -EXIT_CODE=$? - -echo "" -echo "--- Full Execution Log ---" -cat "$LOG_FILE" -echo "--- End Log ---" -echo "" - -if [ $EXIT_CODE -eq 0 ]; then - echo "✅ Local GitHub Actions passed." - exit 0 -else - echo "❌ Local GitHub Actions failed (exit code: ${EXIT_CODE})." - exit 1 -fi From 1f7cea87eaf53e90bcfcbad7d2454aea077864bc Mon Sep 17 00:00:00 2001 From: "unicoderbot[bot]" <269805761+unicoderbot[bot]@users.noreply.github.com> Date: Mon, 6 Jul 2026 11:52:37 +0000 Subject: [PATCH 14/14] fix: address review feedback on PR #1636 Co-authored-by: marcossevilla --- .gitignore | 4 -- .../malformed_fixture/pubspec.yaml | 10 ++++ .../malformed_fixture/very_good.yaml | 3 ++ .../very_good_config_test.dart | 54 +++++++++---------- test/src/commands/test/test_test.dart | 27 +++++----- .../fixtures/all_test_options.yaml | 23 ++++++++ .../very_good_config_test.dart | 35 ++++-------- 7 files changed, 82 insertions(+), 74 deletions(-) create mode 100644 e2e/test/commands/test/very_good_config/malformed_fixture/pubspec.yaml create mode 100644 e2e/test/commands/test/very_good_config/malformed_fixture/very_good.yaml create mode 100644 test/src/very_good_config/fixtures/all_test_options.yaml diff --git a/.gitignore b/.gitignore index 2890b4b29..4ea56d1fb 100644 --- a/.gitignore +++ b/.gitignore @@ -37,7 +37,3 @@ CLAUDE.local.md .codex/ .cursor/ - -# act artifacts -act_output.log -.secrets diff --git a/e2e/test/commands/test/very_good_config/malformed_fixture/pubspec.yaml b/e2e/test/commands/test/very_good_config/malformed_fixture/pubspec.yaml new file mode 100644 index 000000000..cc5ff3815 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/malformed_fixture/pubspec.yaml @@ -0,0 +1,10 @@ +name: malformed_fixture +description: Fixture for testing malformed very_good.yaml. +version: 0.1.0+1 +publish_to: none + +environment: + sdk: ^3.12.0 + +dev_dependencies: + test: ^1.24.3 diff --git a/e2e/test/commands/test/very_good_config/malformed_fixture/very_good.yaml b/e2e/test/commands/test/very_good_config/malformed_fixture/very_good.yaml new file mode 100644 index 000000000..149ff8996 --- /dev/null +++ b/e2e/test/commands/test/very_good_config/malformed_fixture/very_good.yaml @@ -0,0 +1,3 @@ +- not +- a +- map diff --git a/e2e/test/commands/test/very_good_config/very_good_config_test.dart b/e2e/test/commands/test/very_good_config/very_good_config_test.dart index a2219e9e2..f4f0ba88a 100644 --- a/e2e/test/commands/test/very_good_config/very_good_config_test.dart +++ b/e2e/test/commands/test/very_good_config/very_good_config_test.dart @@ -35,9 +35,10 @@ void main() { Directory.current = tempDirectory; addTearDown(() => Directory.current = cwd); - final result = await commandRunner.run(['test', '--coverage']); - - expect(result, equals(ExitCode.unavailable.code)); + await expectLater( + commandRunner.run(['test', '--coverage']), + completion(equals(ExitCode.unavailable.code)), + ); verify( () => logger.err(any(that: contains('Expected coverage >= 100.00%'))), ).called(1); @@ -71,14 +72,15 @@ void main() { Directory.current = tempDirectory; addTearDown(() => Directory.current = cwd); - final result = await commandRunner.run([ - 'test', - '--coverage', - '--min-coverage', - '0', - ]); - - expect(result, equals(ExitCode.success.code)); + await expectLater( + commandRunner.run([ + 'test', + '--coverage', + '--min-coverage', + '0', + ]), + completion(equals(ExitCode.success.code)), + ); }), ); @@ -91,31 +93,23 @@ void main() { ); addTearDown(() => tempDirectory.deleteSync(recursive: true)); - File(path.join(tempDirectory.path, 'pubspec.yaml')).writeAsStringSync( - ''' -name: malformed_fixture -description: Fixture for testing malformed very_good.yaml. -version: 0.1.0+1 -publish_to: none - -environment: - sdk: ^3.12.0 - -dev_dependencies: - test: ^1.24.3 -''', + final fixture = Directory( + path.join( + Directory.current.path, + 'test/commands/test/very_good_config/malformed_fixture', + ), ); - File( - path.join(tempDirectory.path, 'very_good.yaml'), - ).writeAsStringSync('- not\n- a\n- map'); + + await copyDirectory(fixture, tempDirectory); final cwd = Directory.current; Directory.current = tempDirectory; addTearDown(() => Directory.current = cwd); - final result = await commandRunner.run(['test']); - - expect(result, equals(ExitCode.config.code)); + await expectLater( + commandRunner.run(['test']), + completion(equals(ExitCode.config.code)), + ); verify( () => logger.err( any(that: contains('Could not read `very_good.yaml`')), diff --git a/test/src/commands/test/test_test.dart b/test/src/commands/test/test_test.dart index ef9f7551e..682e3af8f 100644 --- a/test/src/commands/test/test_test.dart +++ b/test/src/commands/test/test_test.dart @@ -951,8 +951,8 @@ void main() { ), ), ); - expect(options.minCoverage, 90); - expect(options.excludeFromCoverage, '**/*.g.dart'); + expect(options.minCoverage, equals(90)); + expect(options.excludeFromCoverage, equals('**/*.g.dart')); expect(options.reportOn, equals(['lib/'])); }); @@ -967,23 +967,20 @@ void main() { test: VeryGoodTestConfig(minCoverage: '90'), ), ); - expect(options.minCoverage, 50); + expect(options.minCoverage, equals(50)); }); - test( - 'falls back to the CLI default when the parsed arg is null ' - 'and the config is unset', - () { - when(() => argResults.wasParsed(any())).thenReturn(true); - when( - () => argResults['collect-coverage-from'], - ).thenReturn(null); + test('falls back to the CLI default when the parsed arg is null ' + 'and the config is unset', () { + when(() => argResults.wasParsed(any())).thenReturn(true); + when( + () => argResults['collect-coverage-from'], + ).thenReturn(null); - final options = FlutterTestOptions.parse(argResults); + final options = FlutterTestOptions.parse(argResults); - expect(options.collectCoverageFrom, CoverageCollectionMode.imports); - }, - ); + expect(options.collectCoverageFrom, CoverageCollectionMode.imports); + }); }); }); } diff --git a/test/src/very_good_config/fixtures/all_test_options.yaml b/test/src/very_good_config/fixtures/all_test_options.yaml new file mode 100644 index 000000000..f5d6c280c --- /dev/null +++ b/test/src/very_good_config/fixtures/all_test_options.yaml @@ -0,0 +1,23 @@ +test: + coverage: true + optimization: false + concurrency: 8 + tags: my-tag + exclude_coverage: "**/*.g.dart" + exclude_tags: skip + min_coverage: 95 + show_uncovered: true + collect_coverage_from: all + update_goldens: true + fail_fast: true + dart_define: + - FOO=bar + - X=42 + dart_define_from_file: defines.env + platform: chrome + report_on: + - lib/ + - packages/foo/lib/ + run_skipped: true + flavor: staging + timeout: 30 diff --git a/test/src/very_good_config/very_good_config_test.dart b/test/src/very_good_config/very_good_config_test.dart index 61cbfa410..430e1fded 100644 --- a/test/src/very_good_config/very_good_config_test.dart +++ b/test/src/very_good_config/very_good_config_test.dart @@ -34,31 +34,16 @@ void main() { }); test('parses all supported test options', () { - final config = VeryGoodConfig.fromString(''' -test: - coverage: true - optimization: false - concurrency: 8 - tags: my-tag - exclude_coverage: "**/*.g.dart" - exclude_tags: skip - min_coverage: 95 - show_uncovered: true - collect_coverage_from: all - update_goldens: true - fail_fast: true - dart_define: - - FOO=bar - - X=42 - dart_define_from_file: defines.env - platform: chrome - report_on: - - lib/ - - packages/foo/lib/ - run_skipped: true - flavor: staging - timeout: 30 -'''); + final fixture = File( + p.join( + 'test', + 'src', + 'very_good_config', + 'fixtures', + 'all_test_options.yaml', + ), + ); + final config = VeryGoodConfig.fromString(fixture.readAsStringSync()); expect(config.test.coverage, isTrue); expect(config.test.optimization, isFalse);