Refactor the barback tests somewhat.

Refactor the barback tests somewhat.

This makes each transformer capable of relatively fine-grained pausing
and resuming of processing. They no longer use MockAsset, instead
using [Asset.fromString] to better simulate real-world transformers.

This also adds several new utility functions to
barback/test/utils.dart.

R=rnystrom@google.com

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

[Bob Nystrom, 2013-07-30 22:25:02]

Thanks for the bite-sized patch!

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
File pkg/barback/lib/src/asset_set.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
pkg/barback/lib/src/asset_set.dart:23: AssetSet.from(Iterable<Asset> other) {
Document, especially that later Assets take precedence over earlier ones with
the same ID.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
pkg/barback/lib/src/asset_set.dart:62: Asset removeId(AssetId id) =>
_assets.remove(id);
Document.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
File pkg/barback/test/asset_set_test.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
pkg/barback/test/asset_set_test.dart:128: test("removes the asset with the ID
from the set", () {
Test removing a non-present asset.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
pkg/barback/test/asset_set_test.dart:133:
expect(set.removeId(fooId).readAsString(), completion(equals("foo")));
Make "returns the removed Asset" a separate test.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
File pkg/barback/test/transformer/mock.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
pkg/barback/test/transformer/mock.dart:107: return newFuture(() {
Is this nested newFuture needed?

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dart
File pkg/barback/test/utils.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:109: /// Schedules an error when loading the asset
identified by [name].
"error" -> "error to be generated".

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:271: predicate((error) => error.id == id, 'id is
$name'));
"is" -> "=="

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:302: final _errors = new Set<AssetId>();
Document.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:360: void _setAssetError(String name) =>
_errors.add(new AssetId.parse(name));
Document.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:385: var error = _errors.contains(id);
"hasError"

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:422: class _MockAsset implements Asset {
Can you get rid of this entirely?

[nweiz, 2013-07-30 22:46:21]

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
File pkg/barback/lib/src/asset_set.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
pkg/barback/lib/src/asset_set.dart:23: AssetSet.from(Iterable<Asset> other) {
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Document, especially that later Assets take precedence over earlier ones with
> the same ID.

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/lib/src/asset_...
pkg/barback/lib/src/asset_set.dart:62: Asset removeId(AssetId id) =>
_assets.remove(id);
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Document.

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
File pkg/barback/test/asset_set_test.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
pkg/barback/test/asset_set_test.dart:128: test("removes the asset with the ID
from the set", () {
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Test removing a non-present asset.

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/asset_set...
pkg/barback/test/asset_set_test.dart:133:
expect(set.removeId(fooId).readAsString(), completion(equals("foo")));
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Make "returns the removed Asset" a separate test.

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
File pkg/barback/test/transformer/mock.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
pkg/barback/test/transformer/mock.dart:107: return newFuture(() {
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Is this nested newFuture needed?

It lets us wait on _isPrimary[asset.id].future if that exists. It's this or that
weird ?:/Future.value construction. I like this a little better, but I could go
either way.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dart
File pkg/barback/test/utils.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:109: /// Schedules an error when loading the asset
identified by [name].
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> "error" -> "error to be generated".

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:271: predicate((error) => error.id == id, 'id is
$name'));
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> "is" -> "=="

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:302: final _errors = new Set<AssetId>();
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Document.

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:360: void _setAssetError(String name) =>
_errors.add(new AssetId.parse(name));
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Document.

Like [_modifyAsset], I didn't document this because it's effectively already
documented above for [setAssetError].

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:385: var error = _errors.contains(id);
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> "hasError"

Done.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:422: class _MockAsset implements Asset {
On 2013/07/30 22:25:02, Bob Nystrom wrote:
> Can you get rid of this entirely?

I considered that, but the fact that MockProvider directly sets [contents]
better emulates file assets, where the results of [readAsString] can
theoretically change from call to call. That shouldn't matter, but this is more
likely to fail if it does end up mattering.

[Bob Nystrom, 2013-07-30 23:14:08]

LGTM!

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
File pkg/barback/test/transformer/mock.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/transform...
pkg/barback/test/transformer/mock.dart:107: return newFuture(() {
On 2013/07/30 22:46:21, nweiz wrote:
> On 2013/07/30 22:25:02, Bob Nystrom wrote:
> > Is this nested newFuture needed?
> 
> It lets us wait on _isPrimary[asset.id].future if that exists. It's this or
that
> weird ?:/Future.value construction. I like this a little better, but I could
go
> either way.

Ah, I see. I didn't notives that the .then((_) => result) was inside the outer
future.

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dart
File pkg/barback/test/utils.dart (right):

https://codereview.chromium.org/21226004/diff/3001/pkg/barback/test/utils.dar...
pkg/barback/test/utils.dart:422: class _MockAsset implements Asset {
On 2013/07/30 22:46:21, nweiz wrote:
> On 2013/07/30 22:25:02, Bob Nystrom wrote:
> > Can you get rid of this entirely?
> 
> I considered that, but the fact that MockProvider directly sets [contents]
> better emulates file assets, where the results of [readAsString] can
> theoretically change from call to call. That shouldn't matter, but this is
more
> likely to fail if it does end up mattering.

SGTM.

[nweiz, 2013-07-30 23:57:29]

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