From 9772358aa4df900f51b6f56e21c0b77fe052c86a Mon Sep 17 00:00:00 2001 From: "nweiz@google.com" Date: Mon, 11 Feb 2013 22:56:00 +0000 Subject: [PATCH] Make the scheduled_test schedule keep track of multiple errors. Review URL: https://codereview.chromium.org//12208096 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@18349 260f80e4-7a28-3924-810f-c04153c831b5 --- pkg/scheduled_test/lib/scheduled_test.dart | 6 +- pkg/scheduled_test/lib/src/schedule.dart | 67 +++++++-- .../test/scheduled_test_test.dart | 128 ++++++++++++++---- 3 files changed, 158 insertions(+), 43 deletions(-) diff --git a/pkg/scheduled_test/lib/scheduled_test.dart b/pkg/scheduled_test/lib/scheduled_test.dart index 23a060ff052..26a67323278 100644 --- a/pkg/scheduled_test/lib/scheduled_test.dart +++ b/pkg/scheduled_test/lib/scheduled_test.dart @@ -2,8 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -// TODO(nweiz): Keep track of and display multiple errors so there's more -// visibility into cascading errors. // TODO(nweiz): Add timeouts to scheduled tests. // TODO(nweiz): Add support for calling [schedule] while the schedule is already // running. @@ -194,7 +192,9 @@ void _test(String description, void body(), Function testFn) { asyncDone(); }).catchError((e) { if (e is ScheduleError) { - unittest.registerException(new ExpectException(e.toString())); + assert(e.schedule.errors.contains(e)); + assert(e.schedule == currentSchedule); + unittest.registerException(e.schedule.errorString()); } else if (e is AsyncError) { unittest.registerException(e.error, e.stackTrace); } else { diff --git a/pkg/scheduled_test/lib/src/schedule.dart b/pkg/scheduled_test/lib/src/schedule.dart index dd9e6d3a776..d1f5ea34033 100644 --- a/pkg/scheduled_test/lib/src/schedule.dart +++ b/pkg/scheduled_test/lib/src/schedule.dart @@ -22,7 +22,7 @@ class Schedule { TaskQueue _tasks; /// The queue of tasks to run if an error is caught while running [tasks]. The - /// error will be available via [error]. These tasks won't be run if no error + /// error will be available in [errors]. These tasks won't be run if no error /// occurs. Note that expectation failures count as errors. /// /// This queue runs before [onComplete], and errors in [onComplete] will not @@ -35,12 +35,12 @@ class Schedule { /// The queue of tasks to run after [tasks] and possibly [onException] have /// run. This queue will run whether or not an error occurred. If one did, it - /// will be available via [error]. Note that expectation failures count as + /// will be available in [errors]. Note that expectation failures count as /// errors. /// /// This queue runs after [onException]. If an error occurs while running - /// [onException], that error will be available via [error] in place of the - /// original error. + /// [onException], that error will be available in [errors] after the original + /// error. /// /// If an error occurs in a task in this queue, all further tasks will be /// skipped. @@ -59,11 +59,20 @@ class Schedule { bool get done => _done; bool _done = false; - /// The error thrown by the task queue. This will only be set while running - /// [onException] and [onComplete], since an error in [tasks] will cause it to - /// terminate immediately. - ScheduleError get error => _error; - ScheduleError _error; + // TODO(nweiz): make this a read-only view once issue 8321 is fixed. + + /// Errors thrown by the task queues. + /// + /// When running tasks in [tasks], this will always be empty. If an error + /// occurs in [tasks], it will be added to this list and then [onException] + /// will be run. If an error occurs there as well, it will be added to this + /// list and [onComplete] will be run. Errors thrown during [onComplete] will + /// also be added to this list, although no scheduled tasks will be run + /// afterwards. + /// + /// Any out-of-band callbacks that throw errors will also have those errors + /// added to this list. + final errors = []; /// The task queue that's currently being run, or `null` if there is no such /// queue. One of [tasks], [onException], or [onComplete]. This will be `null` @@ -99,11 +108,27 @@ class Schedule { return tasks._run(); }).catchError((e) { - _error = e; - return onException._run().then((_) { + errors.add(e); + return onException._run().catchError((innerError) { + // If an error occurs in a task in the onException queue, make sure it's + // registered in the error list and re-throw it. We could also re-throw + // `e`; ultimately, all the errors will be shown to the user if any + // ScheduleError is thrown. + errors.add(innerError); + throw innerError; + }).then((_) { + // If there are no errors in the onException queue, re-throw the + // original error that caused it to run. throw e; }); - }).whenComplete(() => onComplete._run()).whenComplete(() { + }).whenComplete(() { + return onComplete._run().catchError((e) { + // If an error occurs in a task in the onComplete queue, make sure it's + // registered in the error list and re-throw it. + errors.add(e); + throw e; + }); + }).whenComplete(() { _done = true; }); } @@ -116,10 +141,11 @@ class Schedule { var scheduleError = new ScheduleError.from(this, error, stackTrace: stackTrace, task: currentTask); if (_done) { + errors.add(scheduleError); throw new StateError( "An out-of-band error was signaled outside of wrapAsync after the " - "schedule finished running:" - "${prefixLines(scheduleError.toString())}"); + "schedule finished running.\n" + "${errorString()}"); } else if (currentQueue == null) { // If we're not done but there's no current queue, that means we haven't // started yet and thus we're in setUp or the synchronous body of the @@ -160,6 +186,15 @@ class Schedule { }; } + /// Returns a string representation of all errors registered on this schedule. + String errorString() { + if (errors.isEmpty) return "The schedule had no errors."; + if (errors.length == 1) return errors.first.toString(); + var errorStrings = errors.map((e) => e.toString()).join("\n================" + "================================================================\n"); + return "The schedule had ${errors.length} errors:\n$errorStrings"; + } + /// Returns a [Future] that will complete once there are no pending /// out-of-band callbacks. Future _awaitNoPendingCallbacks() { @@ -212,6 +247,7 @@ class TaskQueue { _schedule._currentTask = task; if (_error != null) throw _error; return task.fn().catchError((e) { + if (_error != null) _schedule.errors.add(_error); throw new ScheduleError.from(_schedule, e, task: task); }); }).whenComplete(() { @@ -225,6 +261,9 @@ class TaskQueue { /// Signals that an out-of-band error has been detected and the queue should /// stop running as soon as possible. void _signalError(ScheduleError error) { + // If multiple errors are detected while a task is running, make sure the + // earlier ones are recorded in the schedule. + if (_error != null) _schedule.errors.add(_error); _error = error; } diff --git a/pkg/scheduled_test/test/scheduled_test_test.dart b/pkg/scheduled_test/test/scheduled_test_test.dart index 690319c6133..1b8b29bd525 100644 --- a/pkg/scheduled_test/test/scheduled_test_test.dart +++ b/pkg/scheduled_test/test/scheduled_test_test.dart @@ -293,20 +293,20 @@ void main() { }); }, passing: ['test 2']); - expectTestsPass('currentSchedule.error contains the error in the onComplete ' + expectTestsPass('currentSchedule.errors contains the error in the onComplete ' 'queue', () { - var error; + var errors; test('test 1', () { currentSchedule.onComplete.schedule(() { - error = currentSchedule.error; + errors = currentSchedule.errors; }); throw 'error'; }); test('test 2', () { - expect(error, new isInstanceOf()); - expect(error.error, equals('error')); + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error'])); }); }, passing: ['test 2']); @@ -395,71 +395,147 @@ void main() { }); }, passing: ['test 2']); - expectTestsPass('currentSchedule.error contains the error in the onException ' - 'queue', () { - var error; + expectTestsPass('currentSchedule.errors contains the error in the ' + 'onException queue', () { + var errors; test('test 1', () { currentSchedule.onException.schedule(() { - error = currentSchedule.error; + errors = currentSchedule.errors; }); throw 'error'; }); test('test 2', () { - expect(error, new isInstanceOf()); - expect(error.error, equals('error')); + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error'])); }); }, passing: ['test 2']); - expectTestsPass('currentSchedule.error contains an error passed into ' + expectTestsPass('currentSchedule.errors contains an error passed into ' 'signalError synchronously', () { - var error; + var errors; test('test 1', () { currentSchedule.onException.schedule(() { - error = currentSchedule.error; + errors = currentSchedule.errors; }); currentSchedule.signalError('error'); }); test('test 2', () { - expect(error, new isInstanceOf()); - expect(error.error, equals('error')); + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error'])); }); }, passing: ['test 2']); - expectTestsPass('currentSchedule.error contains an error passed into ' + expectTestsPass('currentSchedule.errors contains an error passed into ' 'signalError asynchronously', () { - var error; + var errors; test('test 1', () { currentSchedule.onException.schedule(() { - error = currentSchedule.error; + errors = currentSchedule.errors; }); schedule(() => currentSchedule.signalError('error')); }); test('test 2', () { - expect(error, new isInstanceOf()); - expect(error.error, equals('error')); + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error'])); }); }, passing: ['test 2']); - expectTestsPass('currentSchedule.error contains an error passed into ' + expectTestsPass('currentSchedule.errors contains an error passed into ' 'signalError out-of-band', () { - var error; + var errors; test('test 1', () { currentSchedule.onException.schedule(() { - error = currentSchedule.error; + errors = currentSchedule.errors; }); sleep(50).then(wrapAsync((_) => currentSchedule.signalError('error'))); }); test('test 2', () { - expect(error, new isInstanceOf()); - expect(error.error, equals('error')); + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error'])); + }); + }, passing: ['test 2']); + + expectTestsPass('currentSchedule.errors contains errors from both the task ' + 'queue and the onException queue in onComplete', () { + var errors; + test('test 1', () { + currentSchedule.onComplete.schedule(() { + errors = currentSchedule.errors; + }); + + currentSchedule.onException.schedule(() { + throw 'error2'; + }); + + throw 'error1'; + }); + + test('test 2', () { + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['error1', 'error2'])); + }); + }, passing: ['test 2']); + + expectTestsPass('currentSchedule.errors contains multiple out-of-band errors ' + 'from both the main task queue and onException in onComplete', () { + var errors; + test('test 1', () { + currentSchedule.onComplete.schedule(() { + errors = currentSchedule.errors; + }); + + currentSchedule.onException.schedule(() { + sleep(25).then(wrapAsync((_) { + throw 'error3'; + })); + sleep(50).then(wrapAsync((_) { + throw 'error4'; + })); + }); + + sleep(25).then(wrapAsync((_) { + throw 'error1'; + })); + sleep(50).then(wrapAsync((_) { + throw 'error2'; + })); + }); + + test('test 2', () { + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), + orderedEquals(['error1', 'error2', 'error3', 'error4'])); + }); + }, passing: ['test 2']); + + expectTestsPass('currentSchedule.errors contains both an out-of-band error ' + 'and an error raised afterwards in a task', () { + var errors; + test('test 1', () { + currentSchedule.onComplete.schedule(() { + errors = currentSchedule.errors; + }); + + sleep(25).then(wrapAsync((_) { + throw 'out-of-band'; + })); + + schedule(() => sleep(50).then((_) { + throw 'in-band'; + })); + }); + + test('test 2', () { + expect(errors, everyElement(new isInstanceOf())); + expect(errors.map((e) => e.error), equals(['out-of-band', 'in-band'])); }); }, passing: ['test 2']);