Added '--package-root' to http_server.dart.

Added '--package-root' to http_server.dart.

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

[ricow1, 2013-02-01 10:19:11]

LGTM

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1601: print('$i.
${TestUtils.dartTestExecutable.toNativePath()} '
The fact that we do the exact same thing two places makes me wonder if we should
extract this into a utility funciton?

[kustermann, 2013-02-01 10:22:17]

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1601: print('$i.
${TestUtils.dartTestExecutable.toNativePath()} '
On 2013/02/01 10:19:12, ricow1 wrote:
> The fact that we do the exact same thing two places makes me wonder if we
should
> extract this into a utility funciton?

That is true. It actually belongs IMHO to http_server.dart. But for now, I'd
like to land this simple fix.

[ahe, 2013-02-01 10:28:59]

LGTM.

I think we should add a method to TestingServerRunner which prints the
information about how to start it. I'd prefer to turn all the static methods
into instance methods and have a TestingServerRunner instance per configuration.

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/http_se...
File tools/testing/dart/http_server.dart (right):

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/http_se...
tools/testing/dart/http_server.dart:24: parser.addOption('mode', abbr: 'm',
help: 'Testing mode.',
Unused option.

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/http_se...
tools/testing/dart/http_server.dart:27: defaultsTo: 'ia32');
Unused option.

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_pr...
File tools/testing/dart/test_progress.dart (right):

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_pr...
tools/testing/dart/test_progress.dart:209: '-a ${test.configuration["arch"]} '
-a and -m are unused.

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
File tools/testing/dart/test_runner.dart (right):

https://codereview.chromium.org/12087130/diff/4001/tools/testing/dart/test_ru...
tools/testing/dart/test_runner.dart:1604: '-a ${test.configuration["arch"]} '
-m and -a unused.

[kustermann, 2013-02-01 10:45:03]

Agreed, because it is currently broken anyway for "--mode=debug,release", I
guess! If we make instances of TestingServerRunner, then we can pass in package
root and build directory as arguments and can remove the mode/architecture
parameters.

On 2013/02/01 10:28:59, ahe wrote:
> LGTM.
> 
> I think we should add a method to TestingServerRunner which prints the
> information about how to start it. I'd prefer to turn all the static methods
> into instance methods and have a TestingServerRunner instance per
configuration.

[Emily Fortuna, 2013-02-01 17:41:24]

lgtm once you take out -m and -a options since they're no longer necessary.

[Emily Fortuna, 2013-02-01 17:43:11]

On 2013/02/01 10:45:03, kustermann wrote:
> Agreed, because it is currently broken anyway for "--mode=debug,release", I
> guess! 

You can only test one configuration at a time, so for repro-ing tests locally,
you'd have to select the particular mode (debug or release) that you're
interested in. 




> 
> On 2013/02/01 10:28:59, ahe wrote:
> > LGTM.
> > 
> > I think we should add a method to TestingServerRunner which prints the
> > information about how to start it. I'd prefer to turn all the static methods
> > into instance methods and have a TestingServerRunner instance per
> configuration.