Add temporary directory for dartc compilation of tests.

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 8808912286dafdce739ef42d2890d6888d1914d4..73f6caaaa42c5eb89708a2e818523525a47234bc 100644
--- a/tools/testing/dart/test_runner.dart
+++ b/tools/testing/dart/test_runner.dart
@@ -406,9 +406,11 @@ class ProcessQueue {
   int _maxProcesses;
   bool _verbose;
   bool _listTests;
+  bool _keepGeneratedTests;
   Function _enqueueMoreWork;
   Queue<TestCase> _tests;
   ProgressIndicator _progress;
+  String _temporaryDirectory;
   // For dartc batch processing we keep a list of batch processes.
   List<DartcBatchRunnerProcess> _batchProcesses;
   // Cache information about test cases per test suite. For multiple
@@ -422,7 +424,8 @@ class ProcessQueue {
                bool printTiming,
                Function this._enqueueMoreWork,
                [bool verbose = false,

[Mads Ager (chromium), 2012/01/18 06:57:17]

We should go back to using this.verbose, this.listTests, and
this.keepGeneratedTests here instead of the initilizers below. This is left-over
from a temporary change where that syntax was not allowed in named optional
arguments.

[Bill Hesse, 2012/01/18 11:52:38]

Done.

-                bool listTests = false])
+                bool listTests = false,
+                bool keepGeneratedTests = false])
       : _tests = new Queue<TestCase>(),
         _progress = new ProgressIndicator.fromName(progress,
                                                    startTime,
@@ -430,13 +433,20 @@ class ProcessQueue {
         _batchProcesses = new List<DartcBatchRunnerProcess>(),
         _testCache = new Map<String, List<TestInformation>>(),
         _verbose = verbose,
-        _listTests = listTests {
+        _listTests = listTests,
+        _keepGeneratedTests = keepGeneratedTests {
+    if (new Platform().operatingSystem() != 'windows') {

[Mads Ager (chromium), 2012/01/18 06:57:17]

Can we filter out better when we create the temporary directory? For instance do
it in addTestSuite if we are adding a test suite that needs a temporary
directory? Then that would be the place to add an assert too that we are not
running on Windows?

[Bill Hesse, 2012/01/18 11:52:38]

It is the components we run, not the test suites, that determine if we need the
temporary directory.  Almost all the test suites (the standard suites) need the
temp directory for component chromium.  For component dartc only some of the
suites need it.  Information about all the configurations is not passed in to
the processQueue constructor, and we would need to know if any configuration
used a relevant component.  Creating the directory is really cheap.
On 2012/01/18 06:57:17, Mads Sig Ager wrote:

+      var tempDir = new Directory('');
+      tempDir.createTempSync();
+      _temporaryDirectory = tempDir.path;
+    }
     if (!_enqueueMoreWork(this)) _progress.allDone();
   }
 
   void addTestSuite(TestSuite testSuite) {
     _activeTestListers++;
-    testSuite.forEachTest(_runTest, _testCache, _testListerDone);
+    testSuite.forEachTest(_runTest, _testCache, _temporaryDirectory,
+                          _testListerDone);
   }
 
   void _testListerDone() {
@@ -451,7 +461,22 @@ class ProcessQueue {
       _progress.allTestsKnown();
       if (_tests.isEmpty() && _numProcesses == 0) {
         _terminateDartcBatchRunners();
-        _progress.allDone();
+        if (_keepGeneratedTests ||
+            new Platform().operatingSystem() == 'windows') {
+          _progress.allDone();
+        } else if (!_temporaryDirectory.startsWith('/tmp/') ||
+                   _temporaryDirectory.contains('/../')) {
+          // Let's be extra careful, since rm -rf is so dangerous.
+          print('Temporary directory $_temporaryDirectory unsafe to delete!');
+          _progress.allDone();
+        } else {
+          var deletion =
+              new Process.start('/bin/rm', ['-rf', _temporaryDirectory]);

[Mads Ager (chromium), 2012/01/18 06:57:17]

Lets add a TODO that this should be replaced by
_temporaryDirectory.delete(recursive: true) once that has been implemented? Then
Windows would be supported as well. 

[Bill Hesse, 2012/01/18 11:52:38]

Done.

+          deletion.startHandler = (){
+            _progress.allDone();
+          };
+          deletion.close();  // Detach process - don't wait for it to finish.

[Mads Ager (chromium), 2012/01/18 06:57:17]

I'm not sure we should do that. It will allow the script to return faster but it
will make it harder to debug if something is wrong. Maybe we should instead add
an errorHandler to the process and print out any errors encountered?

[Bill Hesse, 2012/01/18 11:52:38]

The deletion.close() is redundant, since _progress.allDone() calls
system.exit().  We would have to add waiting for all possible exits of the
deletion command to _progress.allDone().  We don't want to wait for this to
complete, it causes no error if it does not complete correctly, except filling
the temp directory, and shouldn't need debugging.  So the line is removed, but
the behavior will still be the same.

On 2012/01/18 06:57:17, Mads Sig Ager wrote:

+        }
       }
     }
   }