From b90c33c6db8a9d9c77eebac737cae9469e5fc40c Mon Sep 17 00:00:00 2001 From: David Morgan Date: Fri, 18 Jul 2025 10:42:58 +0200 Subject: [PATCH] Support testing builder factories. --- .../generate/build_configuration_test.dart | 30 ++--- .../test/generate/build_test.dart | 43 +++--- build_test/CHANGELOG.md | 12 ++ build_test/lib/src/builder.dart | 16 ++- build_test/lib/src/test_builder.dart | 123 +++++++++++++++--- build_test/pubspec.yaml | 2 +- 6 files changed, 159 insertions(+), 67 deletions(-) diff --git a/build_runner_core/test/generate/build_configuration_test.dart b/build_runner_core/test/generate/build_configuration_test.dart index 564346f5a..069ecb281 100644 --- a/build_runner_core/test/generate/build_configuration_test.dart +++ b/build_runner_core/test/generate/build_configuration_test.dart @@ -4,22 +4,20 @@ import 'package:_test_common/common.dart'; import 'package:build/build.dart'; -import 'package:build_runner_core/build_runner_core.dart'; import 'package:test/test.dart'; void main() { test('uses builder options', () async { Builder copyBuilder(BuilderOptions options) => TestBuilder( buildExtensions: replaceExtension( - options.config['inputExtension'] as String, + options.config['inputExtension'] as String? ?? '', '.copy', ), + name: 'a:optioned_builder', ); - await testPhases( - [ - apply('a:optioned_builder', [copyBuilder], toRoot(), hideOutput: false), - ], + await testBuilderFactories( + [copyBuilder], { 'a|lib/file.nomatch': 'a', 'a|lib/file.matches': 'b', @@ -32,10 +30,10 @@ targets: inputExtension: .matches ''', }, + testingBuilderConfig: false, outputs: {'a|lib/file.copy': 'b'}, ); }); - test('isRoot is applied correctly', () async { Builder copyBuilder(BuilderOptions options) => TestBuilder( buildExtensions: replaceExtension( @@ -43,22 +41,10 @@ targets: options.isRoot ? '.root.copy' : '.dep.copy', ), ); - var packageGraph = buildPackageGraph({ - rootPackage('a'): ['b'], - package('b'): [], - }); - await testPhases( - [ - apply( - 'a:optioned_builder', - [copyBuilder], - toAllPackages(), - hideOutput: true, - ), - ], + await testBuilderFactories( + [copyBuilder], {'a|lib/a.txt': 'a', 'b|lib/b.txt': 'b'}, - outputs: {r'$$a|lib/a.root.copy': 'a', r'$$b|lib/b.dep.copy': 'b'}, - packageGraph: packageGraph, + outputs: {r'a|lib/a.root.copy': 'a', r'b|lib/b.dep.copy': 'b'}, ); }); } diff --git a/build_runner_core/test/generate/build_test.dart b/build_runner_core/test/generate/build_test.dart index a620ea42b..7a4ef9fae 100644 --- a/build_runner_core/test/generate/build_test.dart +++ b/build_runner_core/test/generate/build_test.dart @@ -243,22 +243,19 @@ void main() { }); test('with placeholder as input', () async { - await testPhases( - [ - applyToRoot( - PlaceholderBuilder( - {'lib.txt': 'libText'}.build(), - inputPlaceholder: r'$lib$', - ), - ), - applyToRoot( - PlaceholderBuilder( - {'root.txt': 'rootText'}.build(), - inputPlaceholder: r'$package$', - ), - ), - ], + final builder1 = PlaceholderBuilder( + {'lib.txt': 'libText'}.build(), + inputPlaceholder: r'$lib$', + ); + final builder2 = PlaceholderBuilder( + {'root.txt': 'rootText'}.build(), + inputPlaceholder: r'$package$', + ); + await testBuilders( + [builder1, builder2], {}, + visibleOutputBuilders: {builder1, builder2}, + rootPackage: 'a', outputs: {'a|lib/lib.txt': 'libText', 'a|root.txt': 'rootText'}, ); }); @@ -1030,16 +1027,12 @@ targets: }); test('can\'t read files in .dart_tool', () async { - await testPhases( - [ - apply('', [ - (_) => TestBuilder( - build: copyFrom(makeAssetId('a|.dart_tool/any_file')), - ), - ], toRoot()), - ], - {'a|lib/a.txt': 'a', 'a|.dart_tool/any_file': 'content'}, - status: BuildStatus.failure, + expect( + (await testBuilders( + [TestBuilder(build: copyFrom(makeAssetId('a|.dart_tool/any_file')))], + {'a|lib/a.txt': 'a', 'a|.dart_tool/any_file': 'content'}, + )).buildResult.status, + BuildStatus.failure, ); }); diff --git a/build_test/CHANGELOG.md b/build_test/CHANGELOG.md index b4d94072d..45e316abd 100644 --- a/build_test/CHANGELOG.md +++ b/build_test/CHANGELOG.md @@ -1,3 +1,15 @@ +## 3.4.0-wip + +- Add `testBuilderFactories`: like `testBuilders`, but provide the builder + factories instead of the builders. Use this to allow config read from + `build.yaml` to be passed in to the factory. +- `TestBuilder` now accepts a `name`: this is the name that will be shown + in logging and can be used to refer to the builder in `build.yaml`. +- More realistic test builds: in `resolveSources` and `testBuilders`, stop + builders reading from `.dart_tool`. +- Bug fix: in `testBuilders`, configure the root package correctly when it + has no sources. + ## 3.3.0 - Read build configs using `AssetReader` so they're easier to test: you can now diff --git a/build_test/lib/src/builder.dart b/build_test/lib/src/builder.dart index e0cffab60..7f8c8cd45 100644 --- a/build_test/lib/src/builder.dart +++ b/build_test/lib/src/builder.dart @@ -94,6 +94,7 @@ class TestBuilder implements Builder { @override final Map> buildExtensions; + final String name; final BuildBehavior _build; final BuildBehavior? _extraWork; @@ -109,14 +110,24 @@ class TestBuilder implements Builder { final _buildsCompletedController = StreamController.broadcast(); Stream get buildsCompleted => _buildsCompletedController.stream; + /// A test [Builder]. + /// + /// Runs for all inputs and write outputs with `.copy` appended. Or, pass + /// [buildExtensions] to specify input and output extensions. + /// + /// Copy its input to all possible outputs. Pass [build] to replace this + /// behavior, and/or [extraWork] to add additional behavior. + /// + /// Default name for logging and for `build.yaml` is `TestBuilder`. Pass + /// [name] to set the name. TestBuilder({ Map>? buildExtensions, BuildBehavior? build, BuildBehavior? extraWork, + this.name = 'TestBuilder', }) : buildExtensions = buildExtensions ?? appendExtension('.copy'), _build = build ?? _defaultBehavior, _extraWork = extraWork; - @override Future build(BuildStep buildStep) async { if (!await buildStep.canRead(buildStep.inputId)) return; @@ -125,4 +136,7 @@ class TestBuilder implements Builder { await _extraWork?.call(buildStep, buildExtensions); _buildsCompletedController.add(buildStep.inputId); } + + @override + String toString() => name; } diff --git a/build_test/lib/src/test_builder.dart b/build_test/lib/src/test_builder.dart index f17d2fcd2..b7f02d14a 100644 --- a/build_test/lib/src/test_builder.dart +++ b/build_test/lib/src/test_builder.dart @@ -160,6 +160,62 @@ Future testBuilder( /// Runs [builders] in a test environment. /// +/// Calls [testBuilderFactories] with factories that each return a member of +/// [builders], see that method for details. +/// +/// Because build config is passed via factories, this method does not read +/// build config. To test with build config, use [testBuilderFactories]. +Future testBuilders( + Iterable builders, + Map*/ Object> sourceAssets, { + Set? generateFor, + bool Function(String assetId)? isInput, + String? rootPackage, + Map|Matcher>*/ Object>? outputs, + void Function(LogRecord log)? onLog, + void Function(AssetId, Iterable)? reportUnusedAssetsForInput, + PackageConfig? packageConfig, + Resolvers? resolvers, + Set optionalBuilders = const {}, + Set visibleOutputBuilders = const {}, + bool testingBuilderConfig = true, + TestReaderWriter? readerWriter, + bool enableLowResourceMode = false, +}) { + final builderFactories = []; + final optionalBuilderFactories = Set.identity(); + final visibleOutputBuilderFactories = Set.identity(); + for (final builder in builders) { + Builder builderFactory(_) => builder; + builderFactories.add(builderFactory); + if (optionalBuilders.contains(builder)) { + optionalBuilderFactories.add(builderFactory); + } + if (visibleOutputBuilders.contains(builder)) { + visibleOutputBuilderFactories.add(builderFactory); + } + } + return testBuilderFactories( + builderFactories, + sourceAssets, + generateFor: generateFor, + isInput: isInput, + rootPackage: rootPackage, + outputs: outputs, + onLog: onLog, + reportUnusedAssetsForInput: reportUnusedAssetsForInput, + packageConfig: packageConfig, + resolvers: resolvers, + optionalBuilderFactories: optionalBuilderFactories, + visibleOutputBuilderFactories: visibleOutputBuilderFactories, + testingBuilderConfig: testingBuilderConfig, + readerWriter: readerWriter, + enableLowResourceMode: enableLowResourceMode, + ); +} + +/// Runs [builderFactories] in a test environment. +/// /// The test environment supplies in-memory build [sourceAssets] to the builders /// under test. /// @@ -197,12 +253,13 @@ Future testBuilder( /// Enabling of language experiments is supported through the /// `withEnabledExperiments` method from package:build. /// -/// To mark a builder as optional, add it to [optionalBuilders]. Optional -/// builders only run if their output is used by a non-optional builder. +/// To mark a builder as optional, add its builder to +/// [optionalBuilderFactories]. Optional builders only run if their output is +/// used by a non-optional builder. /// -/// To mark a builder's output as visible, add it to [visibleOutputBuilders]. -/// The builder then writes its outputs next to its input, instead of hidden -/// under `.dart_tool`. +/// To mark a builder's output as visible, add its factory to +/// [visibleOutputBuilderFactories]. The builder then writes its outputs next to +/// its input, instead of hidden under `.dart_tool`. /// /// The default builder config will be overwritten with one that causes the /// builder to run for all inputs. To use the default builder config instead, @@ -217,8 +274,8 @@ Future testBuilder( /// Returns a [TestBuilderResult] with the [BuildResult] and the /// [TestReaderWriter] used for the build, which can be used for further /// checks. -Future testBuilders( - Iterable builders, +Future testBuilderFactories( + Iterable builderFactories, Map*/ Object> sourceAssets, { Set? generateFor, bool Function(String assetId)? isInput, @@ -228,8 +285,8 @@ Future testBuilders( void Function(AssetId, Iterable)? reportUnusedAssetsForInput, PackageConfig? packageConfig, Resolvers? resolvers, - Set optionalBuilders = const {}, - Set visibleOutputBuilders = const {}, + Set optionalBuilderFactories = const {}, + Set visibleOutputBuilderFactories = const {}, bool testingBuilderConfig = true, TestReaderWriter? readerWriter, bool enableLowResourceMode = false, @@ -240,10 +297,18 @@ Future testBuilders( for (var descriptor in sourceAssets.keys) makeAssetId(descriptor), }; + if (inputIds.isEmpty && rootPackage == null) { + throw ArgumentError( + '`sourceAssets` is empty so `rootPackage` must be specified, ' + 'but `rootPackage` is null.', + ); + } + // Differentiate input packages and all packages. Builders run on input // packages; they can read/resolve all packages. Additional packages are // supplied by passing a `readerWriter`. - var inputPackages = {for (var id in inputIds) id.package}; + var inputPackages = + inputIds.isEmpty ? {rootPackage!} : {for (var id in inputIds) id.package}; final allPackages = inputPackages.toSet(); if (readerWriter != null) { for (final asset in readerWriter.testing.assets) { @@ -329,7 +394,11 @@ Future testBuilders( if (package != rootPackage) ...defaultNonRootVisibleAssets, ...inputIds - .where((id) => id.package == package) + .where( + (id) => + id.package == package && + !id.path.startsWith('.dart_tool/'), + ) .map((id) => Glob.quote(id.path)), ], }, @@ -346,16 +415,34 @@ Future testBuilders( deleteFilesByDefault: true, ); - final buildSeries = await BuildSeries.create(buildOptions, environment, [ - for (final builder in builders) + final builderApplications = []; + for (final builderFactory in builderFactories) { + // The real build gets the name from the `build.yaml` where the builder is + // For tests, use the builder class name, or fall back if the test makes the + // builder factory throw. + String name; + try { + name = builderName(builderFactory(const BuilderOptions({}))); + } catch (e) { + name = e.toString(); + } + builderApplications.add( apply( - builderName(builder), - [(_) => builder], + name, + [builderFactory], (p) => inputPackages.contains(p.name), - isOptional: optionalBuilders.contains(builder), - hideOutput: !visibleOutputBuilders.contains(builder), + isOptional: optionalBuilderFactories.contains(builderFactory), + hideOutput: !visibleOutputBuilderFactories.contains(builderFactory), ), - ], {}); + ); + } + + final buildSeries = await BuildSeries.create( + buildOptions, + environment, + builderApplications, + {}, + ); // Run the build. final buildResult = await buildSeries.run({}); diff --git a/build_test/pubspec.yaml b/build_test/pubspec.yaml index 08c57bb59..4268054ea 100644 --- a/build_test/pubspec.yaml +++ b/build_test/pubspec.yaml @@ -1,6 +1,6 @@ name: build_test description: Utilities for writing unit tests of Builders. -version: 3.3.0 +version: 3.4.0-wip repository: https://github.com/dart-lang/build/tree/master/build_test resolution: workspace