Add a filesystem descriptor library to scheduled_test.

Add a filesystem descriptor library to scheduled_test.

BUG=8511
Committed: https://code.google.com/p/dart/source/detail?r=18928

[Bob Nystrom, 2013-02-22 17:58:21]

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
File pkg/scheduled_test/lib/descriptor.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:49: ///           return new
Directory('').createTemp().then((dir) {
Seems like this is almost always the behavior you want. Can it default to this
if you don't set it?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:79: /// validated at this path. Defaults
to the current working directory.
How about defaulting to a temp directory for each schedule?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:89: /// Creates a new [File] descriptor
with [name] and [contents].
new -> new text

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:90: File file(Pattern name, String
contents) => new File(name, contents);
Maybe make contents optional and default to just the file name if omitted?
Sometimes you want to create a file but you don't really care what it contains.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:97: Directory dir(Pattern name,
[List<Entry> contents]) =>
List -> Iterable

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/directory.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:55: entry.stringName ==
split.first);
You might want a .toList() here since you use .length repeatedly below.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:80:
buffer..add(description)..add('\n');
buffer.writeln(description);

and below

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:89:
buffer.add(lastEntryString);
May want a trailing newline here.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/entry.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:15: /// The name of this entry.
For most operations, this must be a [String;
].

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:23: /// Schedules the creation
of the described entry within [parent]. Returns a
[parent] -> the [parent] directory

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:24: /// [Future] that is
completed after the creation is done.
is completed -> completes

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:42: /// contents of the file
will be loaded.
This feels like two conceptually different methods combined into one. How about:

Stream<List<int>> load(String path)
A method only on Directory that finds the right child entry and returns its
contents. It does so by building a path and then calling:

Stream<List<int>> read()
A method on Entry that returns the contents of the given entry. File entry types
implement this. Directory throws.

Thoughts?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/file.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:30: File.bytes(Pattern name,
List<int> contents)
bytes -> binary

The rest of the package seems to use "binary" pretty consistently, and I think
new File.binary() reads well.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:52: "data.";
If the data is pretty small, it might be nice to show a hex dump. Or, if too
big, maybe a hex dump of the head and tail.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:61: "${prefixLines(new
String.fromCharCodes(actualContents))}";
Consider stealing the comparison code in test_pub for this. It tries to
highlight which lines are different or missing so it's easier to see where the
difference is.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/utils.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/utils.dart:26: var matchingEntries = new
Directory(parent).listSync()
Yay listSync()!

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/utils.dart:44: if (pattern is String)
return "'$pattern'";
Do you care about escaping quotes here?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
File pkg/scheduled_test/lib/src/utils.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:35: if (iter1.current != iter2.current)
return false;
The iterator protocol says you need to call moveNext() once to advance to the
first item. This will coincidentally work because it also says conforming types
should return null from current if moveNext() has not been called yet, but it
feels a bit shady to rely on that since you aren't actually comparing items
here.

