languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 09:58:32 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't skip warning-only static error tests on runtime configurations.

Browse source 
If a static error test only has expectations for *warnings* then it
still has well-defined runtime semantics that we want to test.

This distinguishes those tests and runs them on non-front-end
configurations.

Change-Id: I41b8d84a229ba53ad0db0271b28a9b9482ad582d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155305
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: Karl Klose <karlklose@google.com>
Reviewed-by: Nicholas Shahan <nshahan@google.com>
This commit is contained in:
Robert Nystrom 2020-08-06 02:22:56 +00:00 committed by commit-bot@chromium.org
parent b5a3292260
commit e3c5c591d1
6 changed files with 161 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

57
pkg/test_runner/lib/src/static_error.dart
View file

@ -55,6 +55,44 @@ class ErrorSource {
class StaticError implements Comparable<StaticError> {
static const _unspecified = "unspecified";
/// The error codes for all of the analyzer errors that are non-fatal
/// warnings.
///
/// We can't rely on the type ("STATIC_WARNING", etc.) because for historical
/// reasons the "warning" types contain a large number of actual compile
/// errors.
// TODO(rnystrom): This list was generated on 2020/07/24 based on the list
// of error codes in sdk/pkg/analyzer/lib/error/error.dart. Is there a more
// systematic way to handle this?
static const _analyzerWarningCodes = {
"STATIC_WARNING.ANALYSIS_OPTION_DEPRECATED",
"STATIC_WARNING.INCLUDE_FILE_NOT_FOUND",
"STATIC_WARNING.INCLUDED_FILE_WARNING",
"STATIC_WARNING.INVALID_OPTION",
"STATIC_WARNING.INVALID_SECTION_FORMAT",
"STATIC_WARNING.SPEC_MODE_REMOVED",
"STATIC_WARNING.UNRECOGNIZED_ERROR_CODE",
"STATIC_WARNING.UNSUPPORTED_OPTION_WITH_LEGAL_VALUE",
"STATIC_WARNING.UNSUPPORTED_OPTION_WITH_LEGAL_VALUES",
"STATIC_WARNING.UNSUPPORTED_OPTION_WITHOUT_VALUES",
"STATIC_WARNING.UNSUPPORTED_VALUE",
"STATIC_WARNING.CAMERA_PERMISSIONS_INCOMPATIBLE",
"STATIC_WARNING.NO_TOUCHSCREEN_FEATURE",
"STATIC_WARNING.NON_RESIZABLE_ACTIVITY",
"STATIC_WARNING.PERMISSION_IMPLIES_UNSUPPORTED_HARDWARE",
"STATIC_WARNING.SETTING_ORIENTATION_ON_ACTIVITY",
"STATIC_WARNING.UNSUPPORTED_CHROME_OS_FEATURE",
"STATIC_WARNING.UNSUPPORTED_CHROME_OS_HARDWARE",
"STATIC_WARNING.DEAD_NULL_AWARE_EXPRESSION",
"STATIC_WARNING.INVALID_NULL_AWARE_OPERATOR",
"STATIC_WARNING.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_NAMED",
"STATIC_WARNING.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_POSITIONAL",
"STATIC_WARNING.MISSING_ENUM_CONSTANT_IN_SWITCH",
"STATIC_WARNING.UNNECESSARY_NON_NULL_ASSERTION",
"STATIC_WARNING.TOP_LEVEL_INSTANCE_GETTER",
"STATIC_WARNING.TOP_LEVEL_INSTANCE_METHOD",
};
/// Parses the set of static error expectations defined in the Dart source
/// file [source].
static List<StaticError> parseExpectations(String source) =>
@ -296,6 +334,25 @@ class StaticError implements Comparable<StaticError> {
return result;
}
/// Whether this error is only considered a warning on all front ends that
/// report it.
bool get isWarning {
var analyzer = _errors[ErrorSource.analyzer];
if (analyzer != null && !_analyzerWarningCodes.contains(analyzer)) {
return false;
}
// TODO(42787): Once CFE starts reporting warnings, encode that in the
// message somehow and then look for it here.
if (hasError(ErrorSource.cfe)) return false;
// TODO(rnystrom): If the web compilers report warnings, encode that in the
// message somehow and then look for it here.
if (hasError(ErrorSource.web)) return false;
return true;
}
String toString() {
var result = "Error at $location";

16
pkg/test_runner/lib/src/test_file.dart
View file

@ -64,14 +64,24 @@ abstract class _TestFileBase {
/// generated from a multitest. Otherwise, returns an empty string.
String get multitestKey;
/// If the text contains static error expectations, it's a "static error
/// If the test contains static error expectations, it's a "static error
/// test".
///
/// These tests exist to validate that a front end reports the right static
/// errors. They are skipped on configurations that don't intend to test
/// static error reporting.
/// errors. Unless the expected errors are all warnings, a static error test
/// is skipped on configurations that are not purely front end.
bool get isStaticErrorTest => expectedErrors.isNotEmpty;
/// If the tests has no static error expectations, or all of the expectations
/// are warnings, then the test tests runtime semantics.
///
/// Note that this is *not* the negation of [isStaticErrorTest]. A test that
/// contains only warning expectations is both a static "error" test and a
/// runtime test. The test runner will validate that the front ends produce
/// the expected warnings *and* that a runtime also correctly executes the
/// test.
bool get isRuntimeTest => expectedErrors.every((error) => error.isWarning);
/// A hash code used to spread tests across shards.
int get shardHash {
// The VM C++ unit tests have a special fake TestFile with no suite

10
pkg/test_runner/lib/src/test_suite.dart
View file

@ -557,16 +557,14 @@ class StandardTestSuite extends TestSuite {
/// options.
void _testCasesFromTestFile(
TestFile testFile, ExpectationSet expectations, TestCaseEvent onTest) {
// Static error tests are skipped on every implementation except analyzer
// and Fasta.
// TODO(rnystrom): Should other configurations that use CFE support static
// error tests?
// 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 (testFile.isStaticErrorTest &&
// 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;
@ -579,7 +577,7 @@ class StandardTestSuite extends TestSuite {
var expectationSet = expectations.expectations(testFile.name);
if (configuration.compilerConfiguration.hasCompiler &&
(testFile.hasCompileError || testFile.isStaticErrorTest)) {
(testFile.hasCompileError || !testFile.isRuntimeTest)) {
// If a compile-time error is expected, and we're testing a
// compiler, we never need to attempt to run the program (in a
// browser or otherwise).

37
pkg/test_runner/test/static_error_test.dart
View file

@ -11,6 +11,7 @@ import 'utils.dart';
void main() {
testHasError();
testErrorFor();
testIsWarning();
testIsSpecifiedFor();
testCompareTo();
testDescribeDifferences();
@ -78,6 +79,42 @@ void testErrorFor() {
Expect.equals("Web.", all.errorFor(ErrorSource.web));
}
void testIsWarning() {
// Analyzer only.
Expect.isTrue(
makeError(analyzerError: "STATIC_WARNING.INVALID_OPTION").isWarning);
Expect.isFalse(
makeError(analyzerError: "SYNTACTIC_ERROR.MISSING_FUNCTION_BODY")
.isWarning);
Expect.isFalse(makeError(
analyzerError: "COMPILE_TIME_ERROR.NOT_ENOUGH_POSITIONAL_ARGUMENTS")
.isWarning);
// CFE only.
Expect.isFalse(makeError(cfeError: "Any error message.").isWarning);
// Web only.
Expect.isFalse(makeError(webError: "Any error message.").isWarning);
// Multiple front ends.
Expect.isFalse(makeError(
analyzerError: "STATIC_WARNING.INVALID_OPTION",
cfeError: "Any error message.")
.isWarning);
Expect.isFalse(
makeError(cfeError: "Any error message.", webError: "Any error message.")
.isWarning);
Expect.isFalse(makeError(
analyzerError: "STATIC_WARNING.INVALID_OPTION",
webError: "Any error message.")
.isWarning);
Expect.isFalse(makeError(
analyzerError: "STATIC_WARNING.INVALID_OPTION",
cfeError: "Any error message.",
webError: "Any error message.")
.isWarning);
}
void testIsSpecifiedFor() {
var specifiedAll = makeError(
line: 1,

48
pkg/test_runner/test/test_file_test.dart
View file

@ -28,6 +28,7 @@ void main() {
testParseMultitest();
testParseErrorFlags();
testParseErrorExpectations();
testIsRuntimeTest();
testName();
testMultitest();
testShardHash();
@ -590,6 +591,53 @@ int i = "s";
""");
}
void testIsRuntimeTest() {
// No static errors at all.
var file = parseTestFile("");
Expect.isTrue(file.isRuntimeTest);
// Only warnings.
file = parseTestFile("""
int i = "s";
/\/ ^^^
/\/ [analyzer] STATIC_WARNING.INVALID_OPTION
/\/ ^^^
/\/ [analyzer] STATIC_WARNING.INVALID_OPTION
""");
Expect.isTrue(file.isRuntimeTest);
// Errors.
file = parseTestFile("""
int i = "s";
/\/ ^^^
/\/ [analyzer] COMPILE_TIME_ERROR.NOT_ENOUGH_POSITIONAL_ARGUMENTS
""");
Expect.isFalse(file.isRuntimeTest);
file = parseTestFile("""
int i = "s";
/\/ ^^^
/\/ [cfe] Error message.
""");
Expect.isFalse(file.isRuntimeTest);
file = parseTestFile("""
int i = "s";
/\/ ^^^
/\/ [web] Error message.
""");
Expect.isFalse(file.isRuntimeTest);
// Mixed errors and warnings.
file = parseTestFile("""
int i = "s";
/\/ ^^^
/\/ [analyzer] STATIC_WARNING.INVALID_OPTION
/\/ [cfe] Error message.
""");
Expect.isFalse(file.isRuntimeTest);
}
void testName() {
// Immediately inside suite.
var file = TestFile.parse(Path("suite").absolute,

4
pkg/test_runner/test/utils.dart
View file

@ -26,8 +26,8 @@ StandardTestSuite makeTestSuite(
_MockTestSuite(configuration, testFiles);
StaticError makeError(
{int line,
int column,
{int line = 1,
int column = 2,
int length,
String analyzerError,
String cfeError,