Skip to content

Commit 8d76f5f

Browse files
author
John Messerly
committed
have build/test scripts automatically update baselines
we already have to review them, so running a diff and failing the rest of the test pass seems unhelpful. Instead operate like our generated SDK does, and update in place [email protected] Review URL: https://codereview.chromium.org/1188173003.
1 parent de6df99 commit 8d76f5f

File tree

4 files changed

+23
-58
lines changed

4 files changed

+23
-58
lines changed

pkg/dev_compiler/.gitignore

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
build/
44
packages
55

6-
# Or the files created by tools.
6+
# Ignore files created by tools.
77
*.dart.precompiled.js
88
*.js_
99
*.js.deps
@@ -12,11 +12,12 @@ packages
1212
.pub/
1313
node_modules
1414

15+
# Ignore test output
16+
test/codegen/expect/dev_compiler/
17+
test/codegen/expect/sunflower/dev_compiler/
18+
1519
# Created by ./tool/build_sdk.sh
1620
tool/generated_sdk/
1721

18-
# Or our test outputs
19-
test/codegen/actual/
20-
2122
# Include when developing application packages.
2223
pubspec.lock

pkg/dev_compiler/test/codegen_test.dart

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ main(arguments) {
4949
});
5050

5151
var inputDir = path.join(testDirectory, 'codegen');
52-
var actualDir = path.join(inputDir, 'actual');
52+
var expectDir = path.join(inputDir, 'expect');
5353
var paths = new Directory(inputDir)
5454
.listSync()
5555
.where((f) => f is File)
@@ -66,8 +66,8 @@ main(arguments) {
6666
entryPointFile: entryPoint, dartSdkPath: sdkPath),
6767
codegenOptions: new CodegenOptions(
6868
outputDir: subDir == null
69-
? actualDir
70-
: path.join(actualDir, subDir),
69+
? expectDir
70+
: path.join(expectDir, subDir),
7171
emitSourceMaps: sourceMaps,
7272
forceCompile: checkSdk),
7373
useColors: false,
@@ -80,7 +80,7 @@ main(arguments) {
8080
var realSdk = getSdkDir(arguments).path;
8181

8282
// Remove old output, and `packages` symlinks which mess up the diff.
83-
var dir = new Directory(actualDir);
83+
var dir = new Directory(expectDir);
8484
if (dir.existsSync()) dir.deleteSync(recursive: true);
8585
var packagesDirs = new Directory(inputDir)
8686
.listSync(recursive: true)
@@ -100,10 +100,10 @@ main(arguments) {
100100
var success = !result.failure;
101101

102102
// Write compiler messages to disk.
103-
new File(path.join(actualDir, '$filename.txt'))
103+
new File(path.join(expectDir, '$filename.txt'))
104104
.writeAsStringSync(compilerMessages.toString());
105105

106-
var outFile = new File(path.join(actualDir, '$filename.js'));
106+
var outFile = new File(path.join(expectDir, '$filename.js'));
107107
expect(outFile.existsSync(), success,
108108
reason: '${outFile.path} was created iff compilation succeeds');
109109

@@ -133,8 +133,8 @@ main(arguments) {
133133
// be generated against a specific SDK version.
134134
var testSdk = path.join(testDirectory, 'generated_sdk');
135135
var result = compile('dart:core', testSdk, checkSdk: true);
136-
var outputDir = new Directory(path.join(actualDir, 'core'));
137-
var outFile = new File(path.join(actualDir, 'dart/core.js'));
136+
var outputDir = new Directory(path.join(expectDir, 'core'));
137+
var outFile = new File(path.join(expectDir, 'dart/core.js'));
138138
expect(outFile.existsSync(), true,
139139
reason: '${outFile.path} was created for dart:core');
140140
});
@@ -152,7 +152,7 @@ main(arguments) {
152152
var success = !result.failure;
153153

154154
// Write compiler messages to disk.
155-
new File(path.join(actualDir, 'sunflower', 'sunflower.txt'))
155+
new File(path.join(expectDir, 'sunflower', 'sunflower.txt'))
156156
.writeAsStringSync(compilerMessages.toString());
157157

158158
var expectedFiles = [
@@ -163,7 +163,7 @@ main(arguments) {
163163
]..addAll(expectedRuntime);
164164

165165
for (var filepath in expectedFiles) {
166-
var outFile = new File(path.join(actualDir, 'sunflower', filepath));
166+
var outFile = new File(path.join(expectDir, 'sunflower', filepath));
167167
expect(outFile.existsSync(), success,
168168
reason: '${outFile.path} was created iff compilation succeeds');
169169
}
@@ -177,7 +177,7 @@ main(arguments) {
177177
var success = !result.failure;
178178

179179
// Write compiler messages to disk.
180-
new File(path.join(actualDir, 'html_input.txt'))
180+
new File(path.join(expectDir, 'html_input.txt'))
181181
.writeAsStringSync(compilerMessages.toString());
182182

183183
var expectedFiles = [
@@ -190,7 +190,7 @@ main(arguments) {
190190
]..addAll(expectedRuntime);
191191

192192
for (var filepath in expectedFiles) {
193-
var outFile = new File(path.join(actualDir, filepath));
193+
var outFile = new File(path.join(expectDir, filepath));
194194
expect(outFile.existsSync(), success,
195195
reason: '${outFile.path} was created iff compilation succeeds');
196196
}
@@ -200,7 +200,7 @@ main(arguments) {
200200
'dev_compiler/runtime/messages.css'
201201
];
202202
for (var filepath in notExpectedFiles) {
203-
var outFile = new File(path.join(actualDir, filepath));
203+
var outFile = new File(path.join(expectDir, filepath));
204204
expect(outFile.existsSync(), isFalse,
205205
reason: '${outFile.path} should only be generated in server mode');
206206
}
@@ -215,7 +215,7 @@ main(arguments) {
215215
var success = !result.failure;
216216

217217
// Write compiler messages to disk.
218-
new File(path.join(actualDir, 'server_mode', 'html_input.txt'))
218+
new File(path.join(expectDir, 'server_mode', 'html_input.txt'))
219219
.writeAsStringSync(compilerMessages.toString());
220220

221221
var expectedFiles = [
@@ -229,11 +229,11 @@ main(arguments) {
229229
]..addAll(expectedRuntime);
230230

231231
// Parse the HTML file and verify its contents were expected.
232-
var htmlPath = path.join(actualDir, 'server_mode', 'html_input.html');
232+
var htmlPath = path.join(expectDir, 'server_mode', 'html_input.html');
233233
var doc = html.parse(new File(htmlPath).readAsStringSync());
234234

235235
for (var filepath in expectedFiles) {
236-
var outPath = path.join(actualDir, 'server_mode', filepath);
236+
var outPath = path.join(expectDir, 'server_mode', filepath);
237237
expect(new File(outPath).existsSync(), success,
238238
reason: '$outPath was created iff compilation succeeds');
239239

@@ -257,7 +257,7 @@ main(arguments) {
257257
}
258258

259259
// Clean up the server mode folder, otherwise it causes diff churn.
260-
var dir = new Directory(path.join(actualDir, 'server_mode'));
260+
var dir = new Directory(path.join(expectDir, 'server_mode'));
261261
if (dir.existsSync()) dir.deleteSync(recursive: true);
262262
});
263263
}

pkg/dev_compiler/tool/build_sdk.sh

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,10 @@ fi
1717
# https://github.com/dart-lang/dev_compiler/issues/219
1818
dart -c bin/devc.dart --no-source-maps --sdk-check --force-compile -l warning \
1919
--dart-sdk tool/generated_sdk -o lib/runtime/ dart:mirrors \
20-
> tool/generated_sdk/sdk_errors.txt || true
20+
> tool/sdk_expected_errors.txt || true
2121

2222
if [[ ! -f lib/runtime/dart/core.js ]] ; then
2323
echo 'core.js not found, assuming build failed.'
2424
echo './tool/build_sdk.sh can be run to reproduce this.'
2525
exit 1
2626
fi
27-
28-
DIFF_ARGS="-u tool/sdk_expected_errors.txt tool/generated_sdk/sdk_errors.txt"
29-
30-
if ! (diff $DIFF_ARGS > /dev/null) ; then
31-
diff $DIFF_ARGS |\
32-
sed -e "s/^\(+.*\)/\1/" |\
33-
sed -e "s/^\(-.*\)/\1/"
34-
echo
35-
echo 'SDK errors have changed. To update expectations, run:'
36-
echo '$ cp tool/generated_sdk/sdk_errors.txt tool/sdk_expected_errors.txt'
37-
exit 1
38-
fi

pkg/dev_compiler/tool/test.sh

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,24 +6,6 @@ function fail {
66
return 1
77
}
88

9-
# Arguments passed to the diff tool. We exclude:
10-
# - *.map files so they aren't compared, as the diff is not human readable.
11-
# - runtime JS files that are just copied over from the sources and are not
12-
# duplicated in the expected folder.
13-
DIFF_ARGS="-u -r -N --exclude=\*.map expect actual"
14-
15-
function show_diff {
16-
echo "Fail: actual output did not match expected"
17-
echo
18-
diff $DIFF_ARGS |\
19-
sed -e "s/^\(+.*\)/\1/" |\
20-
sed -e "s/^\(-.*\)/\1/"
21-
echo
22-
echo "You can update these expectations with:"
23-
echo "$ pushd `pwd` && cp -a actual/* expect && popd"
24-
fail
25-
}
26-
279
# Some tests require being run from the package root
2810
# switch to the root directory of dev_compiler
2911
cd $( dirname "${BASH_SOURCE[0]}" )/..
@@ -40,12 +22,6 @@ cd $( dirname "${BASH_SOURCE[0]}" )/..
4022
unset COVERALLS_TOKEN
4123
pub run test:test test/all_tests.dart || fail
4224

43-
# validate codegen_test output
44-
pushd test/codegen/ &> /dev/null
45-
rm -r actual/dev_compiler/ actual/sunflower/dev_compiler
46-
diff $DIFF_ARGS > /dev/null || show_diff
47-
popd &> /dev/null
48-
4925
# run self host and analyzer after other tests, because they're ~seconds to run.
5026
pub run test:test test/checker/self_host_test.dart || fail
5127

0 commit comments

Comments
 (0)