Fix an issue with failed assertions using setUpAll/tearDownAll (#2499)

When groups are filtered using `filter()` and not `onPlatform`, we would clone the group and tests without also cloning `setUpAll`/`tearDownAll`, which led to an assertion failure when trying to set the parents.

This change ensures we clone tests in `filter()` like we do in `forPlatform()`.

Fixes #2498

Author: DanTup

pkgs/test/CHANGELOG.md

@@ -1,6 +1,8 @@
-## 1.26.1-wip
+## 1.26.1
 
 * Set a debug name for test isolates.
+* Fix an assertion failure when using `setUpAll` or `tearDownAll` and running
+  with asserts enabled.
 
 ## 1.26.0
 

pkgs/test/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test
-version: 1.26.1-wip
+version: 1.26.1
 description: >-
   A full featured library for writing and running Dart tests across platforms.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test
@@ -36,8 +36,8 @@ dependencies:
   stream_channel: ^2.1.0
 
   # Use an exact version until the test_api and test_core package are stable.
-  test_api: 0.7.5
-  test_core: 0.6.10-wip
+  test_api: 0.7.6
+  test_core: 0.6.10
 
   typed_data: ^1.3.0
   web_socket_channel: '>=2.0.0 <4.0.0'

pkgs/test/test/runner/set_up_all_test.dart

@@ -95,4 +95,19 @@ void main() {
     expect(test.stdout, neverEmits(contains('(setUpAll)')));
     await test.shouldExit(0);
   });
+
+  test('does not fail with asserts enabled - issue #2498', () async {
+    await d.file('test.dart', r'''
+        import 'package:test/test.dart';
+
+        void main() {
+          setUpAll(() {});
+
+          test("test", () {});
+        }
+        ''').create();
+
+    var test = await runTest(['test.dart'], vmArgs: ['--enable-asserts']);
+    await test.shouldExit(0);
+  });
 }

pkgs/test/test/runner/tear_down_all_test.dart

@@ -95,4 +95,19 @@ void main() {
     expect(test.stdout, neverEmits(contains('(tearDownAll)')));
     await test.shouldExit(0);
   });
+
+  test('does not fail with asserts enabled - issue #2498', () async {
+    await d.file('test.dart', r'''
+        import 'package:test/test.dart';
+
+        void main() {
+          tearDownAll(() {});
+
+          test("test", () {});
+        }
+        ''').create();
+
+    var test = await runTest(['test.dart'], vmArgs: ['--enable-asserts']);
+    await test.shouldExit(0);
+  });
 }

pkgs/test_api/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.7.6
+
+* Fix an assertion failure when using `setUpAll` or `tearDownAll` and running
+  with asserts enabled.
+
 ## 0.7.5
 
 * `test()` and `group()` functions now take an optional `TestLocation` that will

pkgs/test_api/lib/src/backend/group.dart

@@ -96,8 +96,21 @@ class Group implements GroupEntry {
         metadata: metadata,
         trace: trace,
         location: location,
-        setUpAll: setUpAll,
-        tearDownAll: tearDownAll);
+        // Always clone these because they are being re-parented.
+        setUpAll: setUpAll?.clone(),
+        tearDownAll: tearDownAll?.clone());
+  }
+
+  @override
+  Group? clone() {
+    var entries = _map((entry) => entry.clone());
+    return Group(name, entries,
+        metadata: metadata,
+        trace: trace,
+        location: location,
+        // Always clone these because they are being re-parented.
+        setUpAll: setUpAll?.clone(),
+        tearDownAll: tearDownAll?.clone());
   }
 
   /// Returns the entries of this group mapped using [callback].

pkgs/test_api/lib/src/backend/group_entry.dart

@@ -51,4 +51,8 @@ abstract class GroupEntry {
   /// Returns `null` if this is a test that doesn't match [callback] or a group
   /// where no child tests match [callback].
   GroupEntry? filter(bool Function(Test) callback);
+
+  /// Returns a clone of this object without the internal `parent` reference
+  /// set so that it may be attached to a new tree.
+  GroupEntry? clone();
 }

pkgs/test_api/lib/src/backend/invoker.dart

@@ -79,12 +79,17 @@ class LocalTest extends Test {
     if (callback(this)) {
       // filter() always returns new copies because they need to be attached
       // to their new parents.
-      return LocalTest._(
-          name, metadata, _body, trace, location, _guarded, isScaffoldAll);
+      return clone();
     }
 
     return null;
   }
+
+  @override
+  Test clone() {
+    return LocalTest._(
+        name, metadata, _body, trace, location, _guarded, isScaffoldAll);
+  }
 }
 
 /// The class responsible for managing the lifecycle of a single local test.

pkgs/test_api/lib/src/backend/test.dart

@@ -42,4 +42,7 @@ abstract class Test implements GroupEntry {
 
   @override
   Test? filter(bool Function(Test) callback);
+
+  @override
+  Test? clone();
 }

pkgs/test_api/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test_api
-version: 0.7.5
+version: 0.7.6
 description: >-
   The user facing API for structuring Dart tests and checking expectations.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test_api