pkg/http_server: virtual dir test cleanup

pkg/http_server: virtual dir test cleanup

...and test util tweaks

R=ajohnsen@google.com

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

[Anders Johnsen, 2014-01-05 19:36:08]

LGTM, with comments. Thanks for the cleanup :)

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/utils.dart
File pkg/http_server/test/utils.dart (right):

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/utils.d...
pkg/http_server/test/utils.dart:11: Future<int> getStatusCode(int port, String
path, {String host,
Please don't change this, this is not the style used for this package. (also,
even if this change, next line should be 4, not 2, lines of indentation).

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
File pkg/http_server/test/virtual_directory_test.dart (right):

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
pkg/http_server/test/virtual_directory_test.dart:555: void _testVirDir(String
name, dynamic func(HttpServer server, Directory dir)) {
Please move these helpers to either utils.dart or top of file.

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
pkg/http_server/test/virtual_directory_test.dart:561: .then((HttpServer value) {
No need for type here.

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
pkg/http_server/test/virtual_directory_test.dart:563: 
Remove empty lines.

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
pkg/http_server/test/virtual_directory_test.dart:570: server.close(force: true),
Move up on above line.

https://codereview.chromium.org/124693002/diff/1/pkg/http_server/test/virtual...
pkg/http_server/test/virtual_directory_test.dart:570: server.close(force: true),
Please don't use force - it'll make it harder to track potential issues.

[kevmoo, 2014-01-05 22:21:21]

Committed patchset #3 manually as r31466 (presubmit successful).

[Siggi Cherem (dart-lang), 2014-01-06 17:40:23]

lgtm, minor comments below

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/ut...
File pkg/http_server/test/utils.dart (right):

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/ut...
pkg/http_server/test/utils.dart:17: Uri uri;
note that we prefer 'var' for local variables. Was this triggering some warning?

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/vi...
File pkg/http_server/test/virtual_directory_test.dart (right):

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/vi...
pkg/http_server/test/virtual_directory_test.dart:50: var virDir = new
VirtualDirectory(dir.path);
maybe it's worth creating this instance in _testVirDir, since you always do this
immediately at the beginning of this callback?

initially I thought also calling virDir.serve, but I can see that you need to
configure it differently on each test.

[kevmoo, 2014-01-06 18:11:32]

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/ut...
File pkg/http_server/test/utils.dart (right):

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/ut...
pkg/http_server/test/utils.dart:17: Uri uri;
I'm a big fan of being explicit w/ the local var type if it is not immediately
assigned.

Keeps the intent clear if one has a big if block, case statement, etc

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/vi...
File pkg/http_server/test/virtual_directory_test.dart (right):

https://codereview.chromium.org/124693002/diff/110001/pkg/http_server/test/vi...
pkg/http_server/test/virtual_directory_test.dart:50: var virDir = new
VirtualDirectory(dir.path);
See https://codereview.chromium.org/120343005/

Moving in that direction