Skip to content

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

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

Merged
merged 3 commits into from
May 19, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion pkgs/test/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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

Expand Down
6 changes: 3 additions & 3 deletions pkgs/test/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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'
Expand Down
15 changes: 15 additions & 0 deletions pkgs/test/test/runner/set_up_all_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why no existing tests triggered this, but did trigger when we didn't have the fix in onPlatform, but I added specific tests for these referencing the issue.

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);
});
}
15 changes: 15 additions & 0 deletions pkgs/test/test/runner/tear_down_all_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
5 changes: 5 additions & 0 deletions pkgs/test_api/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
17 changes: 15 additions & 2 deletions pkgs/test_api/lib/src/backend/group.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
4 changes: 4 additions & 0 deletions pkgs/test_api/lib/src/backend/group_entry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
9 changes: 7 additions & 2 deletions pkgs/test_api/lib/src/backend/invoker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions pkgs/test_api/lib/src/backend/test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,7 @@ abstract class Test implements GroupEntry {

@override
Test? filter(bool Function(Test) callback);

@override
Test? clone();
}
2 changes: 1 addition & 1 deletion pkgs/test_api/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 3 additions & 1 deletion pkgs/test_core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 0.6.10-wip
## 0.6.10

* Set a debug name for test isolates.
* Fix an assertion failure when using `setUpAll` or `tearDownAll` and running
with asserts enabled.

## 0.6.9

Expand Down
7 changes: 6 additions & 1 deletion pkgs/test_core/lib/src/runner/runner_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,14 @@ class RunnerTest extends Test {
if (callback(this)) {
// filter() always returns new copies because they need to be attached
// to their new parents.
return RunnerTest(name, metadata, trace, location, _channel);
return clone();
}

return null;
}

@override
Test clone() {
return RunnerTest(name, metadata, trace, location, _channel);
}
}
4 changes: 2 additions & 2 deletions pkgs/test_core/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: test_core
version: 0.6.10-wip
version: 0.6.10
description: A basic library for writing tests and running them on the VM.
repository: https://github.com/dart-lang/test/tree/master/pkgs/test_core
issue_tracker: https://github.com/dart-lang/test/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Atest
Expand Down Expand Up @@ -28,7 +28,7 @@ dependencies:
stack_trace: ^1.10.0
stream_channel: ^2.1.0
# Use an exact version until the test_api package is stable.
test_api: 0.7.5
test_api: 0.7.6
vm_service: ">=6.0.0 <16.0.0"
yaml: ^3.0.0

Expand Down
Loading