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

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

Commit r4858 broke browser tests because of recursive directory handling.

I've removed the reference to the new recursive directory creation routine
from the 'createOutputDirectory()' method.  I don't understand the current 
directory structure - it might be a bug, but my attempt to fix it
leaves browser tests in a broken state.

Committed: https://code.google.com/p/dart/source/detail?r=4954

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

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:676: // removing it I had lots of problems
with browser tests.
in my previous patch, I replaced all of the below with a reference to
TestUtils.mkdirRecursive(), but didn't capture
the updated 'tempDir' variable.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1051: return tempDir;
returning the directory is new.

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

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:674: //
'out/Debug_ia32/Debug_ia32/generated-tests/...'
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.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:676: // removing it I had lots of problems
with browser tests.
On 2012/03/02 18:25:03, zundel wrote:
> in my previous patch, I replaced all of the below with a reference to
> TestUtils.mkdirRecursive(), but didn't capture
> the updated 'tempDir' variable.

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

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1035: static Directory mkdirRecursive(String
base, String relativePath) {
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.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1038: "This method expects ${base} to already
exist");
"This method" is not really informative to a user getting the message.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1045: tempDir = new Directory(base);
tempDir is never declared.  How does this compile without errors?

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1050: Expect.isTrue(tempDir.existsSync(),
"Failed to create ${tempDir.path}");
I would move the check into the loop.

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

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:674: //
'out/Debug_ia32/Debug_ia32/generated-tests/...'
On 2012/03/05 09:41:02, Bill Hesse wrote:
> 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.

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.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:676: // removing it I had lots of problems
with browser tests.
On 2012/03/05 09:41:02, Bill Hesse wrote:
> On 2012/03/02 18:25:03, zundel wrote:
> > in my previous patch, I replaced all of the below with a reference to
> > TestUtils.mkdirRecursive(), but didn't capture
> > the updated 'tempDir' variable.
> 
> So now that mkdirRecursive returns the resulting Directory, can you use it
here?

Yes (updated patch)

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1038: "This method expects ${base} to already
exist");
On 2012/03/05 09:41:02, Bill Hesse wrote:
> "This method" is not really informative to a user getting the message.

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

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1045: tempDir = new Directory(base);
On 2012/03/05 09:41:02, Bill Hesse wrote:
> tempDir is never declared.  How does this compile without errors?

!!! Fixed.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1050: Expect.isTrue(tempDir.existsSync(),
"Failed to create ${tempDir.path}");
On 2012/03/05 09:41:02, Bill Hesse wrote:
> I would move the check into the loop.

Done.

https://chromiumcodereview.appspot.com/9583016/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:1051: return tempDir;
On 2012/03/02 18:25:03, zundel wrote:
> returning the directory is new.

Done.

[Bill Hesse, 2012-03-05 14:32:26]

LGTM.