Added the ability for testrunner to start the HTTP server with a random port and communicate that p…

Added the ability for testrunner to start the HTTP server with a random port and communicate that port to the tests via a 'testState' map in unittest.
Committed: https://code.google.com/p/dart/source/detail?r=13507

[gram, 2012-10-10 21:52:29]

You will be most amused by the fact that I now always return the Future from
_processHelper.

[Siggi Cherem (dart-lang), 2012-10-10 22:07:28]

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pipeline_...
File utils/testrunner/pipeline_utils.dart (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pipeline_...
utils/testrunner/pipeline_utils.dart:54: * when the process terminates; if
[procId] is instead non-zero
nit: fix comment now that the future is always returned :)

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.yaml
File utils/testrunner/pubspec.yaml (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:1: name: testrunner
yay! awesome!

There are a few other fields you should have so that it shows really nicely when
you upload it to pub:

author: "Dart Team <mailto:misc@dartlang.org>"
homepage: url-to-either where the code or the documentation is
description: A single sentence introducing what this is.
version: 0.0.1

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:3: args: { sdk: args }
probably also add
  unittest: { sdk: unittest }
?

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
File utils/testrunner/run_pipeline.dart (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:102: serverPort = 1024 +
r.nextInt(32768-1024);
nit: spaces around -

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:109: var t = new Timer(SERVER_DELAY, (t) {
seems a bit brittle to wait 1 sec.

How about print something out from the subprocess so we can tell whether the
port was accepted? If the process exists before you get the print message, then
you try again?

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:112: } else if (remainingAttempts == 0){
nit: space before {

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:116: tryStartHTTPServer(r,
remainingAttempts-1);
nit: spaces

[gram, 2012-10-10 23:36:04]

PTAL

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pipeline_...
File utils/testrunner/pipeline_utils.dart (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pipeline_...
utils/testrunner/pipeline_utils.dart:54: * when the process terminates; if
[procId] is instead non-zero
On 2012/10/10 22:07:28, sigmund wrote:
> nit: fix comment now that the future is always returned :)

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.yaml
File utils/testrunner/pubspec.yaml (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:1: name: testrunner
On 2012/10/10 22:07:28, sigmund wrote:
> yay! awesome!
> 
> There are a few other fields you should have so that it shows really nicely
when
> you upload it to pub:
> 
> author: "Dart Team <mailto:misc@dartlang.org>"
> homepage: url-to-either where the code or the documentation is
> description: A single sentence introducing what this is.
> version: 0.0.1

Done, apart from homepage, as I don't know yet what that will be.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:3: args: { sdk: args }
On 2012/10/10 22:07:28, sigmund wrote:
> probably also add
>   unittest: { sdk: unittest }
> ?

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:3: args: { sdk: args }
On 2012/10/10 22:07:28, sigmund wrote:
> probably also add
>   unittest: { sdk: unittest }
> ?

Done that - and later this will mean that we can set the --unittest argument
default sensibly.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
File utils/testrunner/run_pipeline.dart (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:102: serverPort = 1024 +
r.nextInt(32768-1024);
On 2012/10/10 22:07:28, sigmund wrote:
> nit: spaces around - 

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:109: var t = new Timer(SERVER_DELAY, (t) {
On 2012/10/10 22:07:28, sigmund wrote:
> seems a bit brittle to wait 1 sec.
> 
> How about print something out from the subprocess so we can tell whether the
> port was accepted? If the process exists before you get the print message,
then
> you try again?

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:112: } else if (remainingAttempts == 0){
On 2012/10/10 22:07:28, sigmund wrote:
> nit: space before {

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:112: } else if (remainingAttempts == 0){
On 2012/10/10 22:07:28, sigmund wrote:
> nit: space before {

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:116: tryStartHTTPServer(r,
remainingAttempts-1);
On 2012/10/10 22:07:28, sigmund wrote:
> nit: spaces

Done.

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/run_pipel...
utils/testrunner/run_pipeline.dart:116: tryStartHTTPServer(r,
remainingAttempts-1);
On 2012/10/10 22:07:28, sigmund wrote:
> nit: spaces

Done.

[Siggi Cherem (dart-lang), 2012-10-10 23:39:51]

lgtm

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.yaml
File utils/testrunner/pubspec.yaml (right):

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:1: name: testrunner
On 2012/10/10 23:36:04, gram wrote:
> On 2012/10/10 22:07:28, sigmund wrote:
> > yay! awesome!
> > 
> > There are a few other fields you should have so that it shows really nicely
> when
> > you upload it to pub:
> > 
> > author: "Dart Team <mailto:misc@dartlang.org>"
> > homepage: url-to-either where the code or the documentation is
> > description: A single sentence introducing what this is.
> > version: 0.0.1
> 
> Done, apart from homepage, as I don't know yet what that will be.
> 

for some reason the changes to this file didn't get uploaded :(

https://codereview.chromium.org/11103015/diff/2001/utils/testrunner/pubspec.y...
utils/testrunner/pubspec.yaml:3: args: { sdk: args }
On 2012/10/10 23:36:04, gram wrote:
> On 2012/10/10 22:07:28, sigmund wrote:
> > probably also add
> >   unittest: { sdk: unittest }
> > ?
> 
> Done that - and later this will mean that we can set the --unittest argument
> default sensibly.

you can probably just use 'package:unittest/unittest.dart' at that point.