[et] Fix concurrent modification exception (#52247)

We cannot modify the list of build targets as we're iterating over it. Instead of removing non-test/non-executable elements, instead we add executable test targets to a separate testTargets list and use that.

Noticed while working on: flutter/flutter#147071

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: cbracken

tools/engine_tool/lib/src/commands/test_command.dart

@@ -70,18 +70,18 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
       );
     }
 
+    final List<BuildTarget> testTargets = <BuildTarget>[];
     for (final BuildTarget target in selectedTargets) {
-      if (!target.testOnly || target.type != BuildTargetType.executable) {
-        // Remove any targets that aren't testOnly and aren't executable.
-        selectedTargets.remove(target);
+      if (_isTestExecutable(target)) {
+        testTargets.add(target);
       }
       if (target.executable == null) {
         environment.logger.fatal(
             '$target is an executable but is missing the executable path');
       }
     }
     // Chop off the '//' prefix.
-    final List<String> buildTargets = selectedTargets
+    final List<String> buildTargets = testTargets
         .map<String>(
             (BuildTarget target) => target.label.substring('//'.length))
         .toList();
@@ -95,11 +95,16 @@ et test //flutter/fml:fml_benchmarks  # Run a single test target in `//flutter/f
     final WorkerPool workerPool =
         WorkerPool(environment, ProcessTaskProgressReporter(environment));
     final Set<ProcessTask> tasks = <ProcessTask>{};
-    for (final BuildTarget target in selectedTargets) {
+    for (final BuildTarget target in testTargets) {
       final List<String> commandLine = <String>[target.executable!.path];
       tasks.add(ProcessTask(
           target.label, environment, environment.engine.srcDir, commandLine));
     }
     return await workerPool.run(tasks) ? 0 : 1;
   }
+
+  /// Returns true if `target` is a testonly executable.
+  static bool _isTestExecutable(BuildTarget target) {
+    return target.testOnly && target.type == BuildTargetType.executable;
+  }
 }

tools/engine_tool/test/test_command_test.dart

@@ -67,4 +67,28 @@ void main() {
       testEnvironment.cleanup();
     }
   });
+
+  test('test command skips non-testonly executables', () async {
+    final TestEnvironment testEnvironment = TestEnvironment.withTestEngine(
+      cannedProcesses: cannedProcesses,
+    );
+    try {
+      final Environment env = testEnvironment.environment;
+      final ToolCommandRunner runner = ToolCommandRunner(
+        environment: env,
+        configs: configs,
+      );
+      final int result = await runner.run(<String>[
+        'test',
+        '//third_party/protobuf:protoc',
+      ]);
+      expect(result, equals(1));
+      expect(testEnvironment.processHistory.length, lessThan(3));
+      expect(testEnvironment.processHistory.where((ExecutedProcess process) {
+        return process.command[0].contains('protoc');
+      }), isEmpty);
+    } finally {
+      testEnvironment.cleanup();
+    }
+  });
 }