Skip to content

Commit 49f45d2

Browse files
Reverts "Fail tests on exceptions raised after test completed (flutter#144706)" (flutter#144970)
Reverts: flutter#144706 Initiated by: gspencergoog Reason for reverting: This has broken the tree because some tests are still failing post completion. This particular one looks like it might have to do with a gold image not existing. Original PR Author: goderbauer Reviewed By: {Piinks} This change reverts the following previous change: A test was failing silently because of this (see flutter#144353 and fixed in flutter#144709). The failure went undetected for months. Ideally, this should have been a regular non-silent failure. This change makes that so. `package:test` can properly handle reported exceptions outside of test cases. With this change, the test fails as follows: ``` 00:03 +82: Smoke test material/color_scheme/dynamic_content_color.0.dart ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════ The following assertion was thrown running a test (but after the test had completed): setState() called after dispose(): _DynamicColorExampleState#1cd37(lifecycle state: defunct, not mounted) This error happens if you call setState() on a State object for a widget that no longer appears in the widget tree (e.g., whose parent widget no longer includes the widget in its build). This error can occur when code calls setState() from a timer or an animation callback. The preferred solution is to cancel the timer or stop listening to the animation in the dispose() callback. Another solution is to check the "mounted" property of this object before calling setState() to ensure the object is still in the tree. This error might indicate a memory leak if setState() is being called because another object is retaining a reference to this State object after it has been removed from the tree. To avoid memory leaks, consider breaking the reference to this object during dispose(). When the exception was thrown, this was the stack: #0 State.setState.<anonymous closure> (package:flutter/src/widgets/framework.dart:1167:9) #1 State.setState (package:flutter/src/widgets/framework.dart:1202:6) #2 _DynamicColorExampleState._updateImage (package:flutter_api_samples/material/color_scheme/dynamic_content_color.0.dart:191:5) <asynchronous suspension> ════════════════════════════════════════════════════════════════════════════════════════════════════ 00:03 +81 -1: Smoke test material/context_menu/context_menu_controller.0.dart 00:03 +81 -1: Smoke test material/color_scheme/dynamic_content_color.0.dart [E] Test failed. See exception logs above. The test description was: Smoke test material/color_scheme/dynamic_content_color.0.dart This test failed after it had already completed. Make sure to use a matching library which informs the test runner of pending async work. ```
1 parent e66811a commit 49f45d2

File tree

4 files changed

+7
-54
lines changed

4 files changed

+7
-54
lines changed

Diff for: dev/automated_tests/test_smoke_test/fail_test_on_exception_after_test.dart

-29
This file was deleted.

Diff for: dev/bots/test.dart

+1-20
Original file line numberDiff line numberDiff line change
@@ -346,26 +346,7 @@ Future<void> _runTestHarnessTests() async {
346346
: 'Failed to find the stack trace for the pending Timer.\n\n'
347347
'stdout:\n${result.flattenedStdout}\n\n'
348348
'stderr:\n${result.flattenedStderr}';
349-
},
350-
),
351-
() => _runFlutterTest(
352-
automatedTests,
353-
script: path.join('test_smoke_test', 'fail_test_on_exception_after_test.dart'),
354-
expectFailure: true,
355-
printOutput: false,
356-
outputChecker: (CommandResult result) {
357-
const String expectedError = '══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════\n'
358-
'The following StateError was thrown running a test (but after the test had completed):\n'
359-
'Bad state: Exception thrown after test completed.';
360-
if (result.flattenedStdout!.contains(expectedError)) {
361-
return null;
362-
}
363-
return 'Failed to find expected output on stdout.\n\n'
364-
'Expected output:\n$expectedError\n\n'
365-
'Actual stdout:\n${result.flattenedStdout}\n\n'
366-
'Actual stderr:\n${result.flattenedStderr}';
367-
},
368-
),
349+
}),
369350
() => _runFlutterTest(
370351
automatedTests,
371352
script: path.join('test_smoke_test', 'crash1_test.dart'),

Diff for: dev/bots/test/test_test.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,13 @@ void main() {
128128
<String, String>{'SHARD': kTestHarnessShardName, 'SUBSHARD': '1_3'},
129129
);
130130
expectExitCode(result, 0);
131-
expect(result.stdout, contains('Selecting subshard 1 of 3 (tests 1-3 of 9)'));
131+
expect(result.stdout, contains('Selecting subshard 1 of 3 (tests 1-3 of 8)'));
132132

133133
result = await runScript(
134134
<String, String>{'SHARD': kTestHarnessShardName, 'SUBSHARD': '3_3'},
135135
);
136136
expectExitCode(result, 0);
137-
expect(result.stdout, contains('Selecting subshard 3 of 3 (tests 7-9 of 9)'));
137+
expect(result.stdout, contains('Selecting subshard 3 of 3 (tests 7-8 of 8)'));
138138
});
139139

140140
test('exits with code 1 when SUBSHARD index greater than total', () async {

Diff for: packages/flutter_test/lib/src/binding.dart

+4-3
Original file line numberDiff line numberDiff line change
@@ -910,14 +910,15 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
910910
// Ideally, once the test has failed we would stop getting errors from the test.
911911
// However, if someone tries hard enough they could get in a state where this happens.
912912
// If we silently dropped these errors on the ground, nobody would ever know. So instead
913-
// we raise them and fail the test after it has already completed.
913+
// we report them to the console. They don't cause test failures, but hopefully someone
914+
// will see them in the logs at some point.
914915
debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error!
915-
reportTestException(FlutterErrorDetails(
916+
FlutterError.dumpErrorToConsole(FlutterErrorDetails(
916917
exception: exception,
917918
stack: stack,
918919
context: ErrorDescription('running a test (but after the test had completed)'),
919920
library: 'Flutter test framework',
920-
), description);
921+
), forceReport: true);
921922
return;
922923
}
923924
// This is where test failures, e.g. those in expect(), will end up.

0 commit comments

Comments
 (0)