Simplify the enqueuing of tests from all test suites

tools/testing/dart/test_runner.dart

Index: tools/testing/dart/test_runner.dart
diff --git a/tools/testing/dart/test_runner.dart b/tools/testing/dart/test_runner.dart
index aa84ae9cd637787c501b69c64bf22ee0642571e5..0d5208a6f568880c93e2e7083207bf5dbac5ab53 100644
--- a/tools/testing/dart/test_runner.dart
+++ b/tools/testing/dart/test_runner.dart
@@ -1426,15 +1426,14 @@ class BatchRunnerProcess {
  */
 class ProcessQueue {
   int _numProcesses = 0;
-  int _activeTestListers = 0;
   int _maxProcesses;
+  bool _allTestsWereEnqueued = false;
 
   /** The number of tests we allow to actually fail before we stop retrying. */
   int _MAX_FAILED_NO_RETRY = 4;
   bool _verbose;
   bool _listTests;
   Function _allDone;
-  EnqueueMoreWork _enqueueMoreWork;
   Queue<TestCase> _tests;
   ProgressIndicator _progress;
 
@@ -1468,7 +1467,7 @@ class ProcessQueue {
                String progress,
                Date startTime,
                bool printTiming,
-               this._enqueueMoreWork,
+               testSuites,
                this._allDone,
                [bool verbose = false,
                 bool listTests = false])
@@ -1480,20 +1479,7 @@ class ProcessQueue {
                                                    printTiming),
         _batchProcesses = new Map<String, List<BatchRunnerProcess>>(),
         _testCache = new Map<String, List<TestInformation>>() {
-    _checkDone();
-  }
-
-  /**
-   * Registers a TestSuite so that all of its tests will be run.
-   */
-  void addTestSuite(TestSuite testSuite) {
-    _activeTestListers++;
-    testSuite.forEachTest(_runTest, _testCache, _testListerDone);
-  }
-
-  void _testListerDone() {
-    _activeTestListers--;
-    _checkDone();
+    _runTests(testSuites);
   }
 
   /**
@@ -1509,19 +1495,27 @@ class ProcessQueue {
     }
   }
 
-  void _checkDone() {
-    // When there are no more active test listers ask for more work
-    // from process queue users.
-    if (_activeTestListers == 0) {
-     _enqueueMoreWork(this);
-    }
-    // If there is still no work, we are done.
-    if (_activeTestListers == 0) {
-      _progress.allTestsKnown();
-      if (_tests.isEmpty && _numProcesses == 0) {
-        _terminateBatchRunners().then((_) => _cleanupAndMarkDone());
+  void _runTests(List<TestSuite> testSuites) {
+    var numberOfTestSuitesFinished = 0;
+
+    void testSuiteFinished() {

[ricow1, 2013/01/15 12:38:44]

how about making this:
void _runTests(List<TestSuite> testSuites) {
  Iterator<TestSuite> iterator = testSuites.iterator();
  void enqueueNextSuite() {
    if (!iterator.hasNext()) {
      _allTestsWereEnqueued = true;
      _progress.allTestsKnown();
  } else {
    iterator.next().forEachTest(_runTest,
				_testCache,
				enqueueNextSuite);
  }
  enqueueNextSuite();
}

[kustermann, 2013/01/16 09:09:09]

Done.

+      numberOfTestSuitesFinished++;
+      if (numberOfTestSuitesFinished == testSuites.length) {
+        _allTestsWereEnqueued = true;
+        _progress.allTestsKnown();
+      } else {
+        testSuites[numberOfTestSuitesFinished].forEachTest(_runTest,
+                                                           _testCache,
+                                                           testSuiteFinished);
       }
     }
+    // FIXME: For some reason we cannot call this method on all test suites

[ricow1, 2013/01/15 12:38:44]

could we file a bug for this

[kustermann, 2013/01/16 09:09:09]

Done.

+    // in parallel.
+    // If we do, not all tests get enqueued (if --arch=all was specified,
+    // we don't get twice the number of tests [tested on -rvm -cnone])
+    testSuites[0].forEachTest(_runTest,
+                              _testCache,
+                              testSuiteFinished);
   }
 
   /**
@@ -1689,7 +1683,9 @@ class ProcessQueue {
   }
 
   void _tryRunTest() {
-    _checkDone();
+    if (_allTestsWereEnqueued && _tests.isEmpty && _numProcesses == 0) {
+      _terminateBatchRunners().then((_) => _cleanupAndMarkDone());
+    }
     if (_numProcesses < _maxProcesses && !_tests.isEmpty) {
       TestCase test = _tests.removeFirst();
       if (_listTests) {