Skip to content

Commit 8d77908

Browse files
johnniwintherCommit Queue
authored andcommitted
[test_runner] Support update_static_error_tests on Windows
A previous change inadvertently broke the support for Windows. This fixes this and adds an end-to-end test to avoid this. Change-Id: I6f8563665038a7ecb0b30b5dfb7051b606d90620 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420660 Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Bob Nystrom <[email protected]>
1 parent 9e1ae23 commit 8d77908

File tree

4 files changed

+116
-10
lines changed

4 files changed

+116
-10
lines changed

pkg/test_runner/lib/src/path.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ class Path {
2121

2222
Path._internal(this._path, this.isWindowsShare);
2323

24+
/// Normalizes [source] to use '/' as delimiter.
25+
static String normalize(String source) {
26+
return Path(source)._path;
27+
}
28+
2429
static String _clean(String source) {
2530
if (Platform.operatingSystem == 'windows') return _cleanWindows(source);
2631
// Remove trailing slash from directories:

pkg/test_runner/lib/src/test_file.dart

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ abstract class _TestFileBase {
108108
return originPath.relativeTo(_suiteDirectory).toString().hashCode;
109109
}
110110

111-
_TestFileBase(this._suiteDirectory, this.path, this.expectedErrors) {
112-
// The VM C++ unit tests have a special fake TestFile with no path.
113-
assert(path.isAbsolute);
114-
}
111+
_TestFileBase(this._suiteDirectory, this.path, this.expectedErrors);
115112

116113
/// The logical name of the test.
117114
///
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
import 'package:path/path.dart' as p;
7+
import 'package:test_runner/src/utils.dart';
8+
9+
const commonArguments = [
10+
'run',
11+
'pkg/test_runner/tool/update_static_error_tests.dart',
12+
'--update=cfe',
13+
];
14+
15+
const testNameParts = [
16+
'language',
17+
'compile_time_constant',
18+
'compile_time_constant_test'
19+
];
20+
21+
final fileNameParts = [
22+
'tests',
23+
...testNameParts.take(testNameParts.length - 1),
24+
'${testNameParts.last}.dart',
25+
];
26+
27+
const expectedLine = 'Running CFE on 1 file...';
28+
final expectedUpdateText = '${testNameParts.last}.dart (3 errors)';
29+
30+
void main() async {
31+
var testFile = File(p.joinAll(fileNameParts));
32+
var testContent = testFile.readAsStringSync();
33+
try {
34+
var errorFound = false;
35+
36+
var testName = testNameParts.join('/');
37+
errorFound |= await run(testName);
38+
39+
var relativeNativePath = p.joinAll(fileNameParts);
40+
errorFound |= await run(relativeNativePath);
41+
42+
var relativeUriPath = fileNameParts.join('/');
43+
errorFound |= await run(relativeUriPath);
44+
45+
var absoluteNativePath = File(relativeNativePath).absolute.path;
46+
var result = await run(absoluteNativePath);
47+
if (Platform.isWindows) {
48+
// TODO(johnniwinther,rnystrom): Support absolute paths on Windows.
49+
if (!result) {
50+
print('Error: Expected failure on Windows. '
51+
'Update test to expect success on all platforms.');
52+
errorFound = true;
53+
} else {
54+
print('Error on Windows is expected.');
55+
}
56+
} else {
57+
errorFound |= result;
58+
}
59+
60+
if (errorFound) {
61+
print('----------------------------------------------------------------');
62+
throw 'Error found!';
63+
}
64+
} finally {
65+
// Restore original test content.
66+
testFile.writeAsStringSync(testContent);
67+
}
68+
}
69+
70+
Future<bool> run(String input) async {
71+
var executable = Platform.resolvedExecutable;
72+
var arguments = [...commonArguments, input];
73+
print('--------------------------------------------------------------------');
74+
print('Running: $executable ${arguments.join(' ')}');
75+
var process = await Process.start(executable, runInShell: true, arguments);
76+
var lines = <String>[];
77+
process.stdout.forEach((e) => lines.add(decodeUtf8(e)));
78+
process.stderr.forEach((e) => lines.add(decodeUtf8(e)));
79+
var exitCode = await process.exitCode;
80+
var output = lines.join();
81+
print(output);
82+
print('Exit code: $exitCode');
83+
84+
var hasError = false;
85+
if (!output.contains(expectedLine)) {
86+
print('Error: Expected output: $expectedLine');
87+
hasError = true;
88+
}
89+
if (!output.contains(expectedUpdateText)) {
90+
print('Error: Expected update: $expectedUpdateText');
91+
hasError = true;
92+
}
93+
if (exitCode != 0) {
94+
print('Error: Expected exit code: 0');
95+
hasError = true;
96+
}
97+
return hasError;
98+
}

pkg/test_runner/tool/update_static_error_tests.dart

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@ List<TestFile> _listFiles(List<String> pathGlobs,
188188
// or relative to the current directory.
189189
var root = pathGlob.startsWith("tests") ? "." : "tests";
190190

191+
// [Glob] doesn't handle Windows path delimiters, so we normalize it using
192+
// [Path]. [Glob] doesn't handle absolute Windows paths, though, so this
193+
// only supports the relative paths.
194+
pathGlob = Path.normalize(pathGlob);
195+
191196
for (var file in Glob(pathGlob, recursive: true).listSync(root: root)) {
192197
if (file is! File) continue;
193198

@@ -404,7 +409,7 @@ Future<Map<TestFile, List<StaticError>>> _runCfe(
404409
result.stdout as String, parsedErrors, parsedErrors);
405410
for (var error in parsedErrors) {
406411
var testFile =
407-
testFiles.firstWhere((test) => test.path.toString() == error.path);
412+
testFiles.firstWhere((test) => test.path == Path(error.path));
408413
errors.putIfAbsent(testFile, () => []).add(error);
409414
}
410415
}
@@ -468,20 +473,21 @@ void _updateErrors(TestFile testFile, List<StaticError> errors,
468473
// Error expectations can be in imported libraries or part files. Iterate
469474
// over the set of paths that is the main file path plus all paths mentioned
470475
// in expectations, updating them.
471-
var paths = {testFile.path.toString(), for (var error in errors) error.path};
476+
var paths = {testFile.path, for (var error in errors) Path(error.path)};
472477

473478
for (var path in paths) {
474-
var file = File(path);
475-
var pathErrors = errors.where((e) => e.path == path).toList();
479+
var nativePath = path.toNativePath();
480+
var file = File(nativePath);
481+
var pathErrors = errors.where((e) => Path(e.path) == path).toList();
476482
var result = updateErrorExpectations(
477-
path, file.readAsStringSync(), pathErrors,
483+
nativePath, file.readAsStringSync(), pathErrors,
478484
remove: remove, includeContext: includeContext);
479485

480486
if (dryRun) {
481487
print(result);
482488
} else {
483489
file.writeAsString(result);
484-
print('- $path (${_plural(pathErrors, 'error')})');
490+
print('- $nativePath (${_plural(pathErrors, 'error')})');
485491
}
486492
}
487493
}

0 commit comments

Comments
 (0)