dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Run web static error tests on dart2js and DDC and validate output.

Browse Source 
If a static error test contains "[web]" error markers, this runs them
on DDC and dart2js, parses the output, and tests that the compilers
report the expected errors.

Change-Id: I8b846f75828a0078f61c2fc1f3da0a4a587349cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/158100
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
This commit is contained in:
Robert Nystrom
2020-08-12 23:15:38 +00:00
committed by commit-bot@chromium.org
parent 4cf4a05fa6
commit 1c5d3dfd4a
7 changed files with 146 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/test_runner/lib/src/command.dart
+3
View File
@@ -180,6 +180,9 @@ class CompilationCommand extends ProcessCommand {
if (displayName == 'precompiler' || displayName == 'app_jit') {
return VMCommandOutput(
this, exitCode, timedOut, stdout, stderr, time, pid);
} else if (displayName == 'dart2js') {
return Dart2jsCompilerCommandOutput(
this, exitCode, timedOut, stdout, stderr, time, compilationSkipped);
} else if (displayName == 'dartdevc') {
return DevCompilerCommandOutput(this, exitCode, timedOut, stdout, stderr,
time, compilationSkipped, pid);
pkg/test_runner/lib/src/command_output.dart
+93 -11
View File
@@ -967,7 +967,56 @@ class CompilationCommandOutput extends CommandOutput {
}
}
class DevCompilerCommandOutput extends CommandOutput {
class Dart2jsCompilerCommandOutput extends CompilationCommandOutput
with _StaticErrorOutput {
/// Matches the location and message of a dart2js error message, which looks
/// like:
///
/// tests/language_2/some_test.dart:9:3:
/// Error: Some message.
/// BadThing();
/// ^
///
/// The test runner only validates the main error message, and not the
/// suggested fixes, so we only parse the first line.
static final _errorRegexp =
RegExp(r"^([^:]+):(\d+):(\d+):\nError: (.*)$", multiLine: true);
Dart2jsCompilerCommandOutput(
Command command,
int exitCode,
bool timedOut,
List<int> stdout,
List<int> stderr,
Duration time,
bool compilationSkipped)
: super(command, exitCode, timedOut, stdout, stderr, time,
compilationSkipped);
@override
void _parseErrors() {
var errors = <StaticError>[];
_StaticErrorOutput._parseCfeErrors(
ErrorSource.web, _errorRegexp, decodeUtf8(stdout), errors);
errors.forEach(addError);
}
}
class DevCompilerCommandOutput extends CommandOutput with _StaticErrorOutput {
/// Matches the first line of a DDC error message. DDC prints errors to
/// stdout that look like:
///
/// org-dartlang-app:/tests/language_2/some_test.dart:7:21: Error: Some message.
/// Try fixing the code to be less bad.
/// var _ = <int>[if (1) 2];
/// ^
///
/// The test runner only validates the main error message, and not the
/// suggested fixes, so we only parse the first line.
static final _errorRegexp = RegExp(
r"^org-dartlang-app:/([^:]+):(\d+):(\d+): Error: (.*)$",
multiLine: true);
DevCompilerCommandOutput(
Command command,
int exitCode,
@@ -985,6 +1034,11 @@ class DevCompilerCommandOutput extends CommandOutput {
if (hasTimedOut) return Expectation.timeout;
if (hasNonUtf8) return Expectation.nonUtf8Error;
// If it's a static error test, validate the exact errors.
if (testCase.testFile.isStaticErrorTest) {
return _validateExpectedErrors(testCase);
}
// Handle errors / missing errors
if (testCase.hasCompileError) {
return exitCode == 0
@@ -1001,9 +1055,24 @@ class DevCompilerCommandOutput extends CommandOutput {
if (hasCrashed) return Expectation.crash;
if (hasTimedOut) return Expectation.timeout;
if (hasNonUtf8) return Expectation.nonUtf8Error;
// If it's a static error test, validate the exact errors.
if (testCase.testFile.isStaticErrorTest) {
return _validateExpectedErrors(testCase);
}
if (exitCode != 0) return Expectation.compileTimeError;
return Expectation.pass;
}
@override
void _parseErrors() {
var errors = <StaticError>[];
_StaticErrorOutput._parseCfeErrors(
ErrorSource.web, _errorRegexp, decodeUtf8(stdout), errors);
errors.forEach(addError);
}
}
class VMKernelCompilationCommandOutput extends CompilationCommandOutput {
@@ -1178,13 +1247,8 @@ class FastaCommandOutput extends CompilationCommandOutput
with _StaticErrorOutput {
static void parseErrors(String stdout, List<StaticError> errors,
[List<StaticError> warnings]) {
for (var match in _errorRegexp.allMatches(stdout)) {
var line = int.parse(match.group(2));
var column = int.parse(match.group(3));
var message = match.group(4);
errors.add(
StaticError({ErrorSource.cfe: message}, line: line, column: column));
}
_StaticErrorOutput._parseCfeErrors(
ErrorSource.cfe, _errorRegexp, stdout, errors);
}
/// Matches the first line of a Fasta error message. Fasta prints errors to
@@ -1222,6 +1286,19 @@ class FastaCommandOutput extends CompilationCommandOutput
/// Mixin for outputs from a command that implement a Dart front end which
/// reports static errors.
mixin _StaticErrorOutput on CommandOutput {
/// Parses compile errors reported by CFE using the given [regExp] and adds
/// them to [errors] as coming from [errorSource].
static void _parseCfeErrors(ErrorSource errorSource, RegExp regExp,
String stdout, List<StaticError> errors) {
for (var match in regExp.allMatches(stdout)) {
var line = int.parse(match.group(2));
var column = int.parse(match.group(3));
var message = match.group(4);
errors
.add(StaticError({errorSource: message}, line: line, column: column));
}
}
/// Reported static errors, parsed from [stderr].
List<StaticError> get errors {
if (!_parsedErrors) {
@@ -1310,9 +1387,14 @@ mixin _StaticErrorOutput on CommandOutput {
Expectation _validateExpectedErrors(TestCase testCase,
[OutputWriter writer]) {
// Filter out errors that aren't for this configuration.
var errorSource = testCase.configuration.compiler == Compiler.dart2analyzer
? ErrorSource.analyzer
: ErrorSource.cfe;
var errorSource = {
Compiler.dart2analyzer: ErrorSource.analyzer,
Compiler.dart2js: ErrorSource.web,
Compiler.dartdevc: ErrorSource.web,
Compiler.fasta: ErrorSource.cfe
}[testCase.configuration.compiler];
assert(errorSource != null);
var expected = testCase.testFile.expectedErrors
.where((error) => error.hasError(errorSource));
pkg/test_runner/lib/src/test_file.dart
+8
View File
@@ -72,6 +72,14 @@ abstract class _TestFileBase {
/// is skipped on configurations that are not purely front end.
bool get isStaticErrorTest => expectedErrors.isNotEmpty;
/// If the test contains any web-specific (`[web]`) static error expectations,
/// then it's a "web static error test".
///
/// These tests exist to validate that a Dart web compiler reports the right
/// expected errors.
bool get isWebStaticErrorTest =>
expectedErrors.any((error) => error.hasError(ErrorSource.web));
/// If the tests has no static error expectations, or all of the expectations
/// are warnings, then the test tests runtime semantics.
///
pkg/test_runner/lib/src/test_suite.dart
+15 -16
View File
@@ -165,7 +165,19 @@ abstract class TestSuite {
}
}
return true;
// Normal runtime tests are always run.
if (testFile.isRuntimeTest) return true;
// Tests of web-specific static errors are run on web compilers.
if (testFile.isWebStaticErrorTest &&
(configuration.compiler == Compiler.dart2js ||
configuration.compiler == Compiler.dartdevc)) {
return true;
}
// Other static error tests are run on front-end-only configurations.
return configuration.compiler == Compiler.dart2analyzer ||
configuration.compiler == Compiler.fasta;
}
/// Whether a test with [expectations] should be skipped under the current
@@ -557,19 +569,6 @@ class StandardTestSuite extends TestSuite {
/// options.
void _testCasesFromTestFile(
TestFile testFile, ExpectationSet expectations, TestCaseEvent onTest) {
// TODO(rnystrom): Skipping this here is a little unusual because most
// skips are handled in _addTestCase(). However, if the configuration
// is running on a browser, calling _addTestCase() will try to create
// a set of commands which ultimately causes an exception in
// DummyRuntimeConfiguration. This avoids that.
// If the test only has static expectations, skip it on any configurations
// that are not purely front ends.
if (!testFile.isRuntimeTest &&
configuration.compiler != Compiler.dart2analyzer &&
configuration.compiler != Compiler.fasta) {
return;
}
// The configuration must support everything the test needs.
if (!configuration.supportedFeatures.containsAll(testFile.requirements)) {
return;
@@ -690,7 +689,7 @@ class StandardTestSuite extends TestSuite {
commands.addAll(compilationArtifact.commands);
}
if (testFile.hasCompileError &&
if ((testFile.hasCompileError || testFile.isStaticErrorTest) &&
compilerConfiguration.hasCompiler &&
!compilerConfiguration.runRuntimeDespiteMissingCompileTimeError) {
// Do not attempt to run the compiled result. A compilation
@@ -744,9 +743,9 @@ class StandardTestSuite extends TestSuite {
return "/$prefixDartDir/$fileRelativeToDartDir";
}
// Unreachable.
print("Cannot create URL for path $file. Not in build or dart directory.");
exit(1);
throw "unreachable";
}
String _uriForBrowserTest(String pathComponent) {
tests/lib/js/constructor_parameters_static_test.dart
+11 -11
View File
@@ -17,10 +17,10 @@ import 'package:expect/expect.dart';
class Foo {
external Foo({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external factory Foo.fooFactory({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// ^
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
@JS()
@@ -28,21 +28,21 @@ class Foo {
class Bar {
external Bar({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
// Factories of an anonymous class can only contain named parameters.
external factory Bar.barFactoryPositional(int? a);
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Factory constructors for @anonymous JS interop classes should not contain any positional parameters.
external factory Bar.barFactoryOptional([int? a]);
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Factory constructors for @anonymous JS interop classes should not contain any positional parameters.
external factory Bar.barFactoryMixedOptional(int? a, [int? b]);
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Factory constructors for @anonymous JS interop classes should not contain any positional parameters.
external factory Bar.barFactoryMixedNamed(int? a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Factory constructors for @anonymous JS interop classes should not contain any positional parameters.
// Named parameters are okay only for factories of an anonymous class.
external factory Bar.barFactoryNamed({int? a});
@@ -52,10 +52,10 @@ class Bar {
abstract class Baz {
external Baz({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external factory Baz.bazFactory({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// ^
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
main() {}
tests/lib/js/js_annotation_static_test.dart
+6 -6
View File
@@ -17,27 +17,27 @@ class Foo {
@JS()
external Foo(int bar);
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
@JS()
external factory Foo.fooFactory();
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
@JS()
external int get bar;
// ^^^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
@JS()
external set bar(int val);
// ^^^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
@JS()
external int baz();
// ^^^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
@JS()
external static int bazStatic();
// ^^^^^^^^^
// [web] TODO(srujzs): Add error once supported.
// [web] Member has a JS interop annotation but the enclosing class does not.
}
@JS()
tests/lib/js/parameters_static_test.dart
+10 -10
View File
@@ -16,48 +16,48 @@ import 'package:expect/expect.dart';
class Foo {
external int singleNamedArg({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external int mixedNamedArgs(int a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
@JS()
class Bar {
external static int singleNamedArg({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external static int mixedNamedArgs(int a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
external int singleNamedArg({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external int mixedNamedArgs(int a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
@JS()
@anonymous
class Baz {
external int singleNamedArg({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external int mixedNamedArgs(int a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
@JS()
abstract class Qux {
external int singleNamedArg({int? a});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
external int mixedNamedArgs(int a, {int? b});
// ^
// [web] TODO(srujzs): Add error once supported.
// [web] Named parameters for JS interop functions are only allowed in a factory constructor of an @anonymous JS class.
}
main() {}