commit 3daa47b54e5f39241c44858e0d66ce51723040bf
author Ryan Macnak <rmacnak@google.com> Wed Jul 12 17:56:59 2023 +0000
committer Commit Queue <dart-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jul 12 17:56:59 2023 +0000
tree c5ee131a45feb01958a9fd8a376797eaf42c57d0
parent 89e24c1aefaf2f410ddb5eb6fbfa826f91d84f4d

[test_runner] Don't assign all vm/cc tests to the first shard.

Before:
./tools/test.py --shards=10 --shard=1 --list | wc -l  # 3664
./tools/test.py --shards=10 --shard=2 --list | wc -l  # 1047
./tools/test.py --shards=10 --shard=3 --list | wc -l  # 1146

After:
./tools/test.py --shards=10 --shard=1 --list | wc -l  # 1408
./tools/test.py --shards=10 --shard=2 --list | wc -l  # 1306
./tools/test.py --shards=10 --shard=3 --list | wc -l  # 1381

Change-Id: I2107779e79d85976c04db7c01c11581a8d9895b0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313280
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>

pkg/test_runner/lib/src/test_file.dart

diff --git a/pkg/test_runner/lib/src/test_file.dart b/pkg/test_runner/lib/src/test_file.dart
index 31bc023..4518a34 100644
--- a/pkg/test_runner/lib/src/test_file.dart
+++ b/pkg/test_runner/lib/src/test_file.dart
@@ -40,9 +40,6 @@
     _parseOption<String>(filePath, contents, name, (string) => string,
         allowMultiple: allowMultiple);
 
-// Fake path used as sentinel for test files that don't have a path.
-final _fakePath = Path('/fake');
-
 abstract class _TestFileBase {
   /// The test suite directory containing this test.
   final Path? _suiteDirectory;
@@ -93,9 +90,9 @@
   int get shardHash {
     // The VM C++ unit tests have a special fake TestFile with no suite
     // directory or path. Don't crash in that case.
-    // TODO(rnystrom): Is there a cleaner solution? Should we use the C++ file
-    // as the path for the TestFile?
-    if (originPath == _fakePath) return 0;
+    if (_suiteDirectory == null) {
+      return path.toString().hashCode;
+    }
 
     return originPath.relativeTo(_suiteDirectory!).toString().hashCode;
   }
@@ -318,7 +315,7 @@
   }
 
   /// A special fake test file for representing a VM unit test written in C++.
-  TestFile.vmUnitTest(
+  TestFile.vmUnitTest(String name,
       {required this.hasCompileError,
       required this.hasRuntimeError,
       required this.hasCrash})
@@ -338,7 +335,7 @@
         otherResources = [],
         experiments = [],
         isVmIntermediateLanguageTest = false,
-        super(null, _fakePath, []);
+        super(null, Path("/fake/vm/cc/$name"), []);
 
   TestFile._(Path suiteDirectory, Path path, List<StaticError> expectedErrors,
       {this.packages,

pkg/test_runner/lib/src/test_suite.dart

diff --git a/pkg/test_runner/lib/src/test_suite.dart b/pkg/test_runner/lib/src/test_suite.dart
index 314387f..1060d6b 100644
--- a/pkg/test_runner/lib/src/test_suite.dart
+++ b/pkg/test_runner/lib/src/test_suite.dart
@@ -322,7 +322,7 @@
     }
 
     // Update the new workflow based expectations to include [testExpectation].
-    var testFile = TestFile.vmUnitTest(
+    var testFile = TestFile.vmUnitTest(test.name,
         hasCompileError: testExpectation == Expectation.compileTimeError,
         hasRuntimeError: testExpectation == Expectation.runtimeError,
         hasCrash: testExpectation == Expectation.crash);
@@ -461,7 +461,7 @@
     }
 
     // Update the new workflow based expectations to include [testExpectation].
-    final testFile = TestFile.vmUnitTest(
+    final testFile = TestFile.vmUnitTest(test.name,
         hasCompileError: testExpectation == Expectation.compileTimeError,
         hasRuntimeError: testExpectation == Expectation.runtimeError,
         hasCrash: testExpectation == Expectation.crash);

pkg/test_runner/test/test_file_test.dart

diff --git a/pkg/test_runner/test/test_file_test.dart b/pkg/test_runner/test/test_file_test.dart
index ef6ef25..d7e0410 100644
--- a/pkg/test_runner/test/test_file_test.dart
+++ b/pkg/test_runner/test/test_file_test.dart
@@ -783,13 +783,10 @@
   var testFile = parseTestFile("", path: "a_test.dart");
   Expect.type<int>(testFile.shardHash);
 
-  // VM test files are hard-coded to return hash zero because they don't have a
-  // path to base the hash on.
-  Expect.equals(
-      0,
-      TestFile.vmUnitTest(
-              hasCompileError: false, hasCrash: false, hasRuntimeError: false)
-          .shardHash);
+  // VM test files are based on a fake path.
+  testFile = TestFile.vmUnitTest("ExampleTestName",
+      hasCompileError: false, hasCrash: false, hasRuntimeError: false);
+  Expect.type<int>(testFile.shardHash);
 }
 
 void expectParseErrorExpectations(String source, List<StaticError> errors) {