From 8f3150c703369e9dd4fbff93f6d2abe9b36837b6 Mon Sep 17 00:00:00 2001
From: Natalie Weizenbaum <nweiz@google.com>
Date: Wed, 15 Apr 2015 14:34:39 -0700
Subject: [PATCH] Support configuring timeouts per-test and -group.
Support for using Timeout as an annotation will come next.
See #9
R=kevmoo@google.com
Review URL: https://codereview.chromium.org//1090513002
---
lib/src/backend/declarer.dart | 23 ++++++--
lib/src/backend/invoker.dart | 6 +--
lib/src/backend/metadata.dart | 36 ++++++++++---
lib/src/frontend/timeout.dart | 47 +++++++++++++++++
lib/src/utils.dart | 19 +++++++
lib/test.dart | 19 +++++--
test/backend/declarer_test.dart | 33 ++++++++++++
test/backend/invoker_test.dart | 94 +++++++++++++++++++++++++--------
8 files changed, 234 insertions(+), 43 deletions(-)
create mode 100644 lib/src/frontend/timeout.dart
diff --git a/lib/src/backend/declarer.dart b/lib/src/backend/declarer.dart
index 9983754c..01e81407 100644
--- a/lib/src/backend/declarer.dart
+++ b/lib/src/backend/declarer.dart
@@ -6,6 +6,7 @@ library test.backend.declarer;
import 'dart:collection';
+import '../frontend/timeout.dart';
import 'group.dart';
import 'invoker.dart';
import 'metadata.dart';
@@ -32,13 +33,19 @@ class Declarer {
///
/// If [testOn] is passed, it's parsed as a [PlatformSelector], and the test
/// will only be run on matching platforms.
- void test(String description, body(), {String testOn}) {
+ ///
+ /// If [timeout] is passed, it's used to modify or replace the default timeout
+ /// of 30 seconds. Timeout modifications take precedence in suite-group-test
+ /// order, so [timeout] will also modify any timeouts set on the group or
+ /// suite.
+ void test(String description, body(), {String testOn, Timeout timeout}) {
// TODO(nweiz): Once tests have begun running, throw an error if [test] is
// called.
var prefix = _group.description;
if (prefix != null) description = "$prefix $description";
- var metadata = _group.metadata.merge(new Metadata.parse(testOn: testOn));
+ var metadata = _group.metadata.merge(
+ new Metadata.parse(testOn: testOn, timeout: timeout));
var group = _group;
_tests.add(new LocalTest(description, metadata, () {
// TODO(nweiz): It might be useful to throw an error here if a test starts
@@ -56,11 +63,17 @@ class Declarer {
///
/// If [testOn] is passed, it's parsed as a [PlatformSelector], and any tests
/// in the group will only be run on matching platforms.
- void group(String description, void body(), {String testOn}) {
+ ///
+ /// If [timeout] is passed, it's used to modify or replace the default timeout
+ /// of 30 seconds. Timeout modifications take precedence in suite-group-test
+ /// order, so [timeout] will also modify any timeouts set on the group or
+ /// suite.
+ void group(String description, void body(), {String testOn,
+ Timeout timeout}) {
var oldGroup = _group;
- _group = new Group(
- oldGroup, description, new Metadata.parse(testOn: testOn));
+ var metadata = new Metadata.parse(testOn: testOn, timeout: timeout);
+ _group = new Group(oldGroup, description, metadata);
try {
body();
} finally {
diff --git a/lib/src/backend/invoker.dart b/lib/src/backend/invoker.dart
index 057e7049..147741ff 100644
--- a/lib/src/backend/invoker.dart
+++ b/lib/src/backend/invoker.dart
@@ -158,12 +158,12 @@ class Invoker {
// TODO(nweiz): Make the timeout configurable.
// TODO(nweiz): Reset this timer whenever the user's code interacts with
// the library.
- var timer = new Timer(new Duration(seconds: 30), () {
+ var timeout = _test.metadata.timeout.apply(new Duration(seconds: 30));
+ var timer = new Timer(timeout, () {
if (liveTest.isComplete) return;
handleError(
new TimeoutException(
- "Test timed out after 30 seconds.",
- new Duration(seconds: 30)));
+ "Test timed out after ${niceDuration(timeout)}.", timeout));
});
addOutstandingCallback();
diff --git a/lib/src/backend/metadata.dart b/lib/src/backend/metadata.dart
index 811a84a2..bf58b3f5 100644
--- a/lib/src/backend/metadata.dart
+++ b/lib/src/backend/metadata.dart
@@ -4,6 +4,7 @@
library test.backend.metadata;
+import '../frontend/timeout.dart';
import 'platform_selector.dart';
/// Metadata for a test or test suite.
@@ -14,32 +15,51 @@ class Metadata {
/// The selector indicating which platforms the suite supports.
final PlatformSelector testOn;
+ /// The modification to the timeout for the test or suite.
+ final Timeout timeout;
+
/// Creates new Metadata.
///
/// [testOn] defaults to [PlatformSelector.all].
- Metadata({PlatformSelector testOn})
- : testOn = testOn == null ? PlatformSelector.all : testOn;
+ Metadata({PlatformSelector testOn, Timeout timeout})
+ : testOn = testOn == null ? PlatformSelector.all : testOn,
+ timeout = timeout == null ? new Timeout.factor(1) : timeout;
- /// Parses metadata fields from strings.
+ /// Creates a new Metadata, but with fields parsed from strings where
+ /// applicable.
///
/// Throws a [FormatException] if any field is invalid.
- Metadata.parse({String testOn})
+ Metadata.parse({String testOn, Timeout timeout})
: this(
- testOn: testOn == null ? null : new PlatformSelector.parse(testOn));
+ testOn: testOn == null ? null : new PlatformSelector.parse(testOn),
+ timeout: timeout);
/// Dezerializes the result of [Metadata.serialize] into a new [Metadata].
Metadata.deserialize(serialized)
- : this.parse(testOn: serialized['testOn']);
+ : this.parse(
+ testOn: serialized['testOn'],
+ timeout: serialized['timeout']['duration'] == null
+ ? new Timeout.factor(serialized['timeout']['scaleFactor'])
+ : new Timeout(new Duration(
+ microseconds: serialized['timeout']['duration'])));
/// Return a new [Metadata] that merges [this] with [other].
///
/// If the two [Metadata]s have conflicting properties, [other] wins.
Metadata merge(Metadata other) =>
- new Metadata(testOn: testOn.intersect(other.testOn));
+ new Metadata(
+ testOn: testOn.intersect(other.testOn),
+ timeout: timeout.merge(other.timeout));
/// Serializes [this] into a JSON-safe object that can be deserialized using
/// [new Metadata.deserialize].
serialize() => {
- 'testOn': testOn == PlatformSelector.all ? null : testOn.toString()
+ 'testOn': testOn == PlatformSelector.all ? null : testOn.toString(),
+ 'timeout': {
+ 'duration': timeout.duration == null
+ ? null
+ : timeout.duration.inMicroseconds,
+ 'scaleFactor': timeout.scaleFactor
+ }
};
}
diff --git a/lib/src/frontend/timeout.dart b/lib/src/frontend/timeout.dart
new file mode 100644
index 00000000..d96407ef
--- /dev/null
+++ b/lib/src/frontend/timeout.dart
@@ -0,0 +1,47 @@
+// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file
+// 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.
+
+library test.frontend.timeout;
+
+/// A class representing a modification to the default timeout for a test.
+///
+/// By default, a test will time out after 30 seconds. With [new Timeout], that
+/// can be overridden entirely; with [new Timeout.factor], it can be scaled
+/// relative to the default.
+class Timeout {
+ /// The timeout duration.
+ ///
+ /// If set, this overrides the default duration entirely. It will be
+ /// non-`null` only when [scaleFactor] is `null`.
+ final Duration duration;
+
+ /// The timeout factor.
+ ///
+ /// The default timeout will be multiplied by this to get the new timeout.
+ /// Thus a factor of 2 means that the test will take twice as long to time
+ /// out, and a factor of 0.5 means that it will time out twice as quickly.
+ final num scaleFactor;
+
+ /// Declares an absolute timeout that overrides the default.
+ const Timeout(this.duration)
+ : scaleFactor = null;
+
+ /// Declares a relative timeout that scales the default.
+ const Timeout.factor(this.scaleFactor)
+ : duration = null;
+
+ /// Returns a new [Timeout] that merges [this] with [other].
+ ///
+ /// If [other] declares a [duration], that takes precedence. Otherwise, this
+ /// timeout's [duration] or [factor] are multiplied by [other]'s [factor].
+ Timeout merge(Timeout other) {
+ if (other.duration != null) return new Timeout(other.duration);
+ if (duration != null) return new Timeout(duration * other.scaleFactor);
+ return new Timeout.factor(scaleFactor * other.scaleFactor);
+ }
+
+ /// Returns a new [Duration] from applying [this] to [base].
+ Duration apply(Duration base) =>
+ duration == null ? base * scaleFactor : duration;
+}
diff --git a/lib/src/utils.dart b/lib/src/utils.dart
index 7d697981..59edb665 100644
--- a/lib/src/utils.dart
+++ b/lib/src/utils.dart
@@ -149,6 +149,25 @@ String truncate(String text, int maxLength) {
return '...$result';
}
+/// Returns a human-friendly representation of [duration].
+String niceDuration(Duration duration) {
+ var minutes = duration.inMinutes;
+ var seconds = duration.inSeconds % 59;
+ var decaseconds = (duration.inMilliseconds % 1000) ~/ 100;
+
+ var buffer = new StringBuffer();
+ if (minutes != 0) buffer.write("$minutes minutes");
+
+ if (minutes == 0 || seconds != 0) {
+ if (minutes != 0) buffer.write(", ");
+ buffer.write(seconds);
+ if (decaseconds != 0) buffer.write(".$decaseconds");
+ buffer.write(" seconds");
+ }
+
+ return buffer.toString();
+}
+
/// Merges [streams] into a single stream that emits events from all sources.
Stream mergeStreams(Iterable<Stream> streamIter) {
var streams = streamIter.toList();
diff --git a/lib/test.dart b/lib/test.dart
index 9dab658a..3839fe7f 100644
--- a/lib/test.dart
+++ b/lib/test.dart
@@ -12,6 +12,7 @@ import 'src/backend/declarer.dart';
import 'src/backend/invoker.dart';
import 'src/backend/suite.dart';
import 'src/backend/test_platform.dart';
+import 'src/frontend/timeout.dart';
import 'src/runner/reporter/no_io_compact.dart';
import 'src/utils.dart';
@@ -24,6 +25,7 @@ export 'src/frontend/prints_matcher.dart';
export 'src/frontend/test_on.dart';
export 'src/frontend/throws_matcher.dart';
export 'src/frontend/throws_matchers.dart';
+export 'src/frontend/timeout.dart';
/// The global declarer.
///
@@ -66,8 +68,12 @@ Declarer get _declarer {
/// test will only be run on matching platforms.
///
/// [platform selector]: https://github.com/dart-lang/test/#platform-selector-syntax
-void test(String description, body(), {String testOn}) =>
- _declarer.test(description, body, testOn: testOn);
+///
+/// If [timeout] is passed, it's used to modify or replace the default timeout
+/// of 30 seconds. Timeout modifications take precedence in suite-group-test
+/// order, so [timeout] will also modify any timeouts set on the group or suite.
+void test(String description, body(), {String testOn, Timeout timeout}) =>
+ _declarer.test(description, body, testOn: testOn, timeout: timeout);
/// Creates a group of tests.
///
@@ -79,8 +85,13 @@ void test(String description, body(), {String testOn}) =>
/// only be run on matching platforms.
///
/// [platform selector]: https://github.com/dart-lang/test/#platform-selector-syntax
-void group(String description, void body(), {String testOn}) =>
- _declarer.group(description, body, testOn: testOn);
+///
+/// If [timeout] is passed, it's used to modify or replace the default timeout
+/// of 30 seconds. Timeout modifications take precedence in suite-group-test
+/// order, so [timeout] will also modify any timeouts set on the suite, and will
+/// be modified by any timeouts set on individual tests.
+void group(String description, void body(), {String testOn, Timeout timeout}) =>
+ _declarer.group(description, body, testOn: testOn, timeout: timeout);
/// Registers a function to be run before tests.
///
diff --git a/test/backend/declarer_test.dart b/test/backend/declarer_test.dart
index f8640aaa..f8ed99cb 100644
--- a/test/backend/declarer_test.dart
+++ b/test/backend/declarer_test.dart
@@ -6,6 +6,7 @@ import 'dart:async';
import 'package:test/src/backend/declarer.dart';
import 'package:test/src/backend/suite.dart';
+import 'package:test/src/frontend/timeout.dart';
import 'package:test/test.dart';
Declarer _declarer;
@@ -130,6 +131,38 @@ void main() {
expect(_declarer.tests.single.name, "group description");
});
+ test("a test's timeout factor is applied to the group's", () {
+ _declarer.group("group", () {
+ _declarer.test("test", () {},
+ timeout: new Timeout.factor(3));
+ }, timeout: new Timeout.factor(2));
+
+ expect(_declarer.tests, hasLength(1));
+ expect(_declarer.tests.single.metadata.timeout.scaleFactor, equals(6));
+ });
+
+ test("a test's timeout factor is applied to the group's duration", () {
+ _declarer.group("group", () {
+ _declarer.test("test", () {},
+ timeout: new Timeout.factor(2));
+ }, timeout: new Timeout(new Duration(seconds: 10)));
+
+ expect(_declarer.tests, hasLength(1));
+ expect(_declarer.tests.single.metadata.timeout.duration,
+ equals(new Duration(seconds: 20)));
+ });
+
+ test("a test's timeout duration is applied over the group's", () {
+ _declarer.group("group", () {
+ _declarer.test("test", () {},
+ timeout: new Timeout(new Duration(seconds: 15)));
+ }, timeout: new Timeout(new Duration(seconds: 10)));
+
+ expect(_declarer.tests, hasLength(1));
+ expect(_declarer.tests.single.metadata.timeout.duration,
+ equals(new Duration(seconds: 15)));
+ });
+
group(".setUp()", () {
test("is scoped to the group", () {
var setUpRun = false;
diff --git a/test/backend/invoker_test.dart b/test/backend/invoker_test.dart
index 59d9c509..be27391e 100644
--- a/test/backend/invoker_test.dart
+++ b/test/backend/invoker_test.dart
@@ -473,27 +473,6 @@ void main() {
});
});
- test("a test times out after 30 seconds", () {
- new FakeAsync().run((async) {
- var liveTest = _localTest(() {
- Invoker.current.addOutstandingCallback();
- }).load(suite);
-
- expectStates(liveTest, [
- const State(Status.running, Result.success),
- const State(Status.complete, Result.error)
- ]);
-
- expectErrors(liveTest, [(error) {
- expect(lastState.status, equals(Status.complete));
- expect(error, new isInstanceOf<TimeoutException>());
- }]);
-
- liveTest.run();
- async.elapse(new Duration(seconds: 30));
- });
- });
-
test("a test's prints are captured and reported", () {
expect(() {
var liveTest = _localTest(() {
@@ -507,7 +486,76 @@ void main() {
return liveTest.run();
}, prints(isEmpty));
});
+
+ group("timeout:", () {
+ test("a test times out after 30 seconds by default", () {
+ new FakeAsync().run((async) {
+ var liveTest = _localTest(() {
+ Invoker.current.addOutstandingCallback();
+ }).load(suite);
+
+ expectStates(liveTest, [
+ const State(Status.running, Result.success),
+ const State(Status.complete, Result.error)
+ ]);
+
+ expectErrors(liveTest, [(error) {
+ expect(lastState.status, equals(Status.complete));
+ expect(error, new isInstanceOf<TimeoutException>());
+ }]);
+
+ liveTest.run();
+ async.elapse(new Duration(seconds: 30));
+ });
+ });
+
+ test("a test's custom timeout takes precedence", () {
+ new FakeAsync().run((async) {
+ var liveTest = _localTest(() {
+ Invoker.current.addOutstandingCallback();
+ }, metadata: new Metadata(
+ timeout: new Timeout(new Duration(seconds: 15)))).load(suite);
+
+ expectStates(liveTest, [
+ const State(Status.running, Result.success),
+ const State(Status.complete, Result.error)
+ ]);
+
+ expectErrors(liveTest, [(error) {
+ expect(lastState.status, equals(Status.complete));
+ expect(error, new isInstanceOf<TimeoutException>());
+ }]);
+
+ liveTest.run();
+ async.elapse(new Duration(seconds: 15));
+ });
+ });
+
+ test("a timeout factor is applied on top of the 30s default", () {
+ new FakeAsync().run((async) {
+ var liveTest = _localTest(() {
+ Invoker.current.addOutstandingCallback();
+ }, metadata: new Metadata(timeout: new Timeout.factor(0.5)))
+ .load(suite);
+
+ expectStates(liveTest, [
+ const State(Status.running, Result.success),
+ const State(Status.complete, Result.error)
+ ]);
+
+ expectErrors(liveTest, [(error) {
+ expect(lastState.status, equals(Status.complete));
+ expect(error, new isInstanceOf<TimeoutException>());
+ }]);
+
+ liveTest.run();
+ async.elapse(new Duration(seconds: 15));
+ });
+ });
+ });
}
-LocalTest _localTest(body(), {tearDown()}) =>
- new LocalTest("test", new Metadata(), body, tearDown: tearDown);
+LocalTest _localTest(body(), {tearDown(), Metadata metadata}) {
+ if (metadata == null) metadata = new Metadata();
+ return new LocalTest("test", metadata, body, tearDown: tearDown);
+}
--
GitLab