From 5d571d64a298d476689dd60113b0fc67fdb7e157 Mon Sep 17 00:00:00 2001
From: Nate Bosch <nbosch@google.com>
Date: Thu, 3 Aug 2023 10:03:26 -0700
Subject: [PATCH] Add --fail-fast flag (#2040)

Closes #1841

Add a `--fail-fast` flag which stops running test cases after the first
failure.

- in the engine, check for failed tests immediately after running them.
  when fast failures are enabled `close()` the engine after any failure.
  Teardowns and cleanups will run, and any concurrently running tests in
  other suits will finish, but no further tests will start.
- add the argument in `configuration` and parse it with the args.
- add a test with a failing case that prevents later tests from running.
---
 pkgs/test/CHANGELOG.md                        |  4 +++
 pkgs/test/pubspec.yaml                        |  4 +--
 pkgs/test/test/runner/engine_test.dart        | 13 +++++++++
 pkgs/test/test/runner/runner_test.dart        |  1 +
 pkgs/test/test/utils.dart                     | 27 ++++++++++++------
 pkgs/test_core/CHANGELOG.md                   |  4 +++
 pkgs/test_core/lib/src/runner.dart            |  8 ++++--
 .../lib/src/runner/configuration.dart         | 18 ++++++++++++
 .../lib/src/runner/configuration/args.dart    |  3 ++
 pkgs/test_core/lib/src/runner/engine.dart     | 28 +++++++++++++++----
 pkgs/test_core/pubspec.yaml                   |  2 +-
 11 files changed, 92 insertions(+), 20 deletions(-)

diff --git a/pkgs/test/CHANGELOG.md b/pkgs/test/CHANGELOG.md
index f071aa847..d0d24dad0 100644
--- a/pkgs/test/CHANGELOG.md
+++ b/pkgs/test/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 1.24.6-wip
+
+* Add support for discontinuing after the first failing test with `--fail-fast`.
+
 ## 1.24.5
 
 * Change "compiling <path>" to "loading <path>" message in all cases. Surface
diff --git a/pkgs/test/pubspec.yaml b/pkgs/test/pubspec.yaml
index 88ae13348..b690cb6f2 100644
--- a/pkgs/test/pubspec.yaml
+++ b/pkgs/test/pubspec.yaml
@@ -1,5 +1,5 @@
 name: test
-version: 1.24.5
+version: 1.24.6-wip
 description: >-
   A full featured library for writing and running Dart tests across platforms.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test
@@ -35,7 +35,7 @@ dependencies:
 
   # Use an exact version until the test_api and test_core package are stable.
   test_api: 0.6.1
-  test_core: 0.5.5
+  test_core: 0.5.6
 
   typed_data: ^1.3.0
   web_socket_channel: ^2.0.0
diff --git a/pkgs/test/test/runner/engine_test.dart b/pkgs/test/test/runner/engine_test.dart
index 189c548df..428ca0f18 100644
--- a/pkgs/test/test/runner/engine_test.dart
+++ b/pkgs/test/test/runner/engine_test.dart
@@ -129,6 +129,19 @@ void main() {
     expect(engine.run(), completion(isFalse));
   });
 
