Enable use of #import stmts containing relative paths in Dart multitests

Enable use of #import stmts containing relative paths in Dart multitests

This will alow tests like Prefix22NegativeTest an Prefix23Negative
test to be re-written as multitests.

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

[zundel, 2012-03-01 06:06:57]

I think this is the last barrier keeping us from porting the
remaining 'NegativeTest' cases.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
File tools/testing/dart/multitest.dart (right):

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:113: void _copyFile(File source, File dest) {
is there already a canned way to do this somewhere else?

[Bill Hesse, 2012-03-01 15:01:52]

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
File tools/testing/dart/multitest.dart (right):

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:113: void _copyFile(File source, File dest) {
On 2012/03/01 06:06:58, zundel wrote:
> is there already a canned way to do this somewhere else?  
Not that I know of, except by calling the shell.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:115: RandomAccessFile handle =
dest.openSync(FileMode.WRITE);
This will not work except for files in the same directory.  Subdirectories and
'../sibling_dir' will not work unless every directory on the path exists. 
Either check that the target directory exists, and print error if it doesn't, or
don't allow non-trivial paths, or create the necessary directories.  We have
client tests (samples) with relative paths, that work because the relative path
points to the same place from source and dest.  Also, make sure we are not
overwriting sensitive stuff, I suppose.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:125: String pathSep = new
Platform().pathSeparator();
Rather than use pathSeparator, I would use "/" everywhere.  If there is a chance
that topLibrary comes in using "\", then replace all the "\" with "/" there. 
This makes things easier, I think.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:142: if (relativeImportRegExp1.hasMatch(line))
{
I think you can combine the two regexp to 
 '........[(]["\'](?!(dart:|/))([^"\']*)["\']',
since the (?!...) is non-capturing (?).  Then m = .firstMatch
if (m != null) ....

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:170: Set<String> importsToCopy = new
Set<String>();
This initializer is overwritten - can be omitted.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:173: String directory =
CreateMultitestDirectory(outputDir,  testDir);
extra space before testDir?

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:182: File source = new
File('$sourceDirectory$pathSep$import');
Again, no need for $pathSep.  "/" works on all platforms.

[zundel, 2012-03-01 18:06:01]

Thanks for the feedback.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
File tools/testing/dart/multitest.dart (right):

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:113: void _copyFile(File source, File dest) {
On 2012/03/01 15:01:52, Bill Hesse wrote:
> On 2012/03/01 06:06:58, zundel wrote:
> > is there already a canned way to do this somewhere else?  
> Not that I know of, except by calling the shell.

Moved to TestUtils class.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:115: RandomAccessFile handle =
dest.openSync(FileMode.WRITE);
On 2012/03/01 15:01:52, Bill Hesse wrote:
> This will not work except for files in the same directory.  Subdirectories and
> '../sibling_dir' will not work unless every directory on the path exists. 
> Either check that the target directory exists, and print error if it doesn't,
or
> don't allow non-trivial paths, or create the necessary directories.  We have
> client tests (samples) with relative paths, that work because the relative
path
> points to the same place from source and dest.  Also, make sure we are not
> overwriting sensitive stuff, I suppose.

the generated test files don't go away between tests,I assume they are fair game
to be clobbered, so I'm not testing for file existence.  I did add a filter to
disallow any '..' in the relative paths. If someone really needs that they can
just take out the check.

Created the dirs TestUtils.mkdirRecursive()

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:125: String pathSep = new
Platform().pathSeparator();
On 2012/03/01 15:01:52, Bill Hesse wrote:
> Rather than use pathSeparator, I would use "/" everywhere.  If there is a
chance
> that topLibrary comes in using "\", then replace all the "\" with "/" there. 
> This makes things easier, I think.

Done.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:142: if (relativeImportRegExp1.hasMatch(line))
{
On 2012/03/01 15:01:52, Bill Hesse wrote:
> I think you can combine the two regexp to 
>  '........[(]["\'](?!(dart:|/))([^"\']*)["\']',
> since the (?!...) is non-capturing (?).  Then m = .firstMatch
> if (m != null) ....


i've never used the non-capturing before, but it does seem to take up a group
position:

RegExp relativeImportRegExp =
      const RegExp('^#(import|source)[(]["\'](?!(dart:|/))([^"\']*)["\']');

 Match match = relativeImportRegExp.firstMatch(line); 
        if (match != null) {
          print ("group count is ${match.groupCount()}");
          print ("${match.group(1)}");
          print ("${match.group(2)}");
          print ("${match.group(3)}");

group count is 3
import

dummy/library11.dart

Is that what you would have expected (group 2 is empty string)?

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:170: Set<String> importsToCopy = new
Set<String>();
On 2012/03/01 15:01:52, Bill Hesse wrote:
> This initializer is overwritten - can be omitted.

Done.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:173: String directory =
CreateMultitestDirectory(outputDir,  testDir);
On 2012/03/01 15:01:52, Bill Hesse wrote:
> extra space before testDir?

Done.

https://chromiumcodereview.appspot.com/9565001/diff/1/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:182: File source = new
File('$sourceDirectory$pathSep$import');
On 2012/03/01 15:01:52, Bill Hesse wrote:
> Again, no need for $pathSep.  "/" works on all platforms.

Done.

[Bill Hesse, 2012-03-02 08:27:15]

LGTM.
I didn't mean non-capturing as in not creating a group, but as in not advancing
through the string.  I think you can make the change, and use group 3 as the
result.