From 0ad8e81cd36659f0174806c87158515b540f08cd Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 10 Dec 2024 09:56:30 -0800 Subject: [PATCH 1/4] refactor package:native_assets_cli dep on package:test --- .../build/local_asset/test/build_test.dart | 25 ++-- .../lib/code_assets_testing.dart | 2 +- .../lib/src/code_assets/testing.dart | 30 ++-- pkgs/native_assets_cli/lib/test.dart | 136 +++++++++--------- pkgs/native_assets_cli/pubspec.yaml | 2 +- 5 files changed, 108 insertions(+), 87 deletions(-) diff --git a/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart b/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart index 8f2ea6180..a1515981c 100644 --- a/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart +++ b/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart @@ -8,16 +8,17 @@ import 'package:test/test.dart'; import '../hook/build.dart' as build; void main() async { - await testCodeBuildHook( - description: 'test my build hook', - mainMethod: build.main, - check: (_, output) { - expect(output.codeAssets, isNotEmpty); - expect( - output.codeAssets.first.id, - 'package:local_asset/asset.txt', - ); - }, - buildAssetTypes: [CodeAsset.type], - ); + test('test my build hook', () async { + await validateCodeBuildHook( + mainMethod: build.main, + check: (_, output) { + expect(output.codeAssets, isNotEmpty); + expect( + output.codeAssets.first.id, + 'package:local_asset/asset.txt', + ); + }, + buildAssetTypes: [CodeAsset.type], + ); + }); } diff --git a/pkgs/native_assets_cli/lib/code_assets_testing.dart b/pkgs/native_assets_cli/lib/code_assets_testing.dart index 148e875d2..d0c1b5640 100644 --- a/pkgs/native_assets_cli/lib/code_assets_testing.dart +++ b/pkgs/native_assets_cli/lib/code_assets_testing.dart @@ -3,4 +3,4 @@ // BSD-style license that can be found in the LICENSE file. export 'code_assets_builder.dart'; -export 'src/code_assets/testing.dart' show testCodeBuildHook; +export 'src/code_assets/testing.dart' show validateCodeBuildHook; diff --git a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart index 7bb8ef9f9..59d51a4df 100644 --- a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart +++ b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart @@ -4,15 +4,22 @@ import 'dart:async'; -import 'package:meta/meta.dart' show isTest; -import 'package:test/test.dart'; - import '../../code_assets_builder.dart'; import '../../test.dart'; -@isTest -Future testCodeBuildHook({ - required String description, +/// Validate a code build hook; this will throw an exception on validation +/// errors. +/// +/// This is intended to be used from tests, e.g.: +/// +/// ``` +/// test('test my build hook', () async { +/// await validateCodeBuildHook( +/// ... +/// ); +/// }); +/// ``` +Future validateCodeBuildHook({ // ignore: inference_failure_on_function_return_type required Function(List arguments) mainMethod, required FutureOr Function(BuildConfig, BuildOutput) check, @@ -28,8 +35,7 @@ Future testCodeBuildHook({ required List buildAssetTypes, bool? linkingEnabled, }) async { - await testBuildHook( - description: description, + await validateBuildHook( mainMethod: mainMethod, extraConfigSetup: (config) { config.setupCodeConfig( @@ -43,7 +49,13 @@ Future testCodeBuildHook({ ); }, check: (config, output) async { - expect(await validateCodeAssetBuildOutput(config, output), isEmpty); + final validationErrors = + await validateCodeAssetBuildOutput(config, output); + if (validationErrors.isNotEmpty) { + throw ValidationFailure( + 'encountered build output validation issues: $validationErrors'); + } + await check(config, output); }, buildMode: buildMode, diff --git a/pkgs/native_assets_cli/lib/test.dart b/pkgs/native_assets_cli/lib/test.dart index a9e4c289a..57bd09042 100644 --- a/pkgs/native_assets_cli/lib/test.dart +++ b/pkgs/native_assets_cli/lib/test.dart @@ -6,8 +6,6 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io'; -import 'package:meta/meta.dart' show isTest; -import 'package:test/test.dart'; import 'package:yaml/yaml.dart'; import 'native_assets_cli_builder.dart'; @@ -15,9 +13,28 @@ import 'native_assets_cli_internal.dart' show Hook; export 'native_assets_cli_builder.dart'; -@isTest -Future testBuildHook({ - required String description, +/// An exception thrown when validation fails. +class ValidationFailure implements Exception { + final String? message; + + ValidationFailure(this.message); + + @override + String toString() => message.toString(); +} + +/// Validate a build hook; this will throw an exception on validation errors. +/// +/// This is intended to be used from tests, e.g.: +/// +/// ``` +/// test('test my build hook', () async { +/// await validateCodeBuildHook( +/// ... +/// ); +/// }); +/// ``` +Future validateBuildHook({ required void Function(BuildConfigBuilder) extraConfigSetup, required FutureOr Function(List arguments) mainMethod, required FutureOr Function(BuildConfig config, BuildOutput output) @@ -27,50 +44,56 @@ Future testBuildHook({ List? buildAssetTypes, bool? linkingEnabled, }) async { - test( - description, - () async { - final tempDir = await _tempDirForTest(); - final outputDirectory = tempDir.resolve('output/'); - final outputDirectoryShared = tempDir.resolve('output_shared/'); - - await Directory.fromUri(outputDirectory).create(); - await Directory.fromUri(outputDirectoryShared).create(); - - final configBuilder = BuildConfigBuilder(); - configBuilder - ..setupHookConfig( - packageRoot: Directory.current.uri, - packageName: _readPackageNameFromPubspec(), - targetOS: targetOS ?? OS.current, - buildAssetTypes: buildAssetTypes ?? [], - buildMode: buildMode ?? BuildMode.release, - ) - ..setupBuildConfig( - dryRun: false, - linkingEnabled: true, - ) - ..setupBuildRunConfig( - outputDirectory: outputDirectory, - outputDirectoryShared: outputDirectoryShared, - ); - extraConfigSetup(configBuilder); - - final config = BuildConfig(configBuilder.json); - - final configUri = tempDir.resolve(Hook.build.outputName); - _writeJsonTo(configUri, config.json); - await mainMethod(['--config=${configUri.toFilePath()}']); - final output = BuildOutput( - _readJsonFrom(config.outputDirectory.resolve(Hook.build.outputName))); - - // Test conformance of protocol invariants. - expect(await validateBuildOutput(config, output), isEmpty); - - // Run user-defined tests. - check(config, output); - }, - ); + final tempDir = await Directory.systemTemp.createTemp(); + // Deal with Windows temp folder aliases. + final tempUri = + Directory(await tempDir.resolveSymbolicLinks()).uri.normalizePath(); + final outputDirectory = tempUri.resolve('output/'); + final outputDirectoryShared = tempUri.resolve('output_shared/'); + + await Directory.fromUri(outputDirectory).create(); + await Directory.fromUri(outputDirectoryShared).create(); + + final configBuilder = BuildConfigBuilder(); + configBuilder + ..setupHookConfig( + packageRoot: Directory.current.uri, + packageName: _readPackageNameFromPubspec(), + targetOS: targetOS ?? OS.current, + buildAssetTypes: buildAssetTypes ?? [], + buildMode: buildMode ?? BuildMode.release, + ) + ..setupBuildConfig( + dryRun: false, + linkingEnabled: true, + ) + ..setupBuildRunConfig( + outputDirectory: outputDirectory, + outputDirectoryShared: outputDirectoryShared, + ); + extraConfigSetup(configBuilder); + + try { + final config = BuildConfig(configBuilder.json); + + final configUri = tempUri.resolve(Hook.build.outputName); + _writeJsonTo(configUri, config.json); + await mainMethod(['--config=${configUri.toFilePath()}']); + final output = BuildOutput( + _readJsonFrom(config.outputDirectory.resolve(Hook.build.outputName))); + + // Test conformance of protocol invariants. + final validationErrors = await validateBuildOutput(config, output); + if (validationErrors.isNotEmpty) { + throw ValidationFailure( + 'encountered build output validation issues: $validationErrors'); + } + + // Run user-defined tests. + await check(config, output); + } finally { + tempDir.deleteSync(recursive: true); + } } void _writeJsonTo(Uri uri, Map json) { @@ -90,18 +113,3 @@ String _readPackageNameFromPubspec() { final yaml = loadYaml(readAsString) as YamlMap; return yaml['name'] as String; } - -const keepTempKey = 'KEEP_TEMPORARY_DIRECTORIES'; - -Future _tempDirForTest({String? prefix, bool keepTemp = false}) async { - final tempDir = await Directory.systemTemp.createTemp(prefix); - // Deal with Windows temp folder aliases. - final tempUri = - Directory(await tempDir.resolveSymbolicLinks()).uri.normalizePath(); - if ((!Platform.environment.containsKey(keepTempKey) || - Platform.environment[keepTempKey]!.isEmpty) && - !keepTemp) { - addTearDown(() => tempDir.delete(recursive: true)); - } - return tempUri; -} diff --git a/pkgs/native_assets_cli/pubspec.yaml b/pkgs/native_assets_cli/pubspec.yaml index 193e6783e..d72d7b095 100644 --- a/pkgs/native_assets_cli/pubspec.yaml +++ b/pkgs/native_assets_cli/pubspec.yaml @@ -23,10 +23,10 @@ dependencies: logging: ^1.2.0 meta: ^1.15.0 pub_semver: ^2.1.3 - test: ^1.25.7 yaml: ^3.1.2 # Used for reading pubspec.yaml to obtain the package name dev_dependencies: dart_flutter_team_lints: ^2.1.1 file_testing: ^3.0.0 glob: any + test: ^1.25.7 From 9b1073c3ab5663ecc207a3d4b2f4acc84b6587d3 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 10 Dec 2024 10:00:27 -0800 Subject: [PATCH 2/4] update the changelog --- pkgs/native_assets_cli/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkgs/native_assets_cli/CHANGELOG.md b/pkgs/native_assets_cli/CHANGELOG.md index 3b7baec21..6c17887cd 100644 --- a/pkgs/native_assets_cli/CHANGELOG.md +++ b/pkgs/native_assets_cli/CHANGELOG.md @@ -5,6 +5,11 @@ missing. This enables (1) code to run if an asset is missing but that code is not invoked at runtime, and (2) doing fallback implementations in Dart if an asset is missing. +- **Breaking change**: Rename `testBuildHook` to `validateBuildHook` and + `testCodeBuildHook` to `validateCodeBuildHook`. Instead of defining tests, + these methods should now be called from within tests. +- Move the `package:test` dependency from a regular dependency (exported to + calling packages) to a dev_dependency. - Update pubspec.yaml of examples to use 0.9.0 of `package:native_assets_cli`. - Consolidate [CodeAsset] specific things into `lib/src/code_assets/*` From 20742302c549b353bde9692ebffdc44743a6d369 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 10 Dec 2024 11:30:27 -0800 Subject: [PATCH 3/4] review feedback --- pkgs/native_assets_cli/CHANGELOG.md | 6 +- .../build/local_asset/test/build_test.dart | 2 +- .../lib/code_assets_testing.dart | 2 +- .../lib/src/code_assets/testing.dart | 8 +- pkgs/native_assets_cli/lib/test.dart | 88 ++++++++++--------- 5 files changed, 56 insertions(+), 50 deletions(-) diff --git a/pkgs/native_assets_cli/CHANGELOG.md b/pkgs/native_assets_cli/CHANGELOG.md index 6c17887cd..8b6cecb1d 100644 --- a/pkgs/native_assets_cli/CHANGELOG.md +++ b/pkgs/native_assets_cli/CHANGELOG.md @@ -5,9 +5,9 @@ missing. This enables (1) code to run if an asset is missing but that code is not invoked at runtime, and (2) doing fallback implementations in Dart if an asset is missing. -- **Breaking change**: Rename `testBuildHook` to `validateBuildHook` and - `testCodeBuildHook` to `validateCodeBuildHook`. Instead of defining tests, - these methods should now be called from within tests. +- **Breaking change**: Change the behavior of `testBuildHook` and + `testCodeBuildHook`; instead of defining tests, these methods should now be + called from within tests. - Move the `package:test` dependency from a regular dependency (exported to calling packages) to a dev_dependency. - Update pubspec.yaml of examples to use 0.9.0 of `package:native_assets_cli`. diff --git a/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart b/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart index a1515981c..7b8ad34d6 100644 --- a/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart +++ b/pkgs/native_assets_cli/example/build/local_asset/test/build_test.dart @@ -9,7 +9,7 @@ import '../hook/build.dart' as build; void main() async { test('test my build hook', () async { - await validateCodeBuildHook( + await testCodeBuildHook( mainMethod: build.main, check: (_, output) { expect(output.codeAssets, isNotEmpty); diff --git a/pkgs/native_assets_cli/lib/code_assets_testing.dart b/pkgs/native_assets_cli/lib/code_assets_testing.dart index d0c1b5640..148e875d2 100644 --- a/pkgs/native_assets_cli/lib/code_assets_testing.dart +++ b/pkgs/native_assets_cli/lib/code_assets_testing.dart @@ -3,4 +3,4 @@ // BSD-style license that can be found in the LICENSE file. export 'code_assets_builder.dart'; -export 'src/code_assets/testing.dart' show validateCodeBuildHook; +export 'src/code_assets/testing.dart' show testCodeBuildHook; diff --git a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart index 59d51a4df..a800cabfd 100644 --- a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart +++ b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart @@ -14,12 +14,12 @@ import '../../test.dart'; /// /// ``` /// test('test my build hook', () async { -/// await validateCodeBuildHook( +/// await testCodeBuildHook( /// ... /// ); /// }); /// ``` -Future validateCodeBuildHook({ +Future testCodeBuildHook({ // ignore: inference_failure_on_function_return_type required Function(List arguments) mainMethod, required FutureOr Function(BuildConfig, BuildOutput) check, @@ -35,7 +35,7 @@ Future validateCodeBuildHook({ required List buildAssetTypes, bool? linkingEnabled, }) async { - await validateBuildHook( + await testBuildHook( mainMethod: mainMethod, extraConfigSetup: (config) { config.setupCodeConfig( @@ -52,7 +52,7 @@ Future validateCodeBuildHook({ final validationErrors = await validateCodeAssetBuildOutput(config, output); if (validationErrors.isNotEmpty) { - throw ValidationFailure( + throw VerificationException( 'encountered build output validation issues: $validationErrors'); } diff --git a/pkgs/native_assets_cli/lib/test.dart b/pkgs/native_assets_cli/lib/test.dart index 57bd09042..48f43caad 100644 --- a/pkgs/native_assets_cli/lib/test.dart +++ b/pkgs/native_assets_cli/lib/test.dart @@ -13,28 +13,18 @@ import 'native_assets_cli_internal.dart' show Hook; export 'native_assets_cli_builder.dart'; -/// An exception thrown when validation fails. -class ValidationFailure implements Exception { - final String? message; - - ValidationFailure(this.message); - - @override - String toString() => message.toString(); -} - /// Validate a build hook; this will throw an exception on validation errors. /// /// This is intended to be used from tests, e.g.: /// /// ``` /// test('test my build hook', () async { -/// await validateCodeBuildHook( +/// await testCodeBuildHook( /// ... /// ); /// }); /// ``` -Future validateBuildHook({ +Future testBuildHook({ required void Function(BuildConfigBuilder) extraConfigSetup, required FutureOr Function(List arguments) mainMethod, required FutureOr Function(BuildConfig config, BuildOutput output) @@ -44,36 +34,39 @@ Future validateBuildHook({ List? buildAssetTypes, bool? linkingEnabled, }) async { + const keepTempKey = 'KEEP_TEMPORARY_DIRECTORIES'; + final tempDir = await Directory.systemTemp.createTemp(); - // Deal with Windows temp folder aliases. - final tempUri = - Directory(await tempDir.resolveSymbolicLinks()).uri.normalizePath(); - final outputDirectory = tempUri.resolve('output/'); - final outputDirectoryShared = tempUri.resolve('output_shared/'); - - await Directory.fromUri(outputDirectory).create(); - await Directory.fromUri(outputDirectoryShared).create(); - - final configBuilder = BuildConfigBuilder(); - configBuilder - ..setupHookConfig( - packageRoot: Directory.current.uri, - packageName: _readPackageNameFromPubspec(), - targetOS: targetOS ?? OS.current, - buildAssetTypes: buildAssetTypes ?? [], - buildMode: buildMode ?? BuildMode.release, - ) - ..setupBuildConfig( - dryRun: false, - linkingEnabled: true, - ) - ..setupBuildRunConfig( - outputDirectory: outputDirectory, - outputDirectoryShared: outputDirectoryShared, - ); - extraConfigSetup(configBuilder); try { + // Deal with Windows temp folder aliases. + final tempUri = + Directory(await tempDir.resolveSymbolicLinks()).uri.normalizePath(); + final outputDirectory = tempUri.resolve('output/'); + final outputDirectoryShared = tempUri.resolve('output_shared/'); + + await Directory.fromUri(outputDirectory).create(); + await Directory.fromUri(outputDirectoryShared).create(); + + final configBuilder = BuildConfigBuilder(); + configBuilder + ..setupHookConfig( + packageRoot: Directory.current.uri, + packageName: _readPackageNameFromPubspec(), + targetOS: targetOS ?? OS.current, + buildAssetTypes: buildAssetTypes ?? [], + buildMode: buildMode ?? BuildMode.release, + ) + ..setupBuildConfig( + dryRun: false, + linkingEnabled: true, + ) + ..setupBuildRunConfig( + outputDirectory: outputDirectory, + outputDirectoryShared: outputDirectoryShared, + ); + extraConfigSetup(configBuilder); + final config = BuildConfig(configBuilder.json); final configUri = tempUri.resolve(Hook.build.outputName); @@ -85,17 +78,30 @@ Future validateBuildHook({ // Test conformance of protocol invariants. final validationErrors = await validateBuildOutput(config, output); if (validationErrors.isNotEmpty) { - throw ValidationFailure( + throw VerificationException( 'encountered build output validation issues: $validationErrors'); } // Run user-defined tests. await check(config, output); } finally { - tempDir.deleteSync(recursive: true); + final keepTempDir = (Platform.environment[keepTempKey] ?? '').isNotEmpty; + if (!keepTempDir) { + tempDir.deleteSync(recursive: true); + } } } +/// An exception thrown when build hook verification fails. +class VerificationException implements Exception { + final String? message; + + VerificationException(this.message); + + @override + String toString() => message.toString(); +} + void _writeJsonTo(Uri uri, Map json) { final encoder = const JsonEncoder().fuse(const Utf8Encoder()); File.fromUri(uri).writeAsBytesSync(encoder.convert(json)); From a157dbe59a77462e81715bc2c2f99db824136290 Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Tue, 10 Dec 2024 13:00:30 -0800 Subject: [PATCH 4/4] create and throw ValidationFailure --- .../lib/src/code_assets/testing.dart | 3 ++- pkgs/native_assets_cli/lib/src/validation.dart | 10 ++++++++++ pkgs/native_assets_cli/lib/test.dart | 13 ++----------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart index a800cabfd..e7231ea9d 100644 --- a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart +++ b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart @@ -6,6 +6,7 @@ import 'dart:async'; import '../../code_assets_builder.dart'; import '../../test.dart'; +import '../validation.dart'; /// Validate a code build hook; this will throw an exception on validation /// errors. @@ -52,7 +53,7 @@ Future testCodeBuildHook({ final validationErrors = await validateCodeAssetBuildOutput(config, output); if (validationErrors.isNotEmpty) { - throw VerificationException( + throw ValidationFailure( 'encountered build output validation issues: $validationErrors'); } diff --git a/pkgs/native_assets_cli/lib/src/validation.dart b/pkgs/native_assets_cli/lib/src/validation.dart index c20edd3fd..72c01c7a3 100644 --- a/pkgs/native_assets_cli/lib/src/validation.dart +++ b/pkgs/native_assets_cli/lib/src/validation.dart @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:io'; + import '../native_assets_cli_builder.dart'; typedef ValidationErrors = List; @@ -99,3 +100,12 @@ List _validateAssetsForLinking( } return errors; } + +class ValidationFailure implements Exception { + final String? message; + + ValidationFailure(this.message); + + @override + String toString() => message.toString(); +} diff --git a/pkgs/native_assets_cli/lib/test.dart b/pkgs/native_assets_cli/lib/test.dart index 48f43caad..a2f130ab9 100644 --- a/pkgs/native_assets_cli/lib/test.dart +++ b/pkgs/native_assets_cli/lib/test.dart @@ -10,6 +10,7 @@ import 'package:yaml/yaml.dart'; import 'native_assets_cli_builder.dart'; import 'native_assets_cli_internal.dart' show Hook; +import 'src/validation.dart'; export 'native_assets_cli_builder.dart'; @@ -78,7 +79,7 @@ Future testBuildHook({ // Test conformance of protocol invariants. final validationErrors = await validateBuildOutput(config, output); if (validationErrors.isNotEmpty) { - throw VerificationException( + throw ValidationFailure( 'encountered build output validation issues: $validationErrors'); } @@ -92,16 +93,6 @@ Future testBuildHook({ } } -/// An exception thrown when build hook verification fails. -class VerificationException implements Exception { - final String? message; - - VerificationException(this.message); - - @override - String toString() => message.toString(); -} - void _writeJsonTo(Uri uri, Map json) { final encoder = const JsonEncoder().fuse(const Utf8Encoder()); File.fromUri(uri).writeAsBytesSync(encoder.convert(json));