Re-apply "Enable use of #import stmts containing relative paths in Dart multitests"

tools/testing/dart/test_suite.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /**
  * Classes and methods for enumerating and preparing tests.
  *
  * This library includes:
  *
  * - Creating tests by listing all the Dart files in certain directories,
@@ skipping 652 matching lines @@
       }
       buildPath.removeRange(0, 1);
       if (buildPath.last() == '') buildPath.removeLast();
       buildPath.addAll(generatedTestPath);
       generatedTestPath = buildPath;
       tempDir = new Directory(tempDirPath);
       if (!tempDir.existsSync()) {
         tempDir.createSync();
       }
     }
+    // TODO(zundel): this actually creates and returns a path like 
+    // 'out/Debug_ia32/Debug_ia32/generated-tests/...'

[Bill Hesse, 2012/03/05 09:41:02]

The level of the directory that a generated test is copied to is important if it
contains relative paths to test framework files in the checkout.  I didn't see
these sorts of paths, but that is why the paths can be fragile.

[zundel, 2012/03/05 11:34:36]

Run this command in an unaltered version of test.dart to reproduce:

rm out/Release_ia32
./tools/build.py -m release
./tools/test.py -m release -c frogium corelib

You'll see all thefrogium tests are put under out/Release_ia32/Release_ia32/...

which seemed incorrect to me. 

+    // I think the extra mode/architecture label redundant, but after
+    // removing it I had lots of problems with browser tests.

[zundel, 2012/03/02 18:25:03]

in my previous patch, I replaced all of the below with a reference to
TestUtils.mkdirRecursive(), but didn't capture
the updated 'tempDir' variable.

[Bill Hesse, 2012/03/05 09:41:02]

So now that mkdirRecursive returns the resulting Directory, can you use it here?

[zundel, 2012/03/05 11:34:36]

Yes (updated patch)

     tempDirPath = new File(tempDirPath).fullPathSync().replaceAll('\\', '/');
 
     for (String subdirectory in generatedTestPath) {
       tempDirPath = '$tempDirPath/$subdirectory';
       tempDir = new Directory(tempDirPath);
       if (!tempDir.existsSync()) {
         tempDir.createSync();
       }
     }
     return tempDir;
@@ skipping 333 matching lines @@
          '$dartDir/third_party/hamcrest/v1_3/hamcrest-generator-1.3.0RC2.jar',
          '$dartDir/third_party/hamcrest/v1_3/hamcrest-integration-1.3.0RC2.jar',
          '$dartDir/third_party/hamcrest/v1_3/hamcrest-library-1.3.0RC2.jar',
          '$dartDir/third_party/junit/v4_8_2/junit.jar'],
         ':');  // Path separator.
   }
 }
 
 
 class TestUtils {
+  
+  /** 
+   * Creates a directory using a [relativePath] to an existing 
+   * [base] directory if that [relativePath] does not already exist.
+   */
+  static Directory mkdirRecursive(String base, String relativePath) {

[Bill Hesse, 2012/03/05 09:41:02]

I don't see a way in which you will ever get relativePaths with a
platform-dependent path separator.  The only APIs that return platform-dependent
paths return absolute paths, and those will be processed to make relative paths,
including fixing the path separator.  Putting unneeded generality here just
encourages us to allow platform-dependent paths into the system, rather than
fixing them as soon as we get them.

+    Directory baseDir = new Directory(base);
+    Expect.isTrue(baseDir.existsSync(),
+      "This method expects ${base} to already exist"); 

[Bill Hesse, 2012/03/05 09:41:02]

"This method" is not really informative to a user getting the message.

[zundel, 2012/03/05 11:34:36]

Are you saying to add the method name in?  I just removed "This Method" because
you'll see it in the stack trace.

+    String pathSep = new Platform().pathSeparator();
+    if (pathSep != '/') {
+      relativePath = relativePath.replaceAll(pathSep, '/');
+    }   
+    for (String dir in relativePath.split('/')) {
+      base = "$base/$dir";
+      tempDir = new Directory(base);

[Bill Hesse, 2012/03/05 09:41:02]

tempDir is never declared.  How does this compile without errors?

[zundel, 2012/03/05 11:34:36]

!!! Fixed.

+      if (!tempDir.existsSync()) {
+          tempDir.createSync();
+      }
+    }
+    Expect.isTrue(tempDir.existsSync(), "Failed to create ${tempDir.path}");

[Bill Hesse, 2012/03/05 09:41:02]

I would move the check into the loop.

[zundel, 2012/03/05 11:34:36]

Done.

+    return tempDir;

[zundel, 2012/03/02 18:25:03]

returning the directory is new.

[zundel, 2012/03/05 11:34:36]

Done.

+  }
+
+  /**
+   * Copy a [source] file to a new place.
+   * Assumes that the directory for [dest] already exists.
+   */
+  static void copyFile(File source, File dest) {
+    List contents = source.readAsBytesSync();
+    RandomAccessFile handle = dest.openSync(FileMode.WRITE);
+    handle.writeListSync(contents, 0, contents.length);
+    handle.closeSync();
+  }
+  
   static String executableSuffix(String component) {
     if (new Platform().operatingSystem() == 'windows') {
       if (component != 'frogium'
           && component != 'legium'
           && component != 'webdriver') {
         return '.exe';
       } else {
         return '.bat';
       }
     }
@@ skipping 137 matching lines @@
  * $noCrash tests are expected to be flaky but not crash
  * $pass tests are expected to pass
  * $failOk tests are expected to fail that we won't fix
  * $fail tests are expected to fail that we should fix
  * $crash tests are expected to crash that we should fix
  * $timeout tests are allowed to timeout
 """;
     print(report);
    }
 }