[test_runner] Improve timeout deflaking

The usual deflaking logic is to simply rerun the test 5 times, but if the failure was a timeout this can be very expensive. Instead, only rerun timeout failures twice, and use the last successful run time to set a tighter timeout.

Repeat counts and timeouts are now per-test. The name, repeat count, and timeout make a `DeflakeInfo`. This object is constructed in compare_results.dart, and passed to test.py's `--tests` flag. The recipe was already passing the output of compare_results directly to that flag, so no change is needed to the recipies.

Backwards compatibility:

If you want the old behavior of compare_results.dart, use the `--name-only` flag.

test.py only uses this new logic if it detects that the `--tests` flag is JSON (if it starts with a `{`).

Change-Id: I4113b68c54bfb7fd9a5e8fc9dab7a265807f3e77
Bug: #55044
Fixes: #55044
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435760
Commit-Queue: Liam Appelbe <[email protected]>
Reviewed-by: Alexander Thomas <[email protected]>

Author: liamappelbe

pkg/test_runner/bin/compare_results.dart

@@ -8,10 +8,12 @@
 // readable mode that explains the results and how they changed.
 
 import 'dart:collection';
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:args/args.dart';
 import 'package:test_runner/bot_results.dart';
+import 'package:test_runner/src/deflake_info.dart';
 
 class Event {
   final Result? before;
@@ -50,6 +52,18 @@ class Event {
       throw Exception("Unreachable");
     }
   }
+
+  String deflakeInfo(String name) {
+    final isTimeout = after.outcome == 'Timeout';
+    final lastTimeMs = before?.timeMs;
+    return jsonEncode(DeflakeInfo(
+      name: name,
+      repeat: isTimeout ? 2 : 5,
+      timeout: isTimeout && lastTimeMs != null
+          ? ((2 * lastTimeMs) / 1000).ceil()
+          : -1,
+    ).toJson());
+  }
 }
 
 class Options {
@@ -70,6 +84,7 @@ class Options {
   Iterable<String> get statusFilter => ["passing", "flaky", "failing"]
       .where((option) => _options[option] as bool);
   bool get unchanged => _options["unchanged"] as bool;
+  bool get nameOnly => _options["name-only"] as bool;
   bool get verbose => _options["verbose"] as bool;
   List<String> get rest => _options.rest;
 }
@@ -148,8 +163,10 @@ bool search(
             "${before?.matches} ${after.matches} "
             "${before?.flaked} ${after.flaked}";
       }
-    } else {
+    } else if (options.nameOnly) {
       output = name;
+    } else {
+      output = event.deflakeInfo(name);
     }
     final log = logs[event.after.key];
     final bar = '=' * (output.length + 2);
@@ -191,6 +208,8 @@ void main(List<String> args) async {
       abbr: 'u',
       negatable: false,
       help: "Show only tests with unchanged results.");
