[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 <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>

Author: rmacnak-google

pkg/test_runner/lib/src/test_file.dart

@@ -40,9 +40,6 @@ List<String> _parseStringOption(String filePath, String contents, String name,
     _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 @@ abstract class _TestFileBase {
   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 @@ class TestFile extends _TestFileBase {
   }
 
   /// 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 @@ class TestFile extends _TestFileBase {
         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

@@ -322,7 +322,7 @@ class VMTestSuite extends TestSuite {
     }
 
     // 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 @@ class FfiTestSuite extends TestSuite {
     }
 
     // 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

@@ -783,13 +783,10 @@ void testShardHash() {
   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) {