Change the location of the output directory of generated tests to be inside output directory, but n…

Change the location of the output directory of generated tests to be inside output directory, but not relative to the dart root.

This is causing issues on dartium builders where we actually only run because there is a build/Release (and out/Release or xcodebuild/Release for mac/linux) on the bot that has been created inside src/dart.

Also, fix the http server in the testing script to serve files from the output directory - otherwise the tests will not be accessible if an alternative build directory is used.
Committed: https://code.google.com/p/dart/source/detail?r=17848

[kustermann, 2013-01-22 14:10:08]

LGTM with comments.

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart
File tools/test.dart (right):

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart#newcode130
tools/test.dart:130: TestingServerRunner.setPackageRootDir(firstConf);
The methods setBuildDir/setPackageRootDir are somehow strange since they don't
accept a any directory paths as arguments but rather a configuration.

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart#newcode131
tools/test.dart:131: for (var conf in configurations) {
Since there is some http-server stuff in test-runtime.dart, do we need to add
these lines there as well (or alternatively remove the http-server stuff in
test-runtime.dart) to make it consistent?

[Emily Fortuna, 2013-01-22 18:46:06]

This change will not work without modifying tests in the test directory as well
(several html tests and intl tests).

See tests/html/xhr_test.dart for an example of where this will be an issue (the
value of "url" will need to have multiple possible values depending on whether
it's in a Dart or Dartium checkout. Making the test writer have to deal with
this minutiae seems quite unfortunate.

[ahe, 2013-01-23 12:17:41]

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart
File tools/test.dart (right):

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart#newcode130
tools/test.dart:130: TestingServerRunner.setPackageRootDir(firstConf);
I think this slightly breaks passing both -mdebug,release to test.dart. Perhaps
we should have one server per configuration.

[ricow1, 2013-01-23 12:28:06]

On 2013/01/22 18:46:06, Emily Fortuna wrote:
> This change will not work without modifying tests in the test directory as
well
> (several html tests and intl tests).
> 
> See tests/html/xhr_test.dart for an example of where this will be an issue
(the
> value of "url" will need to have multiple possible values depending on whether
> it's in a Dart or Dartium checkout. Making the test writer have to deal with
> this minutiae seems quite unfortunate.
Why is that url pointing to:
  var url = "../../../../tests/html/xhr_cross_origin_data.txt";
instead of
  var url = "/tests/html/xhr_cross_origin_data.txt";
?

[Emily Fortuna, 2013-01-23 18:51:08]

On 2013/01/23 12:28:06, ricow1 wrote:
> On 2013/01/22 18:46:06, Emily Fortuna wrote:
> > This change will not work without modifying tests in the test directory as
> well
> > (several html tests and intl tests).
> > 
> > See tests/html/xhr_test.dart for an example of where this will be an issue
> (the
> > value of "url" will need to have multiple possible values depending on
whether
> > it's in a Dart or Dartium checkout. Making the test writer have to deal with
> > this minutiae seems quite unfortunate.
> Why is that url pointing to:
>   var url = "../../../../tests/html/xhr_cross_origin_data.txt";
> instead of
>   var url = "/tests/html/xhr_cross_origin_data.txt";
> ?

The URL is relative to the location of the generated html test file that will be
making the request.

[Emily Fortuna, 2013-01-23 18:51:21]

On 2013/01/23 18:51:08, Emily Fortuna wrote:
> On 2013/01/23 12:28:06, ricow1 wrote:
> > On 2013/01/22 18:46:06, Emily Fortuna wrote:
> > > This change will not work without modifying tests in the test directory as
> > well
> > > (several html tests and intl tests).
> > > 
> > > See tests/html/xhr_test.dart for an example of where this will be an issue
> > (the
> > > value of "url" will need to have multiple possible values depending on
> whether
> > > it's in a Dart or Dartium checkout. Making the test writer have to deal
with
> > > this minutiae seems quite unfortunate.
> > Why is that url pointing to:
> >   var url = "../../../../tests/html/xhr_cross_origin_data.txt";
> > instead of
> >   var url = "/tests/html/xhr_cross_origin_data.txt";
> > ?
> 
> The URL is relative to the location of the generated html test file that will
be
> making the request.

(which is not ideal...)

[ricow1, 2013-01-29 11:58:35]

PTAL

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart
File tools/test.dart (right):

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart#newcode130
tools/test.dart:130: TestingServerRunner.setPackageRootDir(firstConf);
On 2013/01/23 12:17:41, ahe wrote:
> I think this slightly breaks passing both -mdebug,release to test.dart.
Perhaps
> we should have one server per configuration.
That is actually true already for the existing code - I will file a bug and fix
this in a follow up ( dartbug.com/8179 )

https://codereview.chromium.org/11896031/diff/2001/tools/test.dart#newcode131
tools/test.dart:131: for (var conf in configurations) {
On 2013/01/22 14:10:08, kustermann wrote:
> Since there is some http-server stuff in test-runtime.dart, do we need to add
> these lines there as well (or alternatively remove the http-server stuff in
> test-runtime.dart) to make it consistent?

Done.

[kustermann, 2013-01-29 12:29:30]

LGTM with comments.

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
File tools/testing/dart/http_server.dart (right):

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
tools/testing/dart/http_server.dart:75: print(_packageRootDir);
Why are these "print()"'s here?

In case we wanna make sure that we get the correct package root, you could use
our new debug.log -- DebugLogger.info("foobar")

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
tools/testing/dart/http_server.dart:93: if
(requestPath.toString().startsWith("tests/html/xhr")) print(requestPath);
long line -- finally I can complain about this as well ;-)

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/test_s...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/test_s...
tools/testing/dart/test_suite.dart:1130: print(generatedTestPath.toString());
same here.

[Emily Fortuna, 2013-01-29 18:08:04]

lgtm with Martin's comments.

[ricow1, 2013-01-30 14:38:42]

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
File tools/testing/dart/http_server.dart (right):

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
tools/testing/dart/http_server.dart:75: print(_packageRootDir);
On 2013/01/29 12:29:30, kustermann wrote:
> Why are these "print()"'s here?
> 
> In case we wanna make sure that we get the correct package root, you could use
> our new debug.log -- DebugLogger.info("foobar")
left over debug info, removed

https://codereview.chromium.org/11896031/diff/11001/tools/testing/dart/http_s...
tools/testing/dart/http_server.dart:93: if
(requestPath.toString().startsWith("tests/html/xhr")) print(requestPath);
On 2013/01/29 12:29:30, kustermann wrote:
> long line -- finally I can complain about this as well ;-)
Removed