+  test('.run() does not run more tests after failure for stopOnFirstFailure',
+      () async {
+    var secondTestRan = false;
+    var engine = declareEngine(() {
+      test('failure', () => throw 'oh no');
+      test('subsequent', () {
+        secondTestRan = true;
+      });
+    }, stopOnFirstFailure: true);
+    await expectLater(engine.run(), completion(isFalse));
+    expect(secondTestRan, false);
+  });
+
   test('.run() may not be called more than once', () {
     var engine = Engine.withSuites([]);
     expect(engine.run(), completes);
diff --git a/pkgs/test/test/runner/runner_test.dart b/pkgs/test/test/runner/runner_test.dart
index e2ae78293..6d9a2ffae 100644
--- a/pkgs/test/test/runner/runner_test.dart
+++ b/pkgs/test/test/runner/runner_test.dart
@@ -102,6 +102,7 @@ $_runtimeCompilers
                                       Must be a 32bit unsigned integer or "random".
                                       If "random", pick a random seed to use.
                                       If not passed, do not randomize test case execution order.
+    --[no-]fail-fast                  Stop running tests after the first failure.
 
 Output:
 -r, --reporter=<option>               Set how to print test results.
diff --git a/pkgs/test/test/utils.dart b/pkgs/test/test/utils.dart
index 3c3ef0165..52c708db4 100644
--- a/pkgs/test/test/utils.dart
+++ b/pkgs/test/test/utils.dart
@@ -195,16 +195,24 @@ List<GroupEntry> declare(void Function() body) {
 }
 
 /// Runs [body] with a declarer and returns an engine that runs those tests.
-Engine declareEngine(void Function() body,
-    {bool runSkipped = false, String? coverage}) {
+Engine declareEngine(
+  void Function() body, {
+  bool runSkipped = false,
+  String? coverage,
+  bool stopOnFirstFailure = false,
+}) {
   var declarer = Declarer()..declare(body);
-  return Engine.withSuites([
-    RunnerSuite(
-        const PluginEnvironment(),
-        SuiteConfiguration.runSkipped(runSkipped),
-        declarer.build(),
-        suitePlatform)
-  ], coverage: coverage);
+  return Engine.withSuites(
+    [
+      RunnerSuite(
+          const PluginEnvironment(),
+          SuiteConfiguration.runSkipped(runSkipped),
+          declarer.build(),
+          suitePlatform)
+    ],
+    coverage: coverage,
+    stopOnFirstFailure: stopOnFirstFailure,
+  );
 }
 
 /// Returns a [RunnerSuite] with a default environment and configuration.
@@ -348,6 +356,7 @@ Configuration configuration(
         tags: tags,
         onPlatform: onPlatform,
         testRandomizeOrderingSeed: testRandomizeOrderingSeed,
+        stopOnFirstFailure: false,
         timeout: timeout,
         verboseTrace: verboseTrace,
         chainStackTraces: chainStackTraces,
diff --git a/pkgs/test_core/CHANGELOG.md b/pkgs/test_core/CHANGELOG.md
index 85d88f520..3d3715755 100644
--- a/pkgs/test_core/CHANGELOG.md
+++ b/pkgs/test_core/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.5.6-wip
+
+* Add support for discontinuing after the first failing test with `--fail-fast`.
+
 ## 0.5.5
 
 * Change "compiling <path>" to "loading <path>" message in all cases. Surface
diff --git a/pkgs/test_core/lib/src/runner.dart b/pkgs/test_core/lib/src/runner.dart
index 05531368a..c458b5d23 100644
--- a/pkgs/test_core/lib/src/runner.dart
+++ b/pkgs/test_core/lib/src/runner.dart
@@ -75,9 +75,11 @@ class Runner {
   /// Creates a new runner based on [configuration].
   factory Runner(Configuration config) => config.asCurrent(() {
         var engine = Engine(
-            concurrency: config.concurrency,
-            coverage: config.coverage,
-            testRandomizeOrderingSeed: config.testRandomizeOrderingSeed);
+          concurrency: config.concurrency,
+          coverage: config.coverage,
+          testRandomizeOrderingSeed: config.testRandomizeOrderingSeed,
+          stopOnFirstFailure: config.stopOnFirstFailure,
+        );
 
         var sinks = <IOSink>[];
         Reporter createFileReporter(String reporterName, String filepath) {
diff --git a/pkgs/test_core/lib/src/runner/configuration.dart b/pkgs/test_core/lib/src/runner/configuration.dart
index b8d568c0a..12d3aa665 100644
--- a/pkgs/test_core/lib/src/runner/configuration.dart
+++ b/pkgs/test_core/lib/src/runner/configuration.dart
@@ -207,6 +207,11 @@ class Configuration {
   /// The same seed will shuffle the tests in the same way every time.
   final int? testRandomizeOrderingSeed;
 
+  final bool? _stopOnFirstFailure;
+
+  /// Whether to stop running subsequent tests after a test fails.
+  bool get stopOnFirstFailure => _stopOnFirstFailure ?? false;
+
   /// Returns the current configuration, or a default configuration if no
   /// current configuration is set.
   ///
@@ -270,6 +275,7 @@ class Configuration {
       required Map<String, CustomRuntime>? defineRuntimes,
       required bool? noRetry,
       required int? testRandomizeOrderingSeed,
+      required bool? stopOnFirstFailure,
 
       // Suite-level configuration
       required bool? allowDuplicateTestNames,
@@ -322,6 +328,7 @@ class Configuration {
         defineRuntimes: defineRuntimes,
         noRetry: noRetry,
         testRandomizeOrderingSeed: testRandomizeOrderingSeed,
+        stopOnFirstFailure: stopOnFirstFailure,
         includeTags: includeTags,
         excludeTags: excludeTags,
         globalPatterns: globalPatterns,
@@ -379,6 +386,7 @@ class Configuration {
           Map<String, CustomRuntime>? defineRuntimes,
           bool? noRetry,
           int? testRandomizeOrderingSeed,
+          bool? stopOnFirstFailure,
 
           // Suite-level configuration
           bool? allowDuplicateTestNames,
@@ -430,6 +438,7 @@ class Configuration {
           defineRuntimes: defineRuntimes,
           noRetry: noRetry,
           testRandomizeOrderingSeed: testRandomizeOrderingSeed,
+          stopOnFirstFailure: stopOnFirstFailure,
           allowDuplicateTestNames: allowDuplicateTestNames,
           allowTestRandomization: allowTestRandomization,
           jsTrace: jsTrace,
@@ -496,6 +505,7 @@ class Configuration {
         defineRuntimes: null,
         noRetry: null,
         testRandomizeOrderingSeed: null,
+        stopOnFirstFailure: null,
         ignoreTimeouts: null,
         allowDuplicateTestNames: null,
         allowTestRandomization: null,
@@ -563,6 +573,7 @@ class Configuration {
         defineRuntimes: null,
         noRetry: null,
         testRandomizeOrderingSeed: null,
+        stopOnFirstFailure: null,
         jsTrace: null,
         runSkipped: null,
         dart2jsArgs: null,
@@ -627,6 +638,7 @@ class Configuration {
         defineRuntimes: null,
         noRetry: null,
         testRandomizeOrderingSeed: null,
+        stopOnFirstFailure: null,
         allowDuplicateTestNames: null,
         allowTestRandomization: null,
         jsTrace: null,
@@ -689,6 +701,7 @@ class Configuration {
           overrideRuntimes: null,
           noRetry: null,
           testRandomizeOrderingSeed: null,
+          stopOnFirstFailure: null,
           allowDuplicateTestNames: null,
           allowTestRandomization: null,
           jsTrace: null,
@@ -754,6 +767,7 @@ class Configuration {
       required Map<String, CustomRuntime>? defineRuntimes,
       required bool? noRetry,
       required this.testRandomizeOrderingSeed,
+      required bool? stopOnFirstFailure,
       required BooleanSelector? includeTags,
       required BooleanSelector? excludeTags,
       required Iterable<Pattern>? globalPatterns,
@@ -786,6 +800,7 @@ class Configuration {
         globalPatterns = globalPatterns == null
             ? const {}
             : UnmodifiableSetView(globalPatterns.toSet()),
+        _stopOnFirstFailure = stopOnFirstFailure,
         suiteDefaults = (() {
           var config = suiteDefaults ?? SuiteConfiguration.empty;
           if (pauseAfterLoad == true) {
@@ -839,6 +854,7 @@ class Configuration {
         defineRuntimes: null,
         noRetry: null,
         testRandomizeOrderingSeed: null,
+        stopOnFirstFailure: null,
         includeTags: null,
         excludeTags: null,
       );
@@ -944,6 +960,7 @@ class Configuration {
         noRetry: other._noRetry ?? _noRetry,
         testRandomizeOrderingSeed:
             other.testRandomizeOrderingSeed ?? testRandomizeOrderingSeed,
+        stopOnFirstFailure: other._stopOnFirstFailure ?? _stopOnFirstFailure,
         includeTags: includeTags.intersection(other.includeTags),
         excludeTags: excludeTags.union(other.excludeTags),
         globalPatterns: globalPatterns.union(other.globalPatterns),
@@ -1034,6 +1051,7 @@ class Configuration {
         noRetry: noRetry ?? _noRetry,
         testRandomizeOrderingSeed:
             testRandomizeOrderingSeed ?? this.testRandomizeOrderingSeed,
+        stopOnFirstFailure: _stopOnFirstFailure,
         includeTags: includeTags,
         excludeTags: excludeTags,
         globalPatterns: globalPatterns,
diff --git a/pkgs/test_core/lib/src/runner/configuration/args.dart b/pkgs/test_core/lib/src/runner/configuration/args.dart
index b8baf483c..5faa52da7 100644
--- a/pkgs/test_core/lib/src/runner/configuration/args.dart
+++ b/pkgs/test_core/lib/src/runner/configuration/args.dart
@@ -134,6 +134,8 @@ final ArgParser _parser = (() {
           'Must be a 32bit unsigned integer or "random".\n'
           'If "random", pick a random seed to use.\n'
           'If not passed, do not randomize test case execution order.');
+  parser.addFlag('fail-fast',
+      help: 'Stop running tests after the first failure.\n');
 
   var reporterDescriptions = <String, String>{};
   for (var reporter in allReporters.keys) {
@@ -348,6 +350,7 @@ class _Parser {
         noRetry: _ifParsed('no-retry'),
         testRandomizeOrderingSeed: testRandomizeOrderingSeed,
         ignoreTimeouts: _ifParsed('ignore-timeouts'),
+        stopOnFirstFailure: _ifParsed('fail-fast'),
         // Config that isn't supported on the command line
         addTags: null,
         allowTestRandomization: null,
diff --git a/pkgs/test_core/lib/src/runner/engine.dart b/pkgs/test_core/lib/src/runner/engine.dart
index 578a6bf4d..665fd60ef 100644
--- a/pkgs/test_core/lib/src/runner/engine.dart
+++ b/pkgs/test_core/lib/src/runner/engine.dart
@@ -69,6 +69,9 @@ class Engine {
   /// The same seed will shuffle the tests in the same way every time.
   int? testRandomizeOrderingSeed;
 
+  /// Whether to stop running tests after a failure.
+  bool _stopOnFirstFailure;
+
   /// A pool that limits the number of test suites running concurrently.
   final Pool _runPool;
 
@@ -202,8 +205,16 @@ class Engine {
   /// Omitting this argument or passing `0` disables shuffling.
   ///
   /// [coverage] specifies a directory to output coverage information.
-  Engine({int? concurrency, String? coverage, this.testRandomizeOrderingSeed})
-      : _runPool = Pool(concurrency ?? 1),
+  ///
+  /// If [stopOnFirstFailure] then a single failing test will cause the engine
+  /// to [close] and stop ruunning further tests.
+  Engine({
+    int? concurrency,
+    String? coverage,
+    this.testRandomizeOrderingSeed,
+    bool stopOnFirstFailure = false,
+  })  : _runPool = Pool(concurrency ?? 1),
+        _stopOnFirstFailure = stopOnFirstFailure,
         _coverage = coverage {
     _group.future.then((_) {
       _onTestStartedGroup.close();
@@ -222,8 +233,12 @@ class Engine {
   /// [concurrency] controls how many suites are run at once. If [runSkipped] is
   /// `true`, skipped tests will be run as though they weren't skipped.
   factory Engine.withSuites(List<RunnerSuite> suites,
-      {int? concurrency, String? coverage}) {
-    var engine = Engine(concurrency: concurrency, coverage: coverage);
+      {int? concurrency, String? coverage, bool stopOnFirstFailure = false}) {
+    var engine = Engine(
+      concurrency: concurrency,
+      coverage: coverage,
+      stopOnFirstFailure: stopOnFirstFailure,
+    );
     for (var suite in suites) {
       engine.suiteSink.add(suite);
     }
@@ -371,7 +386,10 @@ class Engine {
     // loop pump to avoid starving non-microtask events.
     await Future(() {});
 
-    if (!_restarted.contains(liveTest)) return;
+    if (!_restarted.contains(liveTest)) {
+      if (_stopOnFirstFailure && liveTest.state.result.isFailing) close();
+      return;
+    }
     await _runLiveTest(suiteController, liveTest.copy(),
         countSuccess: countSuccess);
     _restarted.remove(liveTest);
diff --git a/pkgs/test_core/pubspec.yaml b/pkgs/test_core/pubspec.yaml
index ba7e5c8b2..3d374478b 100644
--- a/pkgs/test_core/pubspec.yaml
+++ b/pkgs/test_core/pubspec.yaml
@@ -1,5 +1,5 @@
 name: test_core
-version: 0.5.5
+version: 0.5.6-wip
 description: A basic library for writing tests and running them on the VM.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test_core