Skip to content

add transformer tests #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/src/builder/build_step_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class BuildStepImpl implements BuildStep {
final AssetWriter _writer;

BuildStepImpl(
this.input, List<AssetId> expectedOutputs, this._reader, this._writer)
this.input, Iterable<AssetId> expectedOutputs, this._reader, this._writer)
: expectedOutputs = new List.unmodifiable(expectedOutputs) {
/// The [input] is always a dependency.
_dependencies.add(input.id);
Expand Down
61 changes: 43 additions & 18 deletions lib/src/transformer/transformer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,17 @@ import '../asset/writer.dart';
import '../builder/builder.dart';
import '../builder/build_step_impl.dart';

abstract class BuilderTransformer implements Transformer, DeclaringTransformer {
/// A [Transformer] which runs multiple [Builder]s.
/// Extend this class and define the [builders] getter to create a [Transformer]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should be explicit about our (lack of) ordering guarantees here, maybe just as simple as "there are no guarantees about the order in which [Builder]s will be run."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

/// out of your custom [Builder]s.
abstract class BuilderTransformer implements Transformer {
/// The only thing you need to implement when extending this class. This
/// declares which builders should be ran.
///
/// **Note**: All [builders] are ran in the same phase, and there are no
/// ordering guarantees. Thus, none of the [builders] can use the outputs of
/// other [builders]. In order to do this you must create a [TransformerGroup]
/// with multiple [BuilderTransformer]s.
List<Builder> get builders;

@override
Expand All @@ -30,23 +40,36 @@ abstract class BuilderTransformer implements Transformer, DeclaringTransformer {
var reader = new _TransformAssetReader(transform);
var writer = new _TransformAssetWriter(transform);

var futures = <Future>[];
for (var builder in builders) {
var expected = _expectedOutputs(transform.primaryInput.id, [builder]);
if (expected.isEmpty) continue;
// Tracks all the expected outputs for each builder to make sure they don't
// overlap.
var allExpected = new Set<build.AssetId>();

// Run all builders at the same time.
await Future.wait(builders.map((builder) async {
var expected = _expectedOutputs(transform.primaryInput.id, [builder]);
if (expected.isEmpty) return;

// Check that no expected outputs already exist.
var preExistingFiles = [];
for (var output in expected) {
if (!allExpected.add(output) ||
await transform.hasInput(_toBarbackAssetId(output))) {
preExistingFiles.add(_toBarbackAssetId(output));
}
}
if (preExistingFiles.isNotEmpty) {
transform.logger.error(
'Builder `$builder` declared outputs `$preExistingFiles` but those '
'files already exist. That build step has been skipped.',
asset: transform.primaryInput.id);
return;
}

// Run the build step.
var buildStep = new BuildStepImpl(input, expected, reader, writer);
futures.add(builder.build(buildStep));
}

await Future.wait(futures);
}

@override
void declareOutputs(DeclaringTransform transform) {
for (var outputId in _expectedOutputs(transform.primaryId, builders)) {
transform.declareOutput(_toBarbackAssetId(outputId));
}
await builder.build(buildStep);
await buildStep.outputsCompleted;
}));
}
}

Expand All @@ -68,8 +91,10 @@ class _TransformAssetWriter implements AssetWriter {
_TransformAssetWriter(this.transform);

@override
Future writeAsString(build.Asset asset, {Encoding encoding: UTF8}) async =>
transform.addOutput(_toBarbackAsset(asset));
Future writeAsString(build.Asset asset, {Encoding encoding: UTF8}) {
transform.addOutput(_toBarbackAsset(asset));
return new Future.value(null);
}
}

/// All the expected outputs for [id] given [builders].
Expand Down
1 change: 1 addition & 0 deletions pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@ dependencies:

dev_dependencies:
test: ^0.12.0
transformer_test: ^0.1.0
5 changes: 5 additions & 0 deletions test/common/common.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
export 'copy_builder.dart';
export 'generic_builder_transformer.dart';
30 changes: 30 additions & 0 deletions test/common/copy_builder.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'dart:async';

import 'package:build/build.dart';

class CopyBuilder implements Builder {
int numCopies;

CopyBuilder({this.numCopies: 1});

Future build(BuildStep buildStep) async {
var ids = declareOutputs(buildStep.input.id);
for (var id in ids) {
buildStep.writeAsString(new Asset(id, buildStep.input.stringContents));
}
}

List<AssetId> declareOutputs(AssetId input) {
var outputs = [];
for (int i = 0; i < numCopies; i++) {
outputs.add(_copiedAssetId(input, numCopies == 1 ? null : i));
}
return outputs;
}
}

AssetId _copiedAssetId(AssetId inputId, int copyNum) =>
inputId.addExtension('.copy${copyNum == null ? '' : '.$copyNum'}');
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:build/build.dart';
import 'package:test/test.dart';

main() {
group('A group of tests', () {
test('First Test', () {
expect(true, isTrue);
});
});
class GenericBuilderTransformer extends BuilderTransformer {
final List<Builder> builders;

GenericBuilderTransformer(this.builders);
}
117 changes: 117 additions & 0 deletions test/transformer/transformer_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright (c) 2016, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
@TestOn('vm')
import 'package:test/test.dart';
import 'package:transformer_test/utils.dart';

import '../common/common.dart';

main() {
var singleCopyTransformer =

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the creation of these to a setUp method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they are stateless, I prefer them to be set up just once.

new GenericBuilderTransformer([new CopyBuilder()]);
var multiCopyTransformer =
new GenericBuilderTransformer([new CopyBuilder(numCopies: 2)]);
var singleAndMultiCopyTransformer = new GenericBuilderTransformer(
[new CopyBuilder(), new CopyBuilder(numCopies: 2)]);

testPhases('single builder, single output', [
[singleCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy': 'hello',
});

testPhases('single builder, multiple outputs', [
[multiCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy.0': 'hello',
'a|web/a.txt.copy.1': 'hello',
});

testPhases('multiple builders, different outputs', [
[singleAndMultiCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy': 'hello',
'a|web/a.txt.copy.0': 'hello',
'a|web/a.txt.copy.1': 'hello',
});

testPhases('multiple builders, same outputs', [
[
new GenericBuilderTransformer([new CopyBuilder(), new CopyBuilder()])
],
], {
'a|web/a.txt': 'hello',
}, {}, [
_fileExistsError('CopyBuilder', ['a|web/a.txt.copy']),
]);

testPhases('multiple phases', [
[singleCopyTransformer],
[multiCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy': 'hello',
'a|web/a.txt.copy.0': 'hello',
'a|web/a.txt.copy.1': 'hello',
'a|web/a.txt.copy.copy.0': 'hello',
'a|web/a.txt.copy.copy.1': 'hello',
});

testPhases('multiple transformers in the same phase', [
[singleCopyTransformer, multiCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy': 'hello',
'a|web/a.txt.copy.0': 'hello',
'a|web/a.txt.copy.1': 'hello',
});

testPhases('cant overwrite files', [
[singleCopyTransformer]
], {
'a|web/a.txt': 'hello',
'a|web/a.txt.copy': 'hello',
}, {}, [
_fileExistsError("CopyBuilder", ["a|web/a.txt.copy"]),
]);

// TODO(jakemac): Skipped because we can't detect this situation today.
// Instead you get a barback error, see
// https://github.com/dart-lang/transformer_test/issues/2
//
// testPhases('builders in the same phase can\'t output the same file', [
// [singleCopyTransformer, new GenericBuilderTransformer([new CopyBuilder()])]
// ], {
// 'a|web/a.txt': 'hello',
// }, {
// 'a|web/a.txt.copy': 'hello',
// }, [
// _fileExistsError("CopyBuilder", ["a|web/a.txt.copy"]),
// ]);

testPhases('builders in separate phases can\'t output the same file', [
[singleCopyTransformer],
[singleCopyTransformer],
], {
'a|web/a.txt': 'hello',
}, {
'a|web/a.txt.copy': 'hello',
}, [
_fileExistsError("CopyBuilder", ["a|web/a.txt.copy"]),
]);
}

String _fileExistsError(String builder, List<String> files) {
return "error: Builder `Instance of '$builder'` declared outputs "
"`$files` but those files already exist. That build step has been "
"skipped.";
}