Refactor test servers to make them less global.

Refactor test servers to make them less global.

This sets the stage for adding tests for #1386.

R=rnystrom@google.com

Committed: https://github.com/dart-lang/pub/commit/4ef5000549ed0ffe620e7c858b8374f920c62d79

[Bob Nystrom, 2016-02-03 18:40:41]

lgtm

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart
File test/descriptor_server.dart (right):

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:1: // Copyright (c) 2015, the Dart project authors. 
Please see the AUTHORS file
2016 🎉

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:43: void serveErrors() {
What do you think of "serveNothing"?

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:48: /// The server, or `null` until it's available.
"until" -> "before".

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:65: /// This server will exist only for the duration
of the pub run. Subsequent
"will exist" -> "exists".

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart
File test/package_server.dart (right):

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:54: /// The [d.DirectoryDescriptor] describing the
server layout of `/api/packages`
Some long lines here.

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:59: /// this test.
This never null now, is it?

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:67: /// this test.
Or this?

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:123: void replace(void callback(PackageServerBuilder
builder)) {
Maybe "replaceWith"? Right now, it reads like it replaces the thing you pass in.

[nweiz, 2016-02-03 20:12:46]

Code review changes

[nweiz, 2016-02-03 20:13:24]

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart
File test/descriptor_server.dart (right):

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:1: // Copyright (c) 2015, the Dart project authors. 
Please see the AUTHORS file
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> 2016 🎉

Done.

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:43: void serveErrors() {
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> What do you think of "serveNothing"?

It's important that this doesn't just serve 404s, but causes the test to
directly fail if any request comes in.

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:48: /// The server, or `null` until it's available.
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> "until" -> "before".

Done.

https://codereview.chromium.org/1664563002/diff/1/test/descriptor_server.dart...
test/descriptor_server.dart:65: /// This server will exist only for the duration
of the pub run. Subsequent
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> "will exist" -> "exists".

Done.

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart
File test/package_server.dart (right):

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:54: /// The [d.DirectoryDescriptor] describing the
server layout of `/api/packages`
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> Some long lines here.

Done.

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:59: /// this test.
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> This never null now, is it?

Done.

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:67: /// this test.
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> Or this?

Done.

https://codereview.chromium.org/1664563002/diff/1/test/package_server.dart#ne...
test/package_server.dart:123: void replace(void callback(PackageServerBuilder
builder)) {
On 2016/02/03 18:40:41, Bob Nystrom wrote:
> Maybe "replaceWith"? Right now, it reads like it replaces the thing you pass
in.

I think this is pretty clear, given that it passes in a callback and not a
value. Also, see Uri.replace(), which replaces fields with the values that are
passed in.

[nweiz, 2016-02-03 20:13:58]

Description was changed from

==========
Refactor test servers to make them less global.

This sets the stage for adding tests for #1386.

R=rnystrom@google.com
==========

to

==========
Refactor test servers to make them less global.

This sets the stage for adding tests for #1386.

R=rnystrom@google.com

Committed:
https://github.com/dart-lang/pub/commit/4ef5000549ed0ffe620e7c858b8374f920c62d79
==========

[nweiz, 2016-02-03 20:13:59]

Committed patchset #2 (id:20001) manually as
4ef5000549ed0ffe620e7c858b8374f920c62d79 (presubmit successful).