[infra] pkg/smith: custom test runner steps are test steps

Change-Id: I6517475d1db1cad707fb0d096e2689ea265f6a9b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133522
Commit-Queue: Karl Klose <[email protected]>
Reviewed-by: Bob Nystrom <[email protected]>

Author: karlklose

pkg/smith/lib/builder.dart

@@ -9,14 +9,13 @@ import 'configuration.dart';
 ///
 /// Each step on a builder runs a script the with provided arguments. If the
 /// script is 'tools/test.py' (which is the default if no script is given in
-/// the test matrix), the step is called a 'test step'. Test steps must include
-/// the '--named_configuration' (for short '-n') option to select the named
-/// [Configuration] to test.
+/// the test matrix), or `testRunner == true`, the step is called a 'test
+/// step'. Test steps must include the '--named_configuration' (for short
+/// '-n') option to select the named [Configuration] to test.
 ///
-/// Test steps and steps with `isTestRunner == true` are expected to produce
-/// test results that are collected during the run of the builder and checked
-/// against the expected results to determine the success or failure of the
-/// build.
+/// Test steps are expected to produce test results that are collected during
+/// the run of the builder and checked against the expected results to determine
+/// the success or failure of the build.
 class Step {
   final String name;
   final String script;
@@ -33,7 +32,7 @@ class Step {
 
   static const testScriptName = "tools/test.py";
 
-  bool get isTestStep => script == testScriptName;
+  bool get isTestStep => script == testScriptName || isTestRunner;
 
   /// Create a [Step] from the 'step template' [map], values for supported
   /// variables [configuration], and the list of supported named configurations.
@@ -44,7 +43,8 @@ class Step {
         .toList();
     var testedConfigurations = <Configuration>[];
     var script = map["script"] as String ?? testScriptName;
-    if (script == testScriptName) {
+    var isTestRunner = map["testRunner"] as bool ?? false;
+    if (script == testScriptName || isTestRunner) {
       // TODO(karlklose): replace with argument parser that can handle all
       // arguments to test.py.
       for (var argument in arguments) {
@@ -80,7 +80,7 @@ class Step {
         <String, String>{...?map["environment"]},
         map["fileset"] as String,
         map["shards"] as int,
-        map["testRunner"] as bool ?? false,
+        isTestRunner,
         testedConfigurations.isEmpty ? null : testedConfigurations.single);
   }
 }

pkg/smith/test/builder_test.dart

@@ -31,6 +31,16 @@ void main() {
       expect(step.isTestStep, isTrue);
       expect(step.testedConfiguration.name, "foo-x64-none-debug-d8-linux");
     });
+    test("custom test runner step", () {
+      var step = Step.parse({
+        "name": "foo",
+        "script": "foo.py",
+        "testRunner": true,
+        "arguments": ["-nfoo-x64-none-debug-d8-linux"]
+      }, {}, configurations);
+      expect(step.isTestStep, isTrue);
+      expect(step.testedConfiguration.name, "foo-x64-none-debug-d8-linux");
+    });
     test("implicit test step", () {
       var step = Step.parse({
         "name": "foo",