From 1808ac338cb498969ddeacb399bb49d62bed8a20 Mon Sep 17 00:00:00 2001 From: Todd Volkert Date: Fri, 18 May 2018 13:58:08 -0700 Subject: [PATCH] Add support for custom test error reporters (#17727) This allows test environments other than `flutter test` to have a hook into the test exception reporting. Some test environments, for example, don't just dump error details to the console, but rather require them to be reported to a separate server. --- ...llow_error_reporter_modification_test.dart | 13 ++++++ dev/bots/test.dart | 7 ++++ packages/flutter_test/lib/flutter_test.dart | 3 +- packages/flutter_test/lib/src/binding.dart | 36 ++++++++++------ .../lib/src/test_exception_reporter.dart | 42 +++++++++++++++++++ .../flutter_test_config.dart | 26 ++++++++++++ .../test_exception_reporter_test.dart | 7 ++++ 7 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 dev/automated_tests/test_smoke_test/disallow_error_reporter_modification_test.dart create mode 100644 packages/flutter_test/lib/src/test_exception_reporter.dart create mode 100644 packages/flutter_test/test/custom_exception_reporter/flutter_test_config.dart create mode 100644 packages/flutter_test/test/custom_exception_reporter/test_exception_reporter_test.dart diff --git a/dev/automated_tests/test_smoke_test/disallow_error_reporter_modification_test.dart b/dev/automated_tests/test_smoke_test/disallow_error_reporter_modification_test.dart new file mode 100644 index 0000000000..6ea424250e --- /dev/null +++ b/dev/automated_tests/test_smoke_test/disallow_error_reporter_modification_test.dart @@ -0,0 +1,13 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + testWidgets('tests must restore the value of reportTestException', (WidgetTester tester) async { + // This test is expected to fail. + reportTestException = (FlutterErrorDetails details, String testDescription) {}; + }); +} diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 2e822f3ce7..e5f8c451d2 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -179,6 +179,13 @@ Future _runTests({List options: const []}) async { printOutput: false, timeout: _kShortTimeout, ); + await _runFlutterTest(automatedTests, + script: path.join('test_smoke_test', 'disallow_error_reporter_modification_test.dart'), + options: options, + expectFailure: true, + printOutput: false, + timeout: _kShortTimeout, + ); await _runCommand(flutter, ['drive', '--use-existing-app'] ..addAll(options) diff --git a/packages/flutter_test/lib/flutter_test.dart b/packages/flutter_test/lib/flutter_test.dart index 43c44d6115..c46e733bb9 100644 --- a/packages/flutter_test/lib/flutter_test.dart +++ b/packages/flutter_test/lib/flutter_test.dart @@ -27,7 +27,7 @@ /// with the following signature: /// /// ```dart -/// void main(FutureOr testMain()); +/// Future main(FutureOr testMain()); /// ``` /// /// The test framework will execute that method and pass it the `main()` method @@ -56,6 +56,7 @@ export 'src/nonconst.dart'; export 'src/platform.dart'; export 'src/stack_manipulation.dart'; export 'src/test_async_utils.dart'; +export 'src/test_exception_reporter.dart'; export 'src/test_pointer.dart'; export 'src/test_text_input.dart'; export 'src/test_vsync.dart'; diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index ffdce670e4..9630449ef7 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -22,6 +22,7 @@ import 'package:vector_math/vector_math_64.dart'; import 'goldens.dart'; import 'stack_manipulation.dart'; import 'test_async_utils.dart'; +import 'test_exception_reporter.dart'; import 'test_text_input.dart'; /// Phases that can be reached by [WidgetTester.pumpWidget] and @@ -358,16 +359,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase assert(_currentTestCompleter != null); if (_pendingExceptionDetails != null) { debugPrint = debugPrintOverride; // just in case the test overrides it -- otherwise we won't see the error! - FlutterError.dumpErrorToConsole(_pendingExceptionDetails, forceReport: true); - // test_package.registerException actually just calls the current zone's error handler (that - // is to say, _parentZone's handleUncaughtError function). FakeAsync doesn't add one of those, - // but the test package does, that's how the test package tracks errors. So really we could - // get the same effect here by calling that error handler directly or indeed just throwing. - // However, we call registerException because that's the semantically correct thing... - String additional = ''; - if (_currentTestDescription != '') - additional = '\nThe test description was: $_currentTestDescription'; - test_package.registerException('Test failed. See exception logs above.$additional', _emptyStackTrace); + reportTestException(_pendingExceptionDetails, _currentTestDescription); _pendingExceptionDetails = null; } _currentTestDescription = null; @@ -504,6 +496,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase await pump(); final bool autoUpdateGoldensBeforeTest = autoUpdateGoldenFiles; + final TestExceptionReporter reportTestExceptionBeforeTest = reportTestException; // run the test await testBody(); @@ -517,6 +510,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase await pump(); invariantTester(); _verifyAutoUpdateGoldensUnset(autoUpdateGoldensBeforeTest); + _verifyReportTestExceptionUnset(reportTestExceptionBeforeTest); _verifyInvariants(); } @@ -566,6 +560,26 @@ abstract class TestWidgetsFlutterBinding extends BindingBase }()); } + void _verifyReportTestExceptionUnset(TestExceptionReporter valueBeforeTest) { + assert(() { + if (reportTestException != valueBeforeTest) { + // We can't report this error to their modified reporter because we + // can't be guaranteed that their reporter will cause the test to fail. + // So we reset the error reporter to its initial value and then report + // this error. + reportTestException = valueBeforeTest; + FlutterError.reportError(new FlutterErrorDetails( + exception: new FlutterError( + 'The value of reportTestException was changed by the test.', + ), + stack: StackTrace.current, + library: 'Flutter test framework', + )); + } + return true; + }()); + } + /// Called by the [testWidgets] function after a test is executed. void postTest() { assert(inTest); @@ -1301,8 +1315,6 @@ class _LiveTestRenderView extends RenderView { } } -final StackTrace _emptyStackTrace = new stack_trace.Chain(const []); - StackTrace _unmangle(StackTrace stack) { if (stack is stack_trace.Trace) return stack.vmTrace; diff --git a/packages/flutter_test/lib/src/test_exception_reporter.dart b/packages/flutter_test/lib/src/test_exception_reporter.dart new file mode 100644 index 0000000000..df8386b846 --- /dev/null +++ b/packages/flutter_test/lib/src/test_exception_reporter.dart @@ -0,0 +1,42 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:flutter/foundation.dart'; +import 'package:stack_trace/stack_trace.dart' as stack_trace; +import 'package:test/test.dart' as test_package; + +/// Signature for the [reportTestException] callback. +typedef void TestExceptionReporter(FlutterErrorDetails details, String testDescription); + +/// A function that is called by the test framework when an unexpected error +/// occurred during a test. +/// +/// This function is responsible for reporting the error to the user such that +/// the user can easily diagnose what failed when inspecting the test results. +/// It is also responsible for reporting the error to the test framework itself +/// in order to cause the test to fail. +/// +/// This function is pluggable to handle the cases where tests are run in +/// contexts _other_ than via `flutter test`. +TestExceptionReporter get reportTestException => _reportTestException; +TestExceptionReporter _reportTestException = _defaultTestExceptionReporter; +set reportTestException(TestExceptionReporter handler) { + assert(handler != null); + _reportTestException = handler; +} + +void _defaultTestExceptionReporter(FlutterErrorDetails errorDetails, String testDescription) { + FlutterError.dumpErrorToConsole(errorDetails, forceReport: true); + // test_package.registerException actually just calls the current zone's error handler (that + // is to say, _parentZone's handleUncaughtError function). FakeAsync doesn't add one of those, + // but the test package does, that's how the test package tracks errors. So really we could + // get the same effect here by calling that error handler directly or indeed just throwing. + // However, we call registerException because that's the semantically correct thing... + String additional = ''; + if (testDescription.isNotEmpty) + additional = '\nThe test description was: $testDescription'; + test_package.registerException('Test failed. See exception logs above.$additional', _emptyStackTrace); +} + +final StackTrace _emptyStackTrace = new stack_trace.Chain(const []); diff --git a/packages/flutter_test/test/custom_exception_reporter/flutter_test_config.dart b/packages/flutter_test/test/custom_exception_reporter/flutter_test_config.dart new file mode 100644 index 0000000000..2f9be2352f --- /dev/null +++ b/packages/flutter_test/test/custom_exception_reporter/flutter_test_config.dart @@ -0,0 +1,26 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:async'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; + +Future main(FutureOr testMain()) async { + reportTestException = (FlutterErrorDetails details, String testDescription) { + expect(details.exception, const isInstanceOf()); + expect(details.exception.message, 'foo'); + expect(testDescription, 'custom exception reporter'); + }; + + // The error that the test throws in [runTest] will be forwarded to our + // reporter and should not cause the test to fail. + await testMain(); +} + +void runTest() { + testWidgets('custom exception reporter', (WidgetTester tester) { + throw new StateError('foo'); + }); +} diff --git a/packages/flutter_test/test/custom_exception_reporter/test_exception_reporter_test.dart b/packages/flutter_test/test/custom_exception_reporter/test_exception_reporter_test.dart new file mode 100644 index 0000000000..d93ed64fa6 --- /dev/null +++ b/packages/flutter_test/test/custom_exception_reporter/test_exception_reporter_test.dart @@ -0,0 +1,7 @@ +// Copyright 2018 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'flutter_test_config.dart' as real_test; + +void main() => real_test.runTest();