Make test.dart self-contained again.

Make test.dart self-contained again.

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

[ahe, 2013-01-16 14:11:25]

Reason given in https://codereview.chromium.org/11810004/.

[Mads Ager (google), 2013-01-16 14:23:04]

LGTM

https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/http_...
File dart/tools/testing/dart/http_server.dart (left):

https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/http_...
dart/tools/testing/dart/http_server.dart:10: import
'../../../pkg/args/lib/args.dart';
This is a shame, we should work to reenable it. Maybe we don't need fancy args
parsing for this but it will make it hard to test locally without this.

[Emily Fortuna, 2013-01-16 19:25:49]

I disagree strongly with this change. We should not remove this functionality
without providing a replacement. Especially since removing this does not
actually make test.dart self-contained! 

PS. Please do not add an additional TODO with my name on it since I already
added a TODO with the *same* content on line 906 of test_suite.dart.

[Jennifer Messerly, 2013-01-16 19:34:00]

As a point of order, this CL should *not* have been committed without waiting
for Emily's input. You are deleting code she recently checked in, after a
lengthy review period which you were on, but did not reply with concerns until
after it was submitted.

Also, I think this is the wrong fix on multiple levels. The right fix would be
to vendor in args.dart -- make a copy in tools/testing. Now test.dart is self
contained, and we didn't lose all of the ability to test the server locally.

https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/test_...
File dart/tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/test_...
dart/tools/testing/dart/test_suite.dart:27: // TODO(efortuna,whess): Remove this
import.
You should not add a TODO with someone else's name unless they agree to add it.
The point of the name beside the TODO is the person that can explain it, not the
person that can do it.

(this is explained in Google style guides. Here's python for example:
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=TODO_C...)

[dgrove, 2013-01-16 19:50:33]

Peter - 

I also believe this change should not have been committed before Emily's input.

I would like to see this change reverted, and a better solution arrived at.

On 2013/01/16 19:34:00, John Messerly wrote:
> As a point of order, this CL should *not* have been committed without waiting
> for Emily's input. You are deleting code she recently checked in, after a
> lengthy review period which you were on, but did not reply with concerns until
> after it was submitted.
> 
> Also, I think this is the wrong fix on multiple levels. The right fix would be
> to vendor in args.dart -- make a copy in tools/testing. Now test.dart is self
> contained, and we didn't lose all of the ability to test the server locally.
> 
>
https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/test_...
> File dart/tools/testing/dart/test_suite.dart (right):
> 
>
https://codereview.chromium.org/11961017/diff/1/dart/tools/testing/dart/test_...
> dart/tools/testing/dart/test_suite.dart:27: // TODO(efortuna,whess): Remove
this
> import.
> You should not add a TODO with someone else's name unless they agree to add
it.
> The point of the name beside the TODO is the person that can explain it, not
the
> person that can do it.
> 
> (this is explained in Google style guides. Here's python for example:
>
http://google-styleguide.googlecode.com/svn/trunk/pyguide.html?showone=TODO_C...)