Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[native_assets_cli] Make package:test dev dep #1799

Merged
merged 5 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkgs/native_assets_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**: 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`.
- Consolidate [CodeAsset] specific things into `lib/src/code_assets/*`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 testCodeBuildHook(
mainMethod: build.main,
check: (_, output) {
expect(output.codeAssets, isNotEmpty);
expect(
output.codeAssets.first.id,
'package:local_asset/asset.txt',
);
},
buildAssetTypes: [CodeAsset.type],
);
});
}
26 changes: 19 additions & 7 deletions pkgs/native_assets_cli/lib/src/code_assets/testing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// 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 testCodeBuildHook(
/// ...
/// );
/// });
/// ```
Future<void> testCodeBuildHook({
required String description,
// ignore: inference_failure_on_function_return_type
required Function(List<String> arguments) mainMethod,
required FutureOr<void> Function(BuildConfig, BuildOutput) check,
Expand All @@ -29,7 +36,6 @@ Future<void> testCodeBuildHook({
bool? linkingEnabled,
}) async {
await testBuildHook(
description: description,
mainMethod: mainMethod,
extraConfigSetup: (config) {
config.setupCodeConfig(
Expand All @@ -43,7 +49,13 @@ Future<void> testCodeBuildHook({
);
},
check: (config, output) async {
expect(await validateCodeAssetBuildOutput(config, output), isEmpty);
final validationErrors =
await validateCodeAssetBuildOutput(config, output);
if (validationErrors.isNotEmpty) {
throw VerificationException(
'encountered build output validation issues: $validationErrors');
}

await check(config, output);
},
buildMode: buildMode,
Expand Down
140 changes: 77 additions & 63 deletions pkgs/native_assets_cli/lib/test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,25 @@ 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';
import 'native_assets_cli_internal.dart' show Hook;

export 'native_assets_cli_builder.dart';

@isTest
/// 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 testCodeBuildHook(
/// ...
/// );
/// });
/// ```
Future<void> testBuildHook({
required String description,
required void Function(BuildConfigBuilder) extraConfigSetup,
required FutureOr<void> Function(List<String> arguments) mainMethod,
required FutureOr<void> Function(BuildConfig config, BuildOutput output)
Expand All @@ -27,50 +34,72 @@ Future<void> testBuildHook({
List<String>? 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);
},
);
const keepTempKey = 'KEEP_TEMPORARY_DIRECTORIES';

final tempDir = await Directory.systemTemp.createTemp();

try {
devoncarew marked this conversation as resolved.
Show resolved Hide resolved
// 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);
_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 VerificationException(
'encountered build output validation issues: $validationErrors');
}

// Run user-defined tests.
await check(config, output);
} finally {
final keepTempDir = (Platform.environment[keepTempKey] ?? '').isNotEmpty;
if (!keepTempDir) {
tempDir.deleteSync(recursive: true);
}
}
}

/// An exception thrown when build hook verification fails.
class VerificationException implements Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this class should be part of the public API. I don't believe users should catch it. So maybe it should an Error instead? Error itself has a message, so we could even throw Error instead of _VerificationError.

Or do you believe users should catch this?

Copy link
Member Author

@devoncarew devoncarew Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I looked into this a bit more. Error itself doesn't have a message; its subclasses do. None of those look like great candidates to throw, and, it looks like expect throws an Exception, not an Error, so I think we should do the same.

I updated this to be a ValidationFailure, implementing an Exception and not an error. I also moved it to a place in the code where it won't be public API.

final String? message;

VerificationException(this.message);

@override
String toString() => message.toString();
}

void _writeJsonTo(Uri uri, Map<String, Object?> json) {
Expand All @@ -90,18 +119,3 @@ String _readPackageNameFromPubspec() {
final yaml = loadYaml(readAsString) as YamlMap;
return yaml['name'] as String;
}

const keepTempKey = 'KEEP_TEMPORARY_DIRECTORIES';
devoncarew marked this conversation as resolved.
Show resolved Hide resolved

Future<Uri> _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;
}
2 changes: 1 addition & 1 deletion pkgs/native_assets_cli/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading