Skip to content

Commit 409ecc7

Browse files
sgrekhovCommit Queue
sgrekhov
authored and
Commit Queue
committed
[Test runner] Enable warnings for dart2js, dart2wasm and DDC
Enable support warnings in tests for `dart2js`, `dart2wasm` and DDC compilers Closes #61289 Change-Id: Ifcbcaca3deb0444e68bddfef7da2e4bdcb385e0d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/448960 Reviewed-by: Alexander Thomas <[email protected]> Commit-Queue: Alexander Thomas <[email protected]> Reviewed-by: Erik Ernst <[email protected]>
1 parent ced0ee9 commit 409ecc7

File tree

7 files changed

+126
-62
lines changed

7 files changed

+126
-62
lines changed

‎pkg/test_runner/lib/src/command_output.dart‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -876,9 +876,10 @@ class CompilationCommandOutput extends CommandOutput {
876876

877877
class Dart2jsCompilerCommandOutput extends CompilationCommandOutput
878878
with _StaticErrorOutput {
879-
static void parseErrors(String stdout, List<StaticError> errors) {
879+
static void parseErrors(String stdout, List<StaticError> errors,
880+
[List<StaticError>? warnings]) {
880881
_StaticErrorOutput._parseCfeErrors(
881-
ErrorSource.web, _errorRegexp, stdout, errors);
882+
ErrorSource.web, _errorRegexp, stdout, errors, warnings);
882883
}
883884

884885
/// Matches the location and message of a dart2js error message, which looks
@@ -893,24 +894,28 @@ class Dart2jsCompilerCommandOutput extends CompilationCommandOutput
893894
/// suggested fixes, so we only parse the first line.
894895
// TODO(rnystrom): Support validating context messages.
895896
static final _errorRegexp =
896-
RegExp(r"^([^:\n\r]+):(\d+):(\d+):\n(Error): (.*)$", multiLine: true);
897+
RegExp(r"^([^:\n\r]+):(\d+):(\d+):\n(Error|Warning): (.*)$",
898+
multiLine: true);
897899

898900
Dart2jsCompilerCommandOutput(super.command, super.exitCode, super.timedOut,
899901
super.stdout, super.stderr, super.time, super.compilationSkipped);
900902

901903
@override
902904
void _parseErrors() {
903905
var errors = <StaticError>[];
904-
parseErrors(decodeUtf8(stdout), errors);
906+
var warnings = <StaticError>[];
907+
parseErrors(decodeUtf8(stdout), errors, warnings);
905908
errors.forEach(addError);
909+
warnings.forEach(addWarning);
906910
}
907911
}
908912

909913
class Dart2WasmCompilerCommandOutput extends CompilationCommandOutput
910914
with _StaticErrorOutput {
911-
static void parseErrors(String stdout, List<StaticError> errors) {
915+
static void parseErrors(String stdout, List<StaticError> errors,
916+
[List<StaticError>? warnings]) {
912917
_StaticErrorOutput._parseCfeErrors(
913-
ErrorSource.web, _errorRegexp, stdout, errors);
918+
ErrorSource.web, _errorRegexp, stdout, errors, warnings);
914919
}
915920

916921
/// Matches the location and message of a dart2wasm error message, which looks
@@ -924,7 +929,8 @@ class Dart2WasmCompilerCommandOutput extends CompilationCommandOutput
924929
/// suggested fixes, so we only parse the first line.
925930
// TODO(rnystrom): Support validating context messages.
926931
static final _errorRegexp =
927-
RegExp(r"^([^:\n\r]+):(\d+):(\d+): (Error): (.*)$", multiLine: true);
932+
RegExp(r"^([^:\n\r]+):(\d+):(\d+): (Error|Warning): (.*)$",
933+
multiLine: true);
928934

929935
Dart2WasmCompilerCommandOutput(super.command, super.exitCode, super.timedOut,
930936
super.stdout, super.stderr, super.time, super.compilationSkipped);
@@ -954,16 +960,19 @@ class Dart2WasmCompilerCommandOutput extends CompilationCommandOutput
954960
@override
955961
void _parseErrors() {
956962
var errors = <StaticError>[];
963+
var warnings = <StaticError>[];
957964
// We expect errors to be printed to `stderr` for dart2wasm.
958-
parseErrors(decodeUtf8(stderr), errors);
965+
parseErrors(decodeUtf8(stderr), errors, warnings);
959966
errors.forEach(addError);
967+
warnings.forEach(addWarning);
960968
}
961969
}
962970

963971
class DevCompilerCommandOutput extends CommandOutput with _StaticErrorOutput {
964-
static void parseErrors(String stdout, List<StaticError> errors) {
972+
static void parseErrors(String stdout, List<StaticError> errors,
973+
[List<StaticError>? warnings]) {
965974
_StaticErrorOutput._parseCfeErrors(
966-
ErrorSource.web, _errorRegexp, stdout, errors);
975+
ErrorSource.web, _errorRegexp, stdout, errors, warnings);
967976
}
968977

969978
/// Matches the first line of a DDC error message. DDC prints errors to
@@ -978,7 +987,7 @@ class DevCompilerCommandOutput extends CommandOutput with _StaticErrorOutput {
978987
/// suggested fixes, so we only parse the first line.
979988
// TODO(rnystrom): Support validating context messages.
980989
static final _errorRegexp = RegExp(
981-
r"^org-dartlang-app:/([^\n\r]+):(\d+):(\d+): (Error): (.*)$",
990+
r"^org-dartlang-app:/([^\n\r]+):(\d+):(\d+): (Error|Warning): (.*)$",
982991
multiLine: true);
983992

984993
DevCompilerCommandOutput(
@@ -1011,8 +1020,10 @@ class DevCompilerCommandOutput extends CommandOutput with _StaticErrorOutput {
10111020
@override
10121021
void _parseErrors() {
10131022
var errors = <StaticError>[];
1014-
parseErrors(decodeUtf8(stdout), errors);
1023+
var warnings = <StaticError>[];
1024+
parseErrors(decodeUtf8(stdout), errors, warnings);
10151025
errors.forEach(addError);
1026+
warnings.forEach(addWarning);
10161027
}
10171028
}
10181029

@@ -1242,7 +1253,7 @@ mixin _StaticErrorOutput on CommandOutput {
12421253
if (severity == "Error") {
12431254
errors.add(error);
12441255
} else {
1245-
warnings!.add(error);
1256+
warnings?.add(error);
12461257
}
12471258
previousError = error;
12481259
}

‎pkg/test_runner/test/update_error_cmdline_test.dart‎

Lines changed: 80 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,55 +6,87 @@ import 'dart:io';
66
import 'package:path/path.dart' as p;
77
import 'package:test_runner/src/utils.dart';
88

9-
const commonArguments = [
10-
'run',
11-
'pkg/test_runner/tool/update_static_error_tests.dart',
12-
'--update=cfe',
13-
];
9+
void main() async {
10+
const testNamePartsCfe = [
11+
'language',
12+
'compile_time_constant',
13+
'compile_time_constant_test'
14+
];
15+
const testNamePartsWeb = ['web', 'extension_type_assert_error_test'];
16+
const expectedLineCfe = 'Running CFE on 1 file...';
17+
final expectedUpdateTextCfe = '${testNamePartsCfe.last}.dart (3 errors)';
18+
final expectedLineCfeWeb =
19+
'Running dart2js on ${toFileName(toFileNameParts(testNamePartsCfe))}...';
20+
21+
final expectedLineWeb =
22+
'Running dart2js on ${toFileName(toFileNameParts(testNamePartsWeb))}...';
23+
final expectedUpdateTextWeb = '${testNamePartsWeb.last}.dart (1 error)';
1424

15-
const testNameParts = [
16-
'language',
17-
'compile_time_constant',
18-
'compile_time_constant_test'
19-
];
25+
await doTest(toFileNameParts(testNamePartsCfe), testNamePartsCfe,
26+
'--update=cfe', [expectedLineCfe], expectedUpdateTextCfe);
27+
await doTest(
28+
toFileNameParts(testNamePartsCfe),
29+
testNamePartsCfe,
30+
'--update=cfe,web',
31+
[expectedLineCfe, expectedLineCfeWeb],
32+
expectedUpdateTextCfe,
33+
runAll: false);
34+
await doTest(toFileNameParts(testNamePartsWeb), testNamePartsWeb,
35+
'--update=web', [expectedLineWeb], expectedUpdateTextWeb,
36+
runAll: false);
37+
await doTest(
38+
toFileNameParts(testNamePartsWeb),
39+
testNamePartsWeb,
40+
'--update=web,cfe',
41+
[expectedLineCfe, expectedLineWeb],
42+
expectedUpdateTextWeb,
43+
runAll: false);
44+
}
2045

21-
final fileNameParts = [
22-
'tests',
23-
...testNameParts.take(testNameParts.length - 1),
24-
'${testNameParts.last}.dart',
25-
];
46+
List<String> toFileNameParts(List<String> testNameParts) => [
47+
'tests',
48+
...testNameParts.take(testNameParts.length - 1),
49+
'${testNameParts.last}.dart',
50+
];
2651

27-
const expectedLine = 'Running CFE on 1 file...';
28-
final expectedUpdateText = '${testNameParts.last}.dart (3 errors)';
52+
String toFileName(List<String> fileNameParts) => p.joinAll(fileNameParts);
2953

30-
void main() async {
31-
var testFile = File(p.joinAll(fileNameParts));
54+
Future<void> doTest(List<String> fileNameParts, List<String> testNameParts,
55+
String target, List<String> expectedLines, String expectedUpdateText,
56+
{bool runAll = true}) async {
57+
var testFile = File(toFileName(fileNameParts));
3258
var testContent = testFile.readAsStringSync();
3359
try {
3460
var errorFound = false;
3561

3662
var testName = testNameParts.join('/');
37-
errorFound |= await run(testName);
63+
errorFound |=
64+
await run(testName, target, expectedLines, expectedUpdateText);
3865

39-
var relativeNativePath = p.joinAll(fileNameParts);
40-
errorFound |= await run(relativeNativePath);
66+
if (runAll) {
67+
var relativeNativePath = p.joinAll(fileNameParts);
68+
errorFound |= await run(
69+
relativeNativePath, target, expectedLines, expectedUpdateText);
4170

42-
var relativeUriPath = fileNameParts.join('/');
43-
errorFound |= await run(relativeUriPath);
71+
var relativeUriPath = fileNameParts.join('/');
72+
errorFound |=
73+
await run(relativeUriPath, target, expectedLines, expectedUpdateText);
4474

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;
75+
var absoluteNativePath = File(relativeNativePath).absolute.path;
76+
var result = await run(
77+
absoluteNativePath, target, expectedLines, expectedUpdateText);
78+
if (Platform.isWindows) {
79+
// TODO(johnniwinther,rnystrom): Support absolute paths on Windows.
80+
if (!result) {
81+
print('Error: Expected failure on Windows. '
82+
'Update test to expect success on all platforms.');
83+
errorFound = true;
84+
} else {
85+
print('Error on Windows is expected.');
86+
}
5387
} else {
54-
print('Error on Windows is expected.');
88+
errorFound |= result;
5589
}
56-
} else {
57-
errorFound |= result;
5890
}
5991

6092
if (errorFound) {
@@ -67,9 +99,15 @@ void main() async {
6799
}
68100
}
69101

70-
Future<bool> run(String input) async {
102+
Future<bool> run(String input, String target, List<String> expectedLines,
103+
String expectedUpdateText) async {
104+
const commonArguments = [
105+
'run',
106+
'pkg/test_runner/tool/update_static_error_tests.dart',
107+
];
108+
71109
var executable = Platform.resolvedExecutable;
72-
var arguments = [...commonArguments, input];
110+
var arguments = [...commonArguments, target, input];
73111
print('--------------------------------------------------------------------');
74112
print('Running: $executable ${arguments.join(' ')}');
75113
var process = await Process.start(executable, runInShell: true, arguments);
@@ -82,9 +120,11 @@ Future<bool> run(String input) async {
82120
print('Exit code: $exitCode');
83121

84122
var hasError = false;
85-
if (!output.contains(expectedLine)) {
86-
print('Error: Expected output: $expectedLine');
87-
hasError = true;
123+
for (var expectedLine in expectedLines) {
124+
if (!output.contains(expectedLine)) {
125+
print('Error: Expected output: $expectedLine');
126+
hasError = true;
127+
}
88128
}
89129
if (!output.contains(expectedUpdateText)) {
90130
print('Error: Expected update: $expectedUpdateText');

‎pkg/test_runner/tool/update_static_error_tests.dart‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,7 @@ Future<void> main(List<String> args) async {
133133
analyzerErrors = await _runAnalyzer(testFiles);
134134
}
135135

136-
// If we're inserting web errors, we also need to gather the CFE errors to
137-
// tell which web errors are web-specific.
138-
if (insertSources.contains(ErrorSource.cfe) ||
139-
insertSources.contains(ErrorSource.web)) {
136+
if (insertSources.contains(ErrorSource.cfe)) {
140137
cfeErrors = await _runCfe(testFiles);
141138
}
142139

@@ -408,9 +405,13 @@ Future<Map<TestFile, List<StaticError>>> _runCfe(
408405
FastaCommandOutput.parseErrors(
409406
result.stdout as String, parsedErrors, parsedErrors);
410407
for (var error in parsedErrors) {
408+
// If an error occurs in a file that is included in the current one, then
409+
// `error.path` may not be found, and `testFile` will be null.
411410
var testFile =
412-
testFiles.firstWhere((test) => test.path == Path(error.path));
413-
errors.putIfAbsent(testFile, () => []).add(error);
411+
testFiles.firstWhereOrNull((test) => test.path == Path(error.path));
412+
if (testFile != null) {
413+
errors.putIfAbsent(testFile, () => []).add(error);
414+
}
414415
}
415416
}
416417

@@ -442,7 +443,8 @@ Future<List<StaticError>> _runDart2jsOnFile(
442443
]);
443444

444445
var errors = <StaticError>[];
445-
Dart2jsCompilerCommandOutput.parseErrors(result.stdout as String, errors);
446+
Dart2jsCompilerCommandOutput.parseErrors(
447+
result.stdout as String, errors, errors);
446448

447449
// We only want the web-specific errors from dart2js, so filter out any errors
448450
// that are also reported by the CFE.

‎tests/lib/js/export/static_interop_mock/subtype_overrides_test.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ extension ComplexAndOptionalInteropMethodsExtension
149149
external B Function(B _) nestedTypes(List<B> arg1, Map<Set<B>, B> arg2);
150150
external B optional(B b, [B? b2]);
151151
external B optionalSubtype(B b, [B b2 = const B()]);
152+
// ^
153+
// [web] Initializers for parameters are ignored on static interop external functions.
152154
}
153155

154156
@JSExport()

‎tests/lib/js/export/validation_test.dart‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,15 +190,18 @@ void testCollisions() {
190190
createJSInteropWrapper(GetSetNoCollision());
191191
}
192192

193-
// Class annotation values are warnings, not values, so they don't show up in
194-
// static error tests.
193+
// Class annotation values are warnings
195194
@JSExport('Invalid')
196195
class ClassWithValue {
196+
// ^
197+
// [web] The value in the `@JSExport` annotation on the class or mixin 'ClassWithValue' will be ignored.
197198
int get getSet => throw '';
198199
}
199200

200201
@JSExport('Invalid')
201202
mixin MixinWithValue {
203+
// ^
204+
// [web] The value in the `@JSExport` annotation on the class or mixin 'MixinWithValue' will be ignored.
202205
int get getSet => throw '';
203206
}
204207

‎tests/lib/js/external_nonjs_static_test.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ extension ExtensionNative on HtmlElement {
165165

166166
external method();
167167
external optionalParameterMethod([int? a, int b = 0]);
168+
// ^
169+
// [web] Initializers for parameters are ignored on static interop external functions.
168170

169171
nonExternalMethod() => 1;
170172
static nonExternalStaticMethod() => 2;

‎tests/lib/js/external_static_test.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ extension ExtensionJS on JSClass {
162162

163163
external method();
164164
external optionalParameterMethod([int? a, int b = 0]);
165+
// ^
166+
// [web] Initializers for parameters are ignored on static interop external functions.
165167

166168
@JS('fieldAnnotation')
167169
external var annotatedField;
@@ -233,6 +235,8 @@ extension ExtensionNative on HtmlElement {
233235

234236
external method();
235237
external optionalParameterMethod([int? a, int b = 0]);
238+
// ^
239+
// [web] Initializers for parameters are ignored on static interop external functions.
236240

237241
nonExternalMethod() => 1;
238242
static nonExternalStaticMethod() => 2;

0 commit comments

Comments
 (0)