diff --git a/build_runner/CHANGELOG.md b/build_runner/CHANGELOG.md index 1ef84f6aa1..7c0dfb6364 100644 --- a/build_runner/CHANGELOG.md +++ b/build_runner/CHANGELOG.md @@ -1,3 +1,7 @@ +## 2.9.1-wip + +- Internal changes for `build_test`. + ## 2.9.0 - Watch mode: handle builder code and config changes without recompiling or diff --git a/build_runner/lib/src/build/build.dart b/build_runner/lib/src/build/build.dart index 8f5b9f72c8..eaec366f64 100644 --- a/build_runner/lib/src/build/build.dart +++ b/build_runner/lib/src/build/build.dart @@ -190,9 +190,11 @@ class Build { } _resolvers.reset(); - buildLog.finishBuild( - result: result.status == BuildStatus.success, - outputs: result.outputs.length, + result = result.copyWith( + errors: buildLog.finishBuild( + result: result.status == BuildStatus.success, + outputs: result.outputs.length, + ), ); return result; } diff --git a/build_runner/lib/src/build/build_result.dart b/build_runner/lib/src/build/build_result.dart index 9d859abec0..2edb8d849e 100644 --- a/build_runner/lib/src/build/build_result.dart +++ b/build_runner/lib/src/build/build_result.dart @@ -18,6 +18,9 @@ class BuildResult { /// The type of failure. final FailureType? failureType; + /// Errors reported. + final BuiltList errors; + /// All outputs created/updated during this build. final BuiltList outputs; @@ -30,6 +33,7 @@ class BuildResult { BuildResult({ required this.status, + BuiltList? errors, BuiltList? outputs, required this.buildOutputReader, this.performance, @@ -38,16 +42,21 @@ class BuildResult { failureType == null && status == BuildStatus.failure ? FailureType.general : failureType, + errors = errors ?? BuiltList(), outputs = outputs ?? BuiltList(); - BuildResult copyWith({BuildStatus? status, FailureType? failureType}) => - BuildResult( - status: status ?? this.status, - outputs: outputs, - buildOutputReader: buildOutputReader, - performance: performance, - failureType: failureType ?? this.failureType, - ); + BuildResult copyWith({ + BuildStatus? status, + FailureType? failureType, + BuiltList? errors, + }) => BuildResult( + status: status ?? this.status, + errors: errors ?? this.errors, + outputs: outputs, + buildOutputReader: buildOutputReader, + performance: performance, + failureType: failureType ?? this.failureType, + ); @override String toString() { diff --git a/build_runner/lib/src/logging/build_log.dart b/build_runner/lib/src/logging/build_log.dart index 72a6bcaf38..94537be092 100644 --- a/build_runner/lib/src/logging/build_log.dart +++ b/build_runner/lib/src/logging/build_log.dart @@ -5,6 +5,7 @@ import 'dart:math'; import 'package:build/build.dart' show AssetId; +import 'package:built_collection/built_collection.dart'; import '../bootstrap/build_process_state.dart'; import '../build_plan/phase.dart'; @@ -67,6 +68,9 @@ class BuildLog { /// The messages logged, bucketed by build phase. final BuildLogMessages _messages = BuildLogMessages(); + /// Errors logged. + final ListBuilder _errors = ListBuilder(); + /// Progress by build phase. final Map _phaseProgress = {}; @@ -167,6 +171,7 @@ class BuildLog { /// Logs an `build_runner` error. void error(String message) { + _errors.add(message); if (_display.displayingBlocks) { _messages.add(severity: Severity.error, message); _display.block(render()); @@ -182,6 +187,7 @@ class BuildLog { String? phaseName, String? context, }) { + if (severity == Severity.error) _errors.add(message); if (_display.displayingBlocks) { _messages.add( severity: severity, @@ -339,7 +345,9 @@ class BuildLog { } /// Logs that the build has finished with [result] and the count of [outputs]. - void finishBuild({required bool result, required int outputs}) { + /// + /// Returns the list of errors logged. + BuiltList finishBuild({required bool result, required int outputs}) { _tick(); final displayingBlocks = _display.displayingBlocks; _status = [ @@ -362,6 +370,10 @@ class BuildLog { AnsiBufferLine(_status).toString(), ); } + + final errors = _errors.build(); + _errors.clear(); + return errors; } /// Renders [message] with optional [error] and [stackTrace]. diff --git a/build_runner/pubspec.yaml b/build_runner/pubspec.yaml index c2c15f8f10..ae533cd8c3 100644 --- a/build_runner/pubspec.yaml +++ b/build_runner/pubspec.yaml @@ -1,5 +1,5 @@ name: build_runner -version: 2.9.0 +version: 2.9.1-wip description: A build system for Dart code generation and modular compilation. repository: https://github.com/dart-lang/build/tree/master/build_runner resolution: workspace diff --git a/build_runner/test/build/build_error_test.dart b/build_runner/test/build/build_error_test.dart index af51ad467b..254bec614e 100644 --- a/build_runner/test/build/build_error_test.dart +++ b/build_runner/test/build/build_error_test.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'package:build/build.dart'; -import 'package:build_runner/src/build/build_result.dart'; import 'package:build_test/build_test.dart'; import 'package:logging/logging.dart'; import 'package:test/test.dart'; @@ -14,14 +13,13 @@ import '../common/common.dart'; void main() { test('should fail if a severe logged', () async { - expect( - (await testBuilders( - [_LoggingBuilder(Level.SEVERE)], - {'a|lib/a.dart': ''}, - outputs: {'a|lib/a.dart.empty': ''}, - )).buildResult.status, - BuildStatus.failure, + final result = await testBuilders( + [_LoggingBuilder(Level.SEVERE)], + {'a|lib/a.dart': ''}, + outputs: {'a|lib/a.dart.empty': ''}, ); + expect(result.succeeded, false); + expect(result.errors, ['Log message for a|lib/a.dart.']); }); test('should fail if a severe was logged on a previous build', () async { @@ -30,7 +28,8 @@ void main() { {'a|lib/a.dart': ''}, outputs: {'a|lib/a.dart.empty': ''}, ); - expect(result.buildResult.status, BuildStatus.failure); + expect(result.succeeded, false); + expect(result.errors, ['Log message for a|lib/a.dart.']); final builder = _LoggingBuilder(Level.SEVERE); result = await testBuilders( @@ -40,7 +39,8 @@ void main() { readerWriter: result.readerWriter, outputs: {'a|lib/a.dart.empty': ''}, ); - expect(result.buildResult.status, BuildStatus.failure); + expect(result.succeeded, false); + expect(result.errors, ['Log message for a|lib/a.dart.']); // Should have failed without actually building again. expect(builder.built, false); }); @@ -53,7 +53,8 @@ void main() { {'a|lib/a.dart': ''}, outputs: {'a|lib/a.dart.empty': ''}, ); - expect(result.buildResult.status, BuildStatus.failure); + expect(result.succeeded, false); + expect(result.errors, ['Log message for a|lib/a.dart.']); result = await testBuilders( [_LoggingBuilder(Level.WARNING)], @@ -62,18 +63,18 @@ void main() { readerWriter: result.readerWriter, outputs: {'a|lib/a.dart.empty': ''}, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); + expect(result.errors, isEmpty); }, ); test('should fail if an exception is thrown', () async { - expect( - (await testBuilders( - [TestBuilder(build: (_, _) => throw Exception('Some build failure'))], - {'a|lib/a.txt': ''}, - )).buildResult.status, - BuildStatus.failure, + final result = await testBuilders( + [TestBuilder(build: (_, _) => throw Exception('Some build failure'))], + {'a|lib/a.txt': ''}, ); + expect(result.succeeded, false); + expect(result.errors, ['Exception: Some build failure']); }); test( @@ -110,7 +111,8 @@ void main() { ], {'a|lib/a.txt': ''}, ); - expect(result.buildResult.status, BuildStatus.failure); + expect(result.succeeded, false); + expect(result.errors, ['Wrote an output then failed']); }, ); } @@ -124,7 +126,7 @@ class _LoggingBuilder implements Builder { @override Future build(BuildStep buildStep) async { built = true; - log.log(level, buildStep.inputId.toString()); + log.log(level, 'Log message for ${buildStep.inputId}.'); await buildStep.canRead(buildStep.inputId); await buildStep.writeAsString(buildStep.inputId.addExtension('.empty'), ''); } diff --git a/build_runner/test/build/build_test.dart b/build_runner/test/build/build_test.dart index 02b17605ea..49e0b73caa 100644 --- a/build_runner/test/build/build_test.dart +++ b/build_runner/test/build/build_test.dart @@ -889,8 +889,8 @@ targets: (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, + )).succeeded, + false, ); }); diff --git a/build_runner/test/build/write_cache_test.dart b/build_runner/test/build/write_cache_test.dart index 7ec65160d9..a2a84cb625 100644 --- a/build_runner/test/build/write_cache_test.dart +++ b/build_runner/test/build/write_cache_test.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'package:build/build.dart'; -import 'package:build_runner/src/build/build_result.dart'; import 'package:build_test/build_test.dart'; import 'package:test/test.dart'; @@ -25,7 +24,7 @@ void main() { readerWriter: readerWriter, enableLowResourceMode: true, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); }); test('with caching writes wait until build completion', () async { @@ -40,7 +39,7 @@ void main() { readerWriter: readerWriter, enableLowResourceMode: false, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); expect( readerWriter.testing.assets, containsAll([ @@ -66,7 +65,7 @@ void main() { readerWriter: readerWriter, enableLowResourceMode: false, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); }); } diff --git a/build_runner/test/build_plan/build_triggers_test.dart b/build_runner/test/build_plan/build_triggers_test.dart index 67aade4700..129e54805f 100644 --- a/build_runner/test/build_plan/build_triggers_test.dart +++ b/build_runner/test/build_plan/build_triggers_test.dart @@ -6,7 +6,6 @@ import 'package:analyzer/dart/analysis/utilities.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:build/build.dart'; import 'package:build_config/build_config.dart'; -import 'package:build_runner/src/build/build_result.dart'; import 'package:build_runner/src/build_plan/build_triggers.dart'; import 'package:build_test/build_test.dart'; import 'package:test/test.dart'; @@ -250,7 +249,7 @@ triggers: outputs: {}, testingBuilderConfig: false, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); }); test('trigger builder in same file', () async { @@ -295,7 +294,7 @@ triggers: outputs: {}, testingBuilderConfig: false, ); - expect(result.buildResult.status, BuildStatus.success); + expect(result.succeeded, true); }); test('trigger builder in part file', () async { diff --git a/build_test/CHANGELOG.md b/build_test/CHANGELOG.md index 93189390a2..fa5e75b97a 100644 --- a/build_test/CHANGELOG.md +++ b/build_test/CHANGELOG.md @@ -1,3 +1,10 @@ +## 3.5.0-wip + +- Improve `TestBuilderResult`: add `succeeded`, `outputs` and `errors`. + Deprecate `buildResult` in favor of these new members. +- Add `verbose` to `testBuilders` and related methods. Like the command line + flag it enables info logging from builders. + ## 3.4.1 - Use `build_runner` 2.9.0. diff --git a/build_test/lib/src/test_builder.dart b/build_test/lib/src/test_builder.dart index e121086c0d..1ff44e0c41 100644 --- a/build_test/lib/src/test_builder.dart +++ b/build_test/lib/src/test_builder.dart @@ -137,6 +137,7 @@ Future testBuilder( PackageConfig? packageConfig, Resolvers? resolvers, TestReaderWriter? readerWriter, + bool verbose = false, bool enableLowResourceMode = false, }) async { return testBuilders( @@ -151,6 +152,7 @@ Future testBuilder( packageConfig: packageConfig, resolvers: resolvers, readerWriter: readerWriter, + verbose: verbose, enableLowResourceMode: enableLowResourceMode, ); } @@ -182,6 +184,7 @@ Future testBuilders( Map> appliesBuilders = const {}, bool testingBuilderConfig = true, TestReaderWriter? readerWriter, + bool verbose = false, bool enableLowResourceMode = false, }) { final builderFactories = []; @@ -222,6 +225,7 @@ Future testBuilders( appliesBuilders: appliesBuildersToFactories, testingBuilderConfig: testingBuilderConfig, readerWriter: readerWriter, + verbose: verbose, enableLowResourceMode: enableLowResourceMode, ); } @@ -283,6 +287,9 @@ Future testBuilders( /// Optionally pass [readerWriter] to set the filesystem that will be used /// during the build. Before the build, [sourceAssets] will be written to it. /// +/// Optionally pass [verbose], which acts like the command line flag: it enables +/// info logging from builders. +/// /// Optionally pass [enableLowResourceMode], which acts like the command /// line flag; in particular it disables file caching. /// @@ -306,6 +313,7 @@ Future testBuilderFactories( Map> appliesBuilders = const {}, bool testingBuilderConfig = true, TestReaderWriter? readerWriter, + bool verbose = false, bool enableLowResourceMode = false, }) async { onLog ??= _printOnFailureOrWrite; @@ -352,6 +360,7 @@ Future testBuilderFactories( buildLog.configuration = buildLog.configuration.rebuild((b) { b.onLog = onLog; + b.verbose = verbose; }); resolvers ??= packageConfig == null && enabledExperiments.isEmpty @@ -459,6 +468,7 @@ Future testBuilderFactories( // ignore: invalid_use_of_visible_for_testing_member buildOptions: BuildOptions.forTests( enableLowResourcesMode: enableLowResourceMode, + verbose: verbose, ), testingOverrides: testingOverrides, ); @@ -487,9 +497,23 @@ Future testBuilderFactories( } class TestBuilderResult { + @Deprecated('Use `succeeded`, `outputs` and `errors` instead.') final BuildResult buildResult; final TestReaderWriter readerWriter; + bool get succeeded => buildResult.status == BuildStatus.success; + + /// The files that were written. + /// + /// If the build was an incremental build, outputs of the previous build that + /// are reused are _not_ included here. + BuiltList get outputs => buildResult.outputs; + + /// The errors that were shown. + /// + /// If the build failed there must be at least one error. + BuiltList get errors => buildResult.errors; + TestBuilderResult({required this.buildResult, required this.readerWriter}); } diff --git a/build_test/pubspec.yaml b/build_test/pubspec.yaml index 315a8b09fd..9e9ff45b12 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.4.1 +version: 3.5.0-wip repository: https://github.com/dart-lang/build/tree/master/build_test resolution: workspace @@ -10,7 +10,7 @@ environment: dependencies: build: '4.0.1' build_config: ^1.0.0 - build_runner: '2.9.0' + build_runner: '2.9.1-wip' built_collection: ^5.1.1 crypto: ^3.0.0 glob: ^2.0.0 diff --git a/build_test/test/test_builder_test.dart b/build_test/test/test_builder_test.dart index f06d865dce..63349b695a 100644 --- a/build_test/test/test_builder_test.dart +++ b/build_test/test/test_builder_test.dart @@ -394,6 +394,95 @@ import 'package:glob/glob.dart'; ); expect(logs, isEmpty); }); + + test('info messages are not logged, warnings are', () async { + final logs = []; + await testBuilders( + [ + TestBuilder( + build: (buildStep, _) async { + log.info('not logged by default'); + log.warning('is logged by default'); + }, + buildExtensions: { + '.dart': ['.g.dart'], + }, + ), + ], + {'test_package|lib/a.dart': ''}, + onLog: (record) { + logs.add(record.toString()); + }, + ); + expect(logs.join('\n'), isNot(contains('not logged by default'))); + expect(logs.join('\n'), contains('is logged by default')); + }); + + test('info messages are logged if `verbose`', () async { + final logs = []; + await testBuilders( + [ + TestBuilder( + build: (buildStep, _) async { + log.info('is now logged'); + }, + buildExtensions: { + '.dart': ['.g.dart'], + }, + ), + ], + {'test_package|lib/a.dart': ''}, + onLog: (record) { + logs.add(record.toString()); + }, + verbose: true, + ); + expect(logs.join('\n'), contains('is now logged')); + }); + + test('severe messages are logged and returned as errors', () async { + final logs = []; + final result = await testBuilders( + [ + TestBuilder( + build: (buildStep, _) async { + log.severe('some error'); + }, + buildExtensions: { + '.dart': ['.g.dart'], + }, + ), + ], + {'test_package|lib/a.dart': ''}, + onLog: (record) { + if (record.level == Level.SEVERE) logs.add(record.toString()); + }, + ); + expect(logs.join('\n'), contains('some error')); + expect(result.errors.join('\n'), contains('some error')); + }); + + test('exceptions are logged and returned as errors', () async { + final logs = []; + final result = await testBuilders( + [ + TestBuilder( + build: (buildStep, _) async { + throw Exception('some exception'); + }, + buildExtensions: { + '.dart': ['.g.dart'], + }, + ), + ], + {'test_package|lib/a.dart': ''}, + onLog: (record) { + if (record.level == Level.SEVERE) logs.add(record.toString()); + }, + ); + expect(logs.join('\n'), contains('Exception: some exception')); + expect(result.errors.join('\n'), contains('Exception: some exception')); + }); } /// Concatenates the contents of multiple text files into a single output.