commit ae019bda364ea21750cbc32b1ec2379a498d4456
author Jens Johansen <jensj@google.com> Wed Mar 13 10:21:10 2024 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 13 10:21:10 2024 +0000
tree aed9bcb2625396e3ed73521e762c0ba790610fd1
parent ef6d68c040a728200d31f41d1864d69e53863cfc

[test_runner] First batch test after compiler launch gets more timeout (v2)

This is a redo of https://dart-review.googlesource.com/c/sdk/+/332220
where extra timeout wasn't given to a new process after a timeout
actually occurred, which can lead to cascades of timeouts in unfortunate
circumstances.
The extra startup time is furthermore increased from 30 seconds to 60
seconds.

Change-Id: I7d052fc1e04b374d3a6d1d82370f8edc1143291e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357280
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: William Hesse <whesse@google.com>

pkg/test_runner/lib/src/process_queue.dart

diff --git a/pkg/test_runner/lib/src/process_queue.dart b/pkg/test_runner/lib/src/process_queue.dart
index 877e5da..630e62c 100644
--- a/pkg/test_runner/lib/src/process_queue.dart
+++ b/pkg/test_runner/lib/src/process_queue.dart
@@ -977,6 +977,7 @@
   late List<String> _arguments;
   String? _runnerType;
 
+  bool _processJustStarted = false;
   io.Process? _process;
   Map<String, String>? _processEnvironmentOverrides;
   late Completer<void> _stdoutCompleter;
@@ -993,7 +994,7 @@
   Timer? _timer;
   int _testCount = 0;
 
-  static const int extraStartupTimeout = 30;
+  static const int extraStartupTimeout = 60;
 
   BatchRunnerProcess({bool useJson = true}) : _useJson = useJson;
 
@@ -1019,17 +1020,13 @@
     if (_process == null) {
       // Start process if not yet started.
       _startProcess(() {
-        // We just started the process, add some extra timeout to account for
-        // the startup cost of the batch compiler.
-        doStartTest(command, timeout + extraStartupTimeout);
+        doStartTest(command, timeout);
       });
     } else if (!sameRunnerType || clearMemoryLeak) {
       // Restart this runner with the right executable for this test if needed.
       _processExitHandler = (_) {
         _startProcess(() {
-          // We just started the process, add some extra timeout to account for
-          // the startup cost of the batch compiler.
-          doStartTest(command, timeout + extraStartupTimeout);
+          doStartTest(command, timeout);
         });
       };
       _process!.kill();
@@ -1059,6 +1056,12 @@
   }
 
   void doStartTest(Command command, int timeout) {
+    if (_processJustStarted) {
+      // We just started the process, add some extra timeout to account for
+      // the startup cost of the batch compiler.
+      _processJustStarted = false;
+      timeout += extraStartupTimeout;
+    }
     _startTime = DateTime.now();
     _testStdout = OutputLog();
     _testStderr = OutputLog();
@@ -1143,6 +1146,7 @@
     try {
       _process = await io.Process.start(executable, arguments,
           environment: environment);
+      _processJustStarted = true;
     } catch (e) {
       // TODO(floitsch): should we try to report the stacktrace?
       print("Process error:");