+  parser.addFlag("name-only",
+      help: "Only show the test names.", negatable: false);
   parser.addFlag("verbose",
       abbr: "v",
       help: "Show the old and new result for each test",

pkg/test_runner/lib/bot_results.dart

@@ -30,6 +30,7 @@ class Result {
   final bool flaked; // From optional flakiness_data argument to constructor.
   final bool? isFlaky; // From results.json after it is extended.
   final String? previousOutcome;
+  final int? timeMs;
 
   Result(
     this.configuration,
@@ -40,7 +41,8 @@ class Result {
     this.changed,
     this.commitHash,
     this.isFlaky,
-    this.previousOutcome, {
+    this.previousOutcome,
+    this.timeMs, {
     this.flaked = false,
   });
 
@@ -56,6 +58,7 @@ class Result {
         commitHash = map["commit_hash"] as String?,
         isFlaky = map["flaky"] as bool?,
         previousOutcome = map["previous_result"] as String?,
+        timeMs = map["timeMs"] as int?,
         flaked = flakinessData != null &&
             (flakinessData["active"] ?? true) == true &&
             (flakinessData["outcomes"] as List).contains(map["result"]);

pkg/test_runner/lib/src/configuration.dart

@@ -11,6 +11,7 @@ import 'package:smith/configuration.dart';
 import 'package:smith/smith.dart';
 
 import 'compiler_configuration.dart';
+import 'deflake_info.dart';
 import 'feature.dart';
 import 'path.dart';
 import 'repository.dart';
@@ -31,7 +32,8 @@ class TestConfiguration {
       this.selectors = const {},
       this.build = false,
       this.testList = const [],
-      this.repeat = 1,
+      this.deflakeInfoMap = const {},
+      int repeat = 1,
       this.batch = false,
       this.copyCoreDumps = false,
       this.rr = false,
@@ -74,7 +76,8 @@ class TestConfiguration {
       : packages = packages ??
             Repository.uri
                 .resolve('.dart_tool/package_config.json')
-                .toFilePath();
+                .toFilePath(),
+        _repeat = repeat;
 
   final Map<String, RegExp?> selectors;
   final Progress progress;
@@ -136,11 +139,12 @@ class TestConfiguration {
   final String? dartPrecompiledPath;
   final String? genSnapshotPath;
   final List<String>? testList;
+  final Map<String, DeflakeInfo> deflakeInfoMap;
 
   final int taskCount;
   final int shardCount;
   final int shard;
-  final int repeat;
+  final int _repeat;
 
   final int testServerPort;
   final int testServerCrossOriginPort;
@@ -221,15 +225,15 @@ class TestConfiguration {
   ///     build/ReleaseX64
   String get buildDirectory => system.outputDirectory + configurationDirectory;
 
-  int? _timeout;
+  int? _defaultTimeout;
 
   // TODO(whesse): Put non-default timeouts explicitly in configs, not this.
   /// Calculates a default timeout based on the compiler and runtime used,
   /// and the mode, architecture, etc.
-  int get timeout {
-    if (_timeout == null) {
+  int get defaultTimeout {
+    if (_defaultTimeout == null) {
       if (configuration.timeout > 0) {
-        _timeout = configuration.timeout;
+        _defaultTimeout = configuration.timeout;
       } else {
         var isReload = hotReload || hotReloadRollback;
 
@@ -241,13 +245,22 @@ class TestConfiguration {
             arch: architecture,
             system: system);
 
-        _timeout = 30 * compilerMultiplier * runtimeMultiplier;
+        _defaultTimeout = 30 * compilerMultiplier * runtimeMultiplier;
       }
     }
 
-    return _timeout!;
+    return _defaultTimeout!;
   }
 
+  /// Returns the timeout for the given test name.
+  int timeout(String name) {
+    final t = deflakeInfoMap[name]?.timeout ?? -1;
+    return t >= 0 ? t : defaultTimeout;
+  }
+
+  /// Returns the repeat count for the given test name.
+  int repeat(String name) => deflakeInfoMap[name]?.repeat ?? _repeat;
+
   List<String> get standardOptions {
     if (compiler != Compiler.dart2js) {
       return const ["--ignore-unrecognized-flags"];

pkg/test_runner/lib/src/deflake_info.dart

@@ -0,0 +1,25 @@
+// Copyright (c) 2025, 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.
+
+/// During deflaking, each test can be run with a custom timeout and repetition
+/// count.
+class DeflakeInfo {
+  String name;
+  int repeat;
+  int timeout;
+
+  DeflakeInfo(
+      {required this.name, required this.repeat, required this.timeout});
+
+  Map<dynamic, dynamic> toJson() => {
+        'name': name,
+        'repeat': repeat,
+        'timeout': timeout,
+      };
+
+  static DeflakeInfo fromJson(Map<dynamic, dynamic> json) => DeflakeInfo(
+      name: json['name'] as String,
+      repeat: json['repeat'] as int? ?? 5,
+      timeout: json['timeout'] as int? ?? -1);
+}

pkg/test_runner/lib/src/options.dart

@@ -11,6 +11,7 @@ import 'package:args/args.dart';
 import 'package:path/path.dart' as path;
 
 import 'configuration.dart';
+import 'deflake_info.dart';
 import 'path.dart';
 import 'repository.dart';
 import 'test_configurations.dart';
@@ -626,12 +627,17 @@ has been specified on the command line.''')
 
     void addConfiguration(Configuration innerConfiguration,
         [String? namedConfiguration]) {
+
+      final (testList, deflakeInfoMap) = parseTestList(
+          data["test-list-contents"] as List<String>?);
+
       var configuration = TestConfiguration(
           configuration: innerConfiguration,
           progress: progress,
           selectors: _expandSelectors(data),
           build: data["build"] as bool,
-          testList: data["test-list-contents"] as List<String>?,
+          testList: testList,
+          deflakeInfoMap: deflakeInfoMap ?? const {},
           repeat: int.parse(data["repeat"] as String),
           batch: !(data["no-batch"] as bool),
           copyCoreDumps: data["copy-coredumps"] as bool,
@@ -1009,3 +1015,16 @@ final Map<String, String> sanitizerEnvironmentVariables = (() {
 
   return environment;
 })();
+
+(List<String>?, Map<String, DeflakeInfo>?) parseTestList(List<String>? raw) {
+  final isJson = raw != null && raw.isNotEmpty && raw[0].startsWith('{');
+  if (!isJson) return (raw, null);
+  final deflakes = <DeflakeInfo>[
+    for (var line in raw)
+        DeflakeInfo.fromJson(jsonDecode(line) as Map<dynamic, dynamic>),
+  ];
+  return (
+    [ for (var i in deflakes) i.name ],
+    { for (var i in deflakes) i.name: i },
+  );
+}

pkg/test_runner/lib/src/process_queue.dart

@@ -253,7 +253,7 @@ class TestCaseEnqueuer {
   /// test completing successfully, just on it completing.
   void _add(TestCase testCase) {
     Node<Command>? lastNode;
-    for (var i = 0; i < testCase.configuration.repeat; ++i) {
+    for (var i = 0; i < testCase.repeat; ++i) {
       if (i > 0) {
         testCase = testCase.indexedCopy(i);
       }

pkg/test_runner/lib/src/test_case.dart

@@ -133,7 +133,7 @@ class TestCase {
   }
 
   int get timeout {
-    var result = configuration.timeout;
+    var result = configuration.timeout(displayName);
     if (expectedOutcomes.contains(Expectation.slow)) {
       result *= _slowTimeoutMultiplier;
     } else if (expectedOutcomes.contains(Expectation.extraSlow)) {
@@ -142,6 +142,8 @@ class TestCase {
     return result;
   }
 
+  int get repeat => configuration.repeat(displayName);
+
   String get configurationString {
     var compiler = configuration.compiler.name;
     var runtime = configuration.runtime.name;

pkg/test_runner/lib/test_runner.dart

@@ -577,6 +577,7 @@ Future<void> deflake(Directory outDirectory, List<String> configurations,
   // Find the list of tests to deflake.
   var deflakeListOutput = await runProcess(Platform.resolvedExecutable, [
     "tools/bots/compare_results.dart",
+    "--name-only",
     "--changed",
     "--failing",
     "--passing",

pkg/test_runner/test/compare_results/compare_results_test.dart

@@ -197,7 +197,8 @@ Result _result(
     String commitHash = 'abcdabcd',
     bool flaked = false,
     bool isFlaky = false,
-    String previousOutcome = 'Pass'}) {
+    String previousOutcome = 'Pass',
+    int timeMs = -1}) {
   return Result(
     configuration,
     name,
@@ -208,6 +209,7 @@ Result _result(
     commitHash,
     isFlaky,
     previousOutcome,
+    timeMs,
     flaked: flaked,
   );
 }