Merge pull request #52 from Workiva/smart-output-of-backwards-compat-…

…file

FEDX-1987: Only generate backwards-compatible output when necessary

.github/workflows/dart_ci.yaml

@@ -36,8 +36,7 @@ jobs:
       - run: dart pub get
       - run: dart run dependency_validator
       - run: dart analyze
-      - run: dart test test/lib
-      - run: dart test test/bin
+      - run: dart test
       - uses: anchore/sbom-action@v0
         if: ${{ matrix.sdk == 'stable' }}
         with:

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 3.1.0
+
+- The `test/dart_test.browser_aggregate.yaml` file was previously always
+generated for backwards-compatibility. With this release, it is only generated
+if a reference to it is found in `dart_test.yaml`.
+
 ## 3.0.3
 
 - Compatible with Dart 3.

bin/browser_aggregate_tests.dart

@@ -23,7 +23,10 @@ final argParser = ArgParser()
   ..addOption('build-args',
       help: 'Args to pass to the build runner process.\n'
           'Run "dart run build_runner build -h -v" to see all available '
-          'options.');
+          'options.')
+  ..addOption('test-args',
+      help: 'Args to pass to the dart test process.\n'
+          'Run "dart test -h" to see all available options.');
 
 enum Mode {
   // Print build and test args separated by `--`
@@ -62,6 +65,7 @@ void main(List<String> args) async {
 
   final bool? release = parsed['release'];
   final String? buildArgs = parsed['build-args'];
+  final String? testArgs = parsed['test-args'];
 
   buildAggregateTestYaml(mode, userBuildArgs: buildArgs);
   final testPaths = parseAggregateTestPaths(mode);
@@ -70,7 +74,8 @@ void main(List<String> args) async {
   } else if (mode == Mode.build) {
     await buildTests(testPaths, release: release, userBuildArgs: buildArgs);
   } else {
-    await runTests(testPaths, release: release, userBuildArgs: buildArgs);
+    await runTests(testPaths,
+        release: release, userBuildArgs: buildArgs, userTestArgs: testArgs);
   }
 }
 
@@ -187,7 +192,7 @@ Future<void> buildTests(List<String> testPaths,
 ///
 /// Includes `--release` if [release] is true.
 Future<void> runTests(List<String> testPaths,
-    {bool? release, String? userBuildArgs}) async {
+    {bool? release, String? userBuildArgs, String? userTestArgs}) async {
   final executable = 'dart';
   final args = [
     'run',
@@ -196,6 +201,7 @@ Future<void> runTests(List<String> testPaths,
     ...buildRunnerBuildArgs(testPaths,
         release: release, userBuildArgs: userBuildArgs),
     '--',
+    ...?userTestArgs?.split(' '),
     testPreset,
   ];
   stdout

lib/src/builder.dart

@@ -333,9 +333,39 @@ class DartTestYamlBuilder extends Builder {
         AssetId(buildStep.inputId.package, 'dart_test.browser_aggregate.yaml');
     await buildStep.writeAsString(outputId, contents.toString());
 
-    final backwardsCompatOutputId = AssetId(
-        buildStep.inputId.package, 'test/dart_test.browser_aggregate.yaml');
-    await buildStep.writeAsString(backwardsCompatOutputId, contents.toString());
+    var shouldWriteBackwardsCompatOutput = false;
+    try {
+      final dartTestYamlId =
+          AssetId(buildStep.inputId.package, 'dart_test.yaml');
+      final dartTestYaml = await buildStep.readAsString(dartTestYamlId);
+      if (dartTestYaml.contains('test/dart_test.browser_aggregate.yaml')) {
+        log.warning(
+            'Found `test/dart_test.browser_aggregate.yaml` in `dart_test.yaml`, will generate it for backwards-compatibility.\n'
+            'Please update your `dart_test.yaml` to include `dart_test.browser_aggregate.yaml` instead.');
+        shouldWriteBackwardsCompatOutput = true;
+      } else {
+        log.fine(
+            'No `test/dart_test.browser_aggregate.yaml` found in `dart_test.yaml`, skipping backwards-compatible generation.');
+      }
+    } on AssetNotFoundException catch (_) {
+      log.fine(
+          'No `dart_test.yaml` found, skipping backwards-compatible generation of `test/dart_test.browser_aggregate.yaml`.');
+    } catch (e) {
+      log.fine(
+          'Error reading `dart_test.yaml`, skipping backwards-compatible generation of `test/dart_test.browser_aggregate.yaml`.',
+          e);
+    }
+
+    if (shouldWriteBackwardsCompatOutput) {
+      final backwardsCompatOutputId = AssetId(
+        buildStep.inputId.package,
+        'test/dart_test.browser_aggregate.yaml',
+      );
+      await buildStep.writeAsString(
+        backwardsCompatOutputId,
+        contents.toString(),
+      );
+    }
   }
 }
 

pubspec.yaml

@@ -20,7 +20,7 @@ dependencies:
   json_annotation: ^4.1.0
   path: ^1.8.0
   test: ^1.17.12
-  test_core: ">=0.4.2 <0.6.0"
+  test_core: ">=0.4.2 <0.7.0"
   yaml: ^3.1.0
 
 dev_dependencies:

test/lib/dart_test_yaml_builder_test.dart

@@ -38,6 +38,32 @@ void main() {
     paths:
       - test/foo_template.browser_aggregate_test.dart
       - test/custom_test.dart
+'''
+      });
+    });
+
+    test('generates the backwards-compatible yaml output when needed',
+        () async {
+      final config = TestHtmlBuilderConfig(browserAggregation: true);
+      final builder = DartTestYamlBuilder();
+      await testBuilder(builder, {
+        'a|dart_test.yaml': 'include: "test/dart_test.browser_aggregate.yaml"',
+        'a|test/test_html_builder_config.json': jsonEncode(config),
+        'a|test/foo_template.browser_aggregate_test.dart': '',
+        'a|test/foo_template.html': '',
+        // This template should be found, but ignored because there is no
+        // accompanying .browser_aggregate_test.dart
+        'a|test/bar_template.html': '',
+        // This test should get included because it has a custom HTML
+        'a|test/custom_test.dart': '',
+        'a|test/custom_test.custom.html': '',
+      }, outputs: {
+        'a|dart_test.browser_aggregate.yaml': '''presets:
+  browser-aggregate:
+    platforms: [chrome]
+    paths:
+      - test/foo_template.browser_aggregate_test.dart
+      - test/custom_test.dart
 ''',
         'a|test/dart_test.browser_aggregate.yaml': '''presets:
   browser-aggregate:

tool/test_example.sh

@@ -3,12 +3,12 @@
 set -e
 
 cd example/project
-dart pub get
+dart pub upgrade
 dart run build_runner build --delete-conflicting-outputs
-dart run build_runner test
-dart run test_html_builder:browser_aggregate_tests
+dart run build_runner test -- --concurrency=1 # concurrency=1 is a workaround for this issue: https://github.com/dart-lang/test/issues/2294
+dart run test_html_builder:browser_aggregate_tests --test-args="--concurrency=1" # concurrency=1 is a workaround for this issue: https://github.com/dart-lang/test/issues/2294
 
 # These commands target a scenario where there is no build cache and someone
 # uses build filters to run a subset of tests.
 dart run build_runner clean
-dart run build_runner test --delete-conflicting-outputs --build-filter="test/unit/css_test.**" -- test/unit/css_test.dart
+dart run build_runner test --delete-conflicting-outputs --build-filter="test/unit/css_test.**" -- --concurrency=1 test/unit/css_test.dart # concurrency=1 is a workaround for this issue: https://github.com/dart-lang/test/issues/2294