Stop putting suite loads into active test (#1587)
\Towards #1311
Removes a UX concern from `Engine` which no longer needs to worry about
how a reporter might display information about load suites. Makes the
interaction between the engine and reporters less subtle.
Use the term "suite load" over "load test" to start pushing terminology
away from conflating tests and loading.
diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index d3d561f..3bdc457 100644
--- a/pkgs/test/CHANGELOG.md
+++ b/pkgs/test/CHANGELOG.md
@@ -1,3 +1,5 @@
+## 1.18.1-dev
+
## 1.18.0
* Add configuration to disallow duplicate test and group names. See the
diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml
index 2430eb0..55f8619 100644
--- a/pkgs/test/pubspec.yaml
+++ b/pkgs/test/pubspec.yaml
@@ -1,5 +1,5 @@
name: test
-version: 1.18.0
+version: 1.18.1-dev
description: >-
A full featured library for writing and running Dart tests across platforms.
repository: https://github.com/dart-lang/test/blob/master/pkgs/test
@@ -33,7 +33,7 @@
yaml: ^3.0.0
# Use an exact version until the test_api and test_core package are stable.
test_api: 0.4.4
- test_core: 0.4.3
+ test_core: 0.4.4
dev_dependencies:
fake_async: ^1.0.0
diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md
index 40134c6..9a7e8ee 100644
--- a/pkgs/test_core/CHANGELOG.md
+++ b/pkgs/test_core/CHANGELOG.md
@@ -1,3 +1,5 @@
+## 0.4.4-dev
+
## 0.4.3
* Add an option to disallow duplicate test or group names in `directRunTests`.
diff --git a/pkgs/test_core/lib/src/runner/engine.dart b/pkgs/test_core/lib/src/runner/engine.dart
index 32bee8f..b26b9c3 100644
--- a/pkgs/test_core/lib/src/runner/engine.dart
+++ b/pkgs/test_core/lib/src/runner/engine.dart
@@ -36,15 +36,10 @@
///
/// The engine has some special logic for [LoadSuite]s and the tests they
/// contain, referred to as "load tests". Load tests exist to provide visibility
-/// into the process of loading test files, but as long as that process is
-/// proceeding normally users usually don't care about it, so the engine only
-/// surfaces running load tests (that is, includes them in [liveTests] and other
-/// collections) under specific circumstances.
-///
-/// If only load tests are running, exactly one load test will be in [active]
-/// and [liveTests]. If this test passes, it will be removed from both [active]
-/// and [liveTests] and *will not* be added to [passed]. If at any point a load
-/// test fails, it will be added to [failed] and [liveTests].
+/// into the process of loading test files. As long as that process is
+/// proceeding normally users usually don't care about it, so the engine does
+/// not include them in [liveTests] and other collections.
+/// If a load test fails, it will be added to [failed] and [liveTests].
///
/// The test suite loaded by a load suite will be automatically be run by the
/// engine; it doesn't need to be added to [suiteSink] manually.
@@ -174,22 +169,21 @@
Set<LiveTest> get failed => _failedGroup.set;
final _failedGroup = UnionSetController<LiveTest>(disjoint: true);
- /// The tests that are still running, in the order they begain running.
+ /// The tests that are still running, in the order they began running.
List<LiveTest> get active => UnmodifiableListView(_active);
final _active = QueueList<LiveTest>();
+ /// The suites that are still loading, in the order they began.
+ List<LiveTest> get activeSuiteLoads =>
+ UnmodifiableListView(_activeSuiteLoads);
+ final _activeSuiteLoads = <LiveTest>{};
+
/// The set of tests that have been marked for restarting.
///
/// This is always a subset of [active]. Once a test in here has finished
/// running, it's run again.
final _restarted = <LiveTest>{};
- /// The tests from [LoadSuite]s that are still running, in the order they
- /// began running.
- ///
- /// This is separate from [active] because load tests aren't always surfaced.
- final _activeLoadTests = <LiveTest>{};
-
/// Whether this engine is idle—that is, not currently executing a test.
bool get isIdle => _group.isIdle;
@@ -356,21 +350,11 @@
await _onUnpaused;
_active.add(liveTest);
- // If there were no active non-load tests, the current active test would
- // have been a load test. In that case, remove it, since now we have a
- // non-load test to add.
- if (_active.first.suite is LoadSuite) _active.removeFirst();
-
var subscription = liveTest.onStateChange.listen(null);
subscription
..onData((state) {
if (state.status != Status.complete) return;
_active.remove(liveTest);
-
- // If we're out of non-load tests, surface a load test.
- if (_active.isEmpty && _activeLoadTests.isNotEmpty) {
- _active.add(_activeLoadTests.first);
- }
})
..onDone(() {
_subscriptions.remove(subscription);
@@ -425,7 +409,7 @@
///
/// Returns the same future as [LiveTest.close].
Future restartTest(LiveTest liveTest) async {
- if (_activeLoadTests.contains(liveTest)) {
+ if (_activeSuiteLoads.contains(liveTest)) {
throw ArgumentError("Can't restart a load test.");
}
@@ -447,24 +431,13 @@
_addLiveSuite(controller.liveSuite);
var liveTest = suite.test.load(suite);
- _activeLoadTests.add(liveTest);
-
- // Only surface the load test if there are no other tests currently running.
- if (_active.isEmpty) _active.add(liveTest);
+ _activeSuiteLoads.add(liveTest);
var subscription = liveTest.onStateChange.listen(null);
subscription
..onData((state) {
if (state.status != Status.complete) return;
- _activeLoadTests.remove(liveTest);
-
- // Only one load test will be active at any given time, and it will always
- // be the only active test. Remove it and, if possible, surface another
- // load test.
- if (_active.isNotEmpty && _active.first.suite == suite) {
- _active.remove(liveTest);
- if (_activeLoadTests.isNotEmpty) _active.add(_activeLoadTests.last);
- }
+ _activeSuiteLoads.remove(liveTest);
})
..onDone(() {
_subscriptions.remove(subscription);
@@ -547,7 +520,7 @@
// Close the running tests first so that we're sure to wait for them to
// finish before we close their suites and cause them to become unloaded.
- var allLiveTests = liveTests.toSet()..addAll(_activeLoadTests);
+ var allLiveTests = liveTests.toSet()..addAll(_activeSuiteLoads);
var futures = allLiveTests.map((liveTest) => liveTest.close()).toList();
// Closing the run pool will close the test suites as soon as their tests
diff --git a/pkgs/test_core/lib/src/runner/reporter/compact.dart b/pkgs/test_core/lib/src/runner/reporter/compact.dart
index f02b7c9..eb3646f 100644
--- a/pkgs/test_core/lib/src/runner/reporter/compact.dart
+++ b/pkgs/test_core/lib/src/runner/reporter/compact.dart
@@ -186,10 +186,12 @@
.listen((_) => _progressLine(_lastProgressMessage ?? '')));
}
- // If this is the first test to start, print a progress line so the user
- // knows what's running. It's possible that the active test may not be
- // [liveTest] because the engine doesn't always surface load tests.
- if (_engine.active.length == 1 && _engine.active.first == liveTest) {
+ // If this is the first test or suite load to start, print a progress line
+ // so the user knows what's running.
+ if ((_engine.active.length == 1 && _engine.active.first == liveTest) ||
+ (_engine.active.isEmpty &&
+ _engine.activeSuiteLoads.length == 1 &&
+ _engine.activeSuiteLoads.first == liveTest)) {
_progressLine(_description(liveTest));
}
diff --git a/pkgs/test_core/lib/src/runner/reporter/expanded.dart b/pkgs/test_core/lib/src/runner/reporter/expanded.dart
index a5a3b99..cd6a490 100644
--- a/pkgs/test_core/lib/src/runner/reporter/expanded.dart
+++ b/pkgs/test_core/lib/src/runner/reporter/expanded.dart
@@ -164,8 +164,9 @@
// emit information about them unless they fail.
_subscriptions.add(liveTest.onStateChange
.listen((state) => _onStateChange(liveTest, state)));
- } else if (_engine.active.length == 1 &&
- _engine.active.first == liveTest &&
+ } else if (_engine.active.isEmpty &&
+ _engine.activeSuiteLoads.length == 1 &&
+ _engine.activeSuiteLoads.first == liveTest &&
liveTest.test.name.startsWith('compiling ')) {
// Print a progress line for load tests that come from compiling JS, since
// that takes a long time.
diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml
index 5218e79..6b6e944 100644
--- a/pkgs/test_core/pubspec.yaml
+++ b/pkgs/test_core/pubspec.yaml
@@ -1,5 +1,5 @@
name: test_core
-version: 0.4.3
+version: 0.4.4-dev
description: A basic library for writing tests and running them on the VM.
homepage: https://github.com/dart-lang/test/blob/master/pkgs/test_core
