Use the polling watcher by default in pub tests for now.

Use the polling watcher by default in pub tests for now.

This adds a few tests that explicitly exercise the native watcher.

R=rnystrom@google.com
BUG=14941
Committed: https://code.google.com/p/dart/source/detail?r=30094

[Bob Nystrom, 2013-11-08 01:32:31]

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback.dart:128: return new
PollingDirectoryWatcher(directory);
How about making this a factory method on WatcherType itself?

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback/sources.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/sources.dart:27: DirectoryWatcher
watcherFactory(String directory)) {
Why not just pass the WatcherType in here?

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/command/serve.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/command/serve.dart:58: help: 'Force the use of a
polling filesystem watcher.');
How about just "--poll"? It's not really "forcing" since you're just picking the
way to do it.

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/test/se...
File sdk/lib/_internal/pub/test/serve/utils.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/test/se...
sdk/lib/_internal/pub/test/serve/utils.dart:118: /// [dart2js] is `false`, does
not compile Dart entrypoints in "web" to
Remove bit about dart2js (oops, my fault for leaving that in).

[nweiz, 2013-11-08 01:44:24]

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback.dart:128: return new
PollingDirectoryWatcher(directory);
On 2013/11/08 01:32:32, Bob Nystrom wrote:
> How about making this a factory method on WatcherType itself?

Done.

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback/sources.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/sources.dart:27: DirectoryWatcher
watcherFactory(String directory)) {
On 2013/11/08 01:32:32, Bob Nystrom wrote:
> Why not just pass the WatcherType in here?

It seemed a little weird to have this just not handle WatcherType.NONE at all,
but done.

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/command/serve.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/command/serve.dart:58: help: 'Force the use of a
polling filesystem watcher.');
On 2013/11/08 01:32:32, Bob Nystrom wrote:
> How about just "--poll"? It's not really "forcing" since you're just picking
the
> way to do it.

That was how I had it originally, but it seemed weird since the default is to
sometimes poll anyway. What this flag means is *always* poll, which is what
"--force-*" often means. Also "--no-force-poll" is more accurate than
"--no-poll".

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/test/se...
File sdk/lib/_internal/pub/test/serve/utils.dart (right):

https://codereview.chromium.org/64723005/diff/1/sdk/lib/_internal/pub/test/se...
sdk/lib/_internal/pub/test/serve/utils.dart:118: /// [dart2js] is `false`, does
not compile Dart entrypoints in "web" to
On 2013/11/08 01:32:32, Bob Nystrom wrote:
> Remove bit about dart2js (oops, my fault for leaving that in).

Done.

[nweiz, 2013-11-08 01:47:55]

Bob verbally lgtm'd this before he left for the day.

[nweiz, 2013-11-08 01:49:19]

Committed patchset #2 manually as r30094 (presubmit successful).

[Bob Nystrom, 2013-11-08 19:29:08]

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback.dart:129: return new
PollingDirectoryWatcher(directory);
Did you miss uploading a change here? This should just pass in the watcher now,
right?

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback/sources.dart (right):

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback/sources.dart:113: static const AUTO =
const _AutoWatcherType();
I think top-level functions may be usable as const expressions. If so, you can
get rid of the subclasses and just do functions. Either way is fine, though.

[Bob Nystrom, 2013-11-08 19:29:16]

lgtm

[nweiz, 2013-11-08 19:32:09]

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback.dart (right):

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback.dart:129: return new
PollingDirectoryWatcher(directory);
On 2013/11/08 19:29:08, Bob Nystrom wrote:
> Did you miss uploading a change here? This should just pass in the watcher
now,
> right?

Yeah, I just forgot to upload the change that got rid of this.

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
File sdk/lib/_internal/pub/lib/src/barback/sources.dart (right):

https://codereview.chromium.org/64723005/diff/70001/sdk/lib/_internal/pub/lib...
sdk/lib/_internal/pub/lib/src/barback/sources.dart:113: static const AUTO =
const _AutoWatcherType();
On 2013/11/08 19:29:08, Bob Nystrom wrote:
> I think top-level functions may be usable as const expressions. If so, you can
> get rid of the subclasses and just do functions. Either way is fine, though.

It feels weird to me to me to have multiple objects with different
implementations of a method done via closures passed to the constructor, even if
it is terser.