diff --git a/pkgs/test_reflective_loader/CHANGELOG.md b/pkgs/test_reflective_loader/CHANGELOG.md index 27a14c6f8..682011349 100644 --- a/pkgs/test_reflective_loader/CHANGELOG.md +++ b/pkgs/test_reflective_loader/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.5.0 + +- Unawaited asynchronous exceptions in tests marked with `@FailingTest()` are + now handled correctly and will allow the test to pass. +- Tests marked with `@FailingTest()` but passing no longer run until the + timeout. + ## 0.4.0 - Add support for one-time set up and teardown in test classes via static diff --git a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart index 8ea837dee..8c8b6abdc 100644 --- a/pkgs/test_reflective_loader/lib/test_reflective_loader.dart +++ b/pkgs/test_reflective_loader/lib/test_reflective_loader.dart @@ -246,29 +246,32 @@ Future _invokeSymbolIfExists( /// - The test fails by throwing an exception /// - The test returns a future which completes with an error. /// - An exception is thrown to the zone handler from a timer task. -Future? _runFailingTest(ClassMirror classMirror, Symbol symbol) { - var passed = false; - return runZonedGuarded(() { +Future _runFailingTest(ClassMirror classMirror, Symbol symbol) async { + _FailedTestResult? result; + + await runZonedGuarded(() { // ignore: void_checks return Future.sync(() => _runTest(classMirror, symbol)).then((_) { - passed = true; - test_package.fail('Test passed - expected to fail.'); + // We can't throw async exceptions inside here because `runZoneGuarded` + // will never complete (see docs on `runZonedGuarded`), so we need to + // capture this state and throw later if there wasn't otherwise an + // exception. + + // If we didn't already have a failure (eg. an unawaited exception) then + // this successful completion is an unexpected pass state. + result ??= _FailedTestResult.pass; }).catchError((Object e) { - // if passed, and we call fail(), rethrow this exception - if (passed) { - // ignore: only_throw_errors - throw e; - } - // otherwise, an exception is not a failure for _runFailingTest + // an awaited exception is always expected failure. + result = _FailedTestResult.expectedFail; }); }, (e, st) { - // if passed, and we call fail(), rethrow this exception - if (passed) { - // ignore: only_throw_errors - throw e; - } - // otherwise, an exception is not a failure for _runFailingTest + result = _FailedTestResult.expectedFail; }); + + // We can safely throw exceptions back outside of the error zone. + if (result == _FailedTestResult.pass) { + throw test_package.TestFailure('Test passed - expected to fail.'); + } } Future _runTest(ClassMirror classMirror, Symbol symbol) async { @@ -389,6 +392,15 @@ class _Test { timeout = null; } +/// The result of a test that was expected to fail. +enum _FailedTestResult { + /// The test (unexpectedly) passed. + pass, + + /// The test failed as expected. + expectedFail, +} + extension on DeclarationMirror { test_package.TestLocation? get testLocation { if (location case var location?) { diff --git a/pkgs/test_reflective_loader/pubspec.yaml b/pkgs/test_reflective_loader/pubspec.yaml index efdec89d1..4a4a0a36a 100644 --- a/pkgs/test_reflective_loader/pubspec.yaml +++ b/pkgs/test_reflective_loader/pubspec.yaml @@ -1,5 +1,5 @@ name: test_reflective_loader -version: 0.4.0 +version: 0.5.0 description: Support for discovering tests and test suites using reflection. repository: https://github.com/dart-lang/tools/tree/main/pkgs/test_reflective_loader issue_tracker: https://github.com/dart-lang/tools/labels/package%3Atest_reflective_loader diff --git a/pkgs/test_reflective_loader/test/test_reflective_loader_test.dart b/pkgs/test_reflective_loader/test/test_reflective_loader_test.dart index ba0319305..5b4d6e067 100644 --- a/pkgs/test_reflective_loader/test/test_reflective_loader_test.dart +++ b/pkgs/test_reflective_loader/test/test_reflective_loader_test.dart @@ -71,10 +71,24 @@ class TestReflectiveLoaderTest { } @failingTest - Future test_fails_throws_async() { + Future test_fails_throws_async() { return Future.error('foo'); } + @failingTest + Future test_fails_throws_async_unawaitedFuture() async { + var completer = Completer(); + // This exception occurs during the test run but isn't directly awaited so + // this method doesn't not itself complete with an error. + // + // This is a legit test failure and will be marked as such without + // the `@failingTest` annotation. + unawaited(Future.value() + .then((_) => throw UnimplementedError('foo')) + .whenComplete(completer.complete)); + await completer.future; + } + @failingTest void test_fails_throws_sync() { throw StateError('foo');