You should be able to just move this to the end of the loop.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:43: /// Returns a [Stream] that will
immediately emit [error] and then close.
Given corelib has Future.immediateError, I wonder if it should have new
Stream.immediateError too? File a bug?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:49: Stream liftStream(Future<Stream>
future) {
Instead of "lift", how about "deferred" or just "future"? It wasn't clear to me
what this did at the callsite.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
File pkg/scheduled_test/test/descriptor_test.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:19: expectTestsPass('file().create
creates a file', () {
Given that Dart has getters, it's confusing that () is left off the method
descriptions here. Can you change them to: file().create(), etc?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:47: "Pattern /name\\.txt/ must be a
string."
Use a raw string to simplify the \\?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:51: 
Test file().create() if the file already exists. I'm guessing it should succeed
and replace. Ditto for binaryFile().

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:111: matches(r"^Entry not found:
'[^']+/name\.txt'\.$"));
Can we do "File" instead of "Entry" in the error?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:766: d.file('name.txt', 'contents')
Should this be an early error? Is it ever valid for a directory to have two
children with the same name?

[nweiz, 2013-02-22 20:57:20]

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
File pkg/scheduled_test/lib/descriptor.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:49: ///           return new
Directory('').createTemp().then((dir) {
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Seems like this is almost always the behavior you want. Can it default to this
> if you don't set it?

I don't think it's possible to have this code run consistently enough to provide
a good user experience. There's no way for a library to register a setUp
function, so it would have to lazily run when the user first creates a
descriptor. This could be after the schedule has started running, which means
that from the user's perspective it's going to be confusing when the sandbox
path is available and when it's not. To say nothing of the confusion if the user
doesn't realize that their directories are being created in some temp directory
somewhere.

I think on the whole it's easier for the user to grok if they have control of
the sandbox creation and management. I could add a library function in a future
CL to make this easier, though.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:79: /// validated at this path. Defaults
to the current working directory.
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> How about defaulting to a temp directory for each schedule?

See above.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:89: /// Creates a new [File] descriptor
with [name] and [contents].
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> new -> new text

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:90: File file(Pattern name, String
contents) => new File(name, contents);
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Maybe make contents optional and default to just the file name if omitted?
> Sometimes you want to create a file but you don't really care what it
contains.

Done, although I think it's more intuitive if it defaults to an empty file.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:97: Directory dir(Pattern name,
[List<Entry> contents]) =>
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> List -> Iterable

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/directory.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:55: entry.stringName ==
split.first);
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> You might want a .toList() here since you use .length repeatedly below.

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:80:
buffer..add(description)..add('\n');
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> buffer.writeln(description);
> 
> and below

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/directory.dart:89:
buffer.add(lastEntryString);
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> May want a trailing newline here.

Nope. This method is like toString in that it's assumed not to return a trailing
newline.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/entry.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:15: /// The name of this entry.
For most operations, this must be a [String;
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> ].

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:23: /// Schedules the creation
of the described entry within [parent]. Returns a
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> [parent] -> the [parent] directory

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:24: /// [Future] that is
completed after the creation is done.
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> is completed -> completes

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/entry.dart:42: /// contents of the file
will be loaded.
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> This feels like two conceptually different methods combined into one. How
about:
> 
> Stream<List<int>> load(String path)
> A method only on Directory that finds the right child entry and returns its
> contents. It does so by building a path and then calling:
> 
> Stream<List<int>> read()
> A method on Entry that returns the contents of the given entry. File entry
types
> implement this. Directory throws.
> 
> Thoughts?

I think it's important that there not be methods specific to a certain type of
Entry, meaning that load() should be on Entry as well. Otherwise, it makes it
harder for new entry types to be added transparently. But I do like this
factoring.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/file.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:30: File.bytes(Pattern name,
List<int> contents)
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> bytes -> binary
> 
> The rest of the package seems to use "binary" pretty consistently, and I think
> new File.binary() reads well.

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:52: "data.";
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> If the data is pretty small, it might be nice to show a hex dump. Or, if too
> big, maybe a hex dump of the head and tail.

I'll add a TODO.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/file.dart:61: "${prefixLines(new
String.fromCharCodes(actualContents))}";
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Consider stealing the comparison code in test_pub for this. It tries to
> highlight which lines are different or missing so it's easier to see where the
> difference is.

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
File pkg/scheduled_test/lib/src/descriptor/utils.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/de...
pkg/scheduled_test/lib/src/descriptor/utils.dart:44: if (pattern is String)
return "'$pattern'";
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Do you care about escaping quotes here?

