Run dart2js directly from test.dart.

Run dart2js directly from test.dart.

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

[kasperl, 2012-04-25 12:23:47]

LGTM.

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

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:446:
'd8${TestUtils.executableSuffix("d8")}';
Maybe TestUtils.executableSuffix("d8") into a local variable to make this easier
to grok?

[ngeoffray, 2012-04-25 12:24:54]

LGTM!

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
File dart/lib/compiler/implementation/dart2js.dart (right):

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/dart2js.dart:52: fail('No files to compile.');
No files -> No file

[Bill Hesse, 2012-04-25 12:46:26]

LGTM.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
File dart/lib/compiler/implementation/dart2js.dart (right):

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/dart2js.dart:43: } else if (argument ==
'--allow-mock-compilation') {
constant on RHS of comparison disagrees with above lines.

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

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:394: for (var dummy in
optionsFromFile["vmOptions"]) {
This count will now be wrong for dart2js tests, since they eliminate the
duplicate run for multiple vmoptions.  Please file a bug if it is not fixed.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:445: var d8 =
'${TestUtils.buildDir(configuration)}/'
I'd like to see the constructor of the d8 path in a helper function, that also
checks its existence.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:722: testUniqueName += '-$optionsName';
Thanks.  I wrote this, and it was ugly seeing the trailing - all the time.

[ahe, 2012-04-25 13:09:52]

Hi Bill, Kasper, and Nicolas,

Thank you for your comments. I'll address them in a follow-up CL.

I had to make a minor tweak to browser test compile-command, see patch set 2.

Cheers,
Peter

[ahe, 2012-04-25 13:49:41]

Comments addressed in: https://chromiumcodereview.appspot.com/10228003

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
File dart/lib/compiler/implementation/dart2js.dart (right):

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/dart2js.dart:43: } else if (argument ==
'--allow-mock-compilation') {
On 2012/04/25 12:46:26, Bill Hesse wrote:
> constant on RHS of comparison disagrees with above lines.

Done.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/lib/compiler/impl...
dart/lib/compiler/implementation/dart2js.dart:52: fail('No files to compile.');
On 2012/04/25 12:24:54, ngeoffray wrote:
> No files -> No file

Done.

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

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:394: for (var dummy in
optionsFromFile["vmOptions"]) {
On 2012/04/25 12:46:26, Bill Hesse wrote:
> This count will now be wrong for dart2js tests, since they eliminate the
> duplicate run for multiple vmoptions.  Please file a bug if it is not fixed.

Done.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:445: var d8 =
'${TestUtils.buildDir(configuration)}/'
On 2012/04/25 12:46:26, Bill Hesse wrote:
> I'd like to see the constructor of the d8 path in a helper function, that also
> checks its existence. 

Done.

https://chromiumcodereview.appspot.com/10151014/diff/1/dart/tools/testing/dar...
dart/tools/testing/dart/test_suite.dart:446:
'd8${TestUtils.executableSuffix("d8")}';
On 2012/04/25 12:23:47, kasperl wrote:
> Maybe TestUtils.executableSuffix("d8") into a local variable to make this
easier
> to grok?

Done.