I'd *like* to escape enough stuff that it's a valid string literal, but we don't
have a library function for that and I don't really want to write it right now.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
File pkg/scheduled_test/lib/src/utils.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:35: if (iter1.current != iter2.current)
return false;
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> The iterator protocol says you need to call moveNext() once to advance to the
> first item. This will coincidentally work because it also says conforming
types
> should return null from current if moveNext() has not been called yet, but it
> feels a bit shady to rely on that since you aren't actually comparing items
> here.
> 
> You should be able to just move this to the end of the loop.

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:43: /// Returns a [Stream] that will
immediately emit [error] and then close.
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Given corelib has Future.immediateError, I wonder if it should have new
> Stream.immediateError too? File a bug?

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/src/ut...
pkg/scheduled_test/lib/src/utils.dart:49: Stream liftStream(Future<Stream>
future) {
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Instead of "lift", how about "deferred" or just "future"? It wasn't clear to
me
> what this did at the callsite.

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
File pkg/scheduled_test/test/descriptor_test.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:19: expectTestsPass('file().create
creates a file', () {
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Given that Dart has getters, it's confusing that () is left off the method
> descriptions here. Can you change them to: file().create(), etc?

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:47: "Pattern /name\\.txt/ must be a
string."
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Use a raw string to simplify the \\?

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:51: 
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Test file().create() if the file already exists. I'm guessing it should
succeed
> and replace. Ditto for binaryFile().

Done for file(). I don't want to re-write all the file() tests for binaryFile(),
since they use the same implementation.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:111: matches(r"^Entry not found:
'[^']+/name\.txt'\.$"));
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Can we do "File" instead of "Entry" in the error?

Done.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/test/descr...
pkg/scheduled_test/test/descriptor_test.dart:766: d.file('name.txt', 'contents')
On 2013/02/22 17:58:21, Bob Nystrom wrote:
> Should this be an early error? Is it ever valid for a directory to have two
> children with the same name?

It will be valid once we add asynchronous descriptors, since their names come up
as "<unknown>" or something like that.

[Bob Nystrom, 2013-02-22 22:08:56]

LGTM, though I'd rather not see load() in Entry.

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
File pkg/scheduled_test/lib/descriptor.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:49: ///           return new
Directory('').createTemp().then((dir) {
On 2013/02/22 20:57:20, nweiz wrote:
> There's no way for a library to register a setUp
> function, so it would have to lazily run when the user first creates a
> descriptor.

Could schedule() do that eagerly?

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:90: File file(Pattern name, String
contents) => new File(name, contents);
On 2013/02/22 20:57:20, nweiz wrote:
> On 2013/02/22 17:58:21, Bob Nystrom wrote:
> > Maybe make contents optional and default to just the file name if omitted?
> > Sometimes you want to create a file but you don't really care what it
> contains.
> 
> Done, although I think it's more intuitive if it defaults to an empty file.

That works too.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/directory.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/directory.dart:72: Stream<List<int>>
read() => errorStream("Can't load the contents of "
"load" -> "read"

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/entry.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/entry.dart:38: /// contents of the entry
at [path]. This only works if [this] is a directory
"entry at [path]" -> "child entry located at [path]"

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/entry.dart:39: /// entry. This operation
is not [schedule]d.
Given "This only works..." I still feel like there's no value in having this on
the Entry class. It's not like all subclasses of Entry are pure implementation
that use the exact same interface. With Directory, there are some different
capabilities too, and I think it's fine if the types reflect that.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/file.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/file.dart:66: "'$pathToLoad' from within
$nameDescription: not a directory.");
Given that Directory is the only type that will implement this, you may as well
move this implementation up to Entry so other types can inherit it.

[nweiz, 2013-02-22 23:02:07]

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

[nweiz, 2013-02-22 23:05:21]

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
File pkg/scheduled_test/lib/descriptor.dart (right):

https://codereview.chromium.org/12330062/diff/1/pkg/scheduled_test/lib/descri...
pkg/scheduled_test/lib/descriptor.dart:49: ///           return new
Directory('').createTemp().then((dir) {
On 2013/02/22 22:08:57, Bob Nystrom wrote:
> On 2013/02/22 20:57:20, nweiz wrote:
> > There's no way for a library to register a setUp
> > function, so it would have to lazily run when the user first creates a
> > descriptor.
> 
> Could schedule() do that eagerly?

It's a Dart constraint, not a unittest constraint. There's no way for a library
being imported to cause any code to be run, including code that would register
this sort of callback.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/directory.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/directory.dart:72: Stream<List<int>>
read() => errorStream("Can't load the contents of "
On 2013/02/22 22:08:57, Bob Nystrom wrote:
> "load" -> "read"

Done.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/entry.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/entry.dart:38: /// contents of the entry
at [path]. This only works if [this] is a directory
On 2013/02/22 22:08:57, Bob Nystrom wrote:
> "entry at [path]" -> "child entry located at [path]"

Done.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/entry.dart:39: /// entry. This operation
is not [schedule]d.
On 2013/02/22 22:08:57, Bob Nystrom wrote:
> Given "This only works..." I still feel like there's no value in having this
on
> the Entry class. It's not like all subclasses of Entry are pure implementation
> that use the exact same interface. With Directory, there are some different
> capabilities too, and I think it's fine if the types reflect that.

That would make it impossible to write a class like FutureDescriptor that can
transparently wrap both Directory and File descriptors.

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
File pkg/scheduled_test/lib/src/descriptor/file.dart (right):

https://codereview.chromium.org/12330062/diff/3003/pkg/scheduled_test/lib/src...
pkg/scheduled_test/lib/src/descriptor/file.dart:66: "'$pathToLoad' from within
$nameDescription: not a directory.");
On 2013/02/22 22:08:57, Bob Nystrom wrote:
> Given that Directory is the only type that will implement this, you may as
well
> move this implementation up to Entry so other types can inherit it.

Done.