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

Remove support for "@compile-error" and similar markers in tests.

Browse source 
These were added during the Dart 2 test migration as a way to indicate
why a test was expected to fail. But, like negative tests, they have
very poorly granularity.

Static error tests are strictly superior.

The co19 tests have been migrated off of these markers for a couple of
years, so our own language tests were the only holdouts. I see no uses
of "@syntax-error", "@runtime-error", or "@static-warning". I have now
migrated all of the tests that contained "@compile-error" to be proper
static error tests.

This simplifies the test runner and makes our tests more precise.

Fix #45634.

Change-Id: I0f46d110b6f322d98187e734195ecba7524574af
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/296720
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Auto-Submit: Bob Nystrom <rnystrom@google.com>
This commit is contained in:
Robert Nystrom 2023-04-24 18:34:59 +00:00 committed by Commit Queue
parent 5978b607d2
commit 988642a94e
3 changed files with 10 additions and 93 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -199,11 +199,6 @@ class TestFile extends _TestFileBase {
ddcOptions: [],
dartOptions: [],
packages: null,
hasSyntaxError: false,
hasCompileError: false,
hasRuntimeError: false,
hasStaticWarning: false,
hasCrash: false,
isMultitest: false,
sharedObjects: [],
otherResources: [],
@ -297,32 +292,6 @@ class TestFile extends _TestFileBase {
var isMultitest = _multitestRegExp.hasMatch(contents);
// TODO(rnystrom): During the migration of the existing tests to Dart 2.0,
// we have a number of tests that used to both generate static type warnings
// and also validate some runtime behavior in an implementation that
// ignores those warnings. Those warnings are now errors. The test code
// validates the runtime behavior can and should be removed, but the code
// that causes the static warning should still be preserved since that is
// part of our coverage of the static type system.
//
// The test needs to indicate that it should have a static error. We could
// put that in the status file, but that makes it confusing because it
// would look like implementations that *don't* report the error are more
// correct. Eventually, we want to have a notation similar to what front_end
// is using for the inference tests where we can put a comment inside the
// test that says "This specific static error should be reported right by
// this token."
//
// That system isn't in place yet, so we do a crude approximation here in
// test.dart. If a test contains `/*@compile-error=`, which matches the
// beginning of the tag syntax that front_end uses, then we assume that
// this test must have a static error somewhere in it.
//
// Redo this code once we have a more precise test framework for detecting
// and locating these errors.
var hasSyntaxError = contents.contains("@syntax-error");
var hasCompileError = hasSyntaxError || contents.contains("@compile-error");
List<StaticError> errorExpectations;
try {
errorExpectations = StaticError.parseExpectations(contents);
@ -335,11 +304,6 @@ class TestFile extends _TestFileBase {
packages: packages,
environment: environment,
isMultitest: isMultitest,
hasSyntaxError: hasSyntaxError,
hasCompileError: hasCompileError,
hasRuntimeError: contents.contains("@runtime-error"),
hasStaticWarning: contents.contains("@static-warning"),
hasCrash: false,
requirements: requirements,
sharedOptions: sharedOptions,
dartOptions: dartOptions,
@ -355,14 +319,14 @@ class TestFile extends _TestFileBase {
/// A special fake test file for representing a VM unit test written in C++.
TestFile.vmUnitTest(
{required this.hasSyntaxError,
required this.hasCompileError,
{required this.hasCompileError,
required this.hasRuntimeError,
required this.hasStaticWarning,
required this.hasCrash})
: packages = null,
environment = {},
isMultitest = false,
hasSyntaxError = false,
hasStaticWarning = false,
requirements = [],
sharedOptions = [],
dartOptions = [],
@ -380,11 +344,6 @@ class TestFile extends _TestFileBase {
{this.packages,
required this.environment,
required this.isMultitest,
required this.hasSyntaxError,
required this.hasCompileError,
required this.hasRuntimeError,
required this.hasStaticWarning,
required this.hasCrash,
required this.requirements,
required this.sharedOptions,
required this.dartOptions,
@ -396,7 +355,12 @@ class TestFile extends _TestFileBase {
required this.otherResources,
required this.experiments,
this.isVmIntermediateLanguageTest = false})
: super(suiteDirectory, path, expectedErrors) {
: hasSyntaxError = false,
hasCompileError = false,
hasRuntimeError = false,
hasStaticWarning = false,
hasCrash = false,
super(suiteDirectory, path, expectedErrors) {
assert(!isMultitest || dartOptions.isEmpty);
}

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

@ -323,10 +323,8 @@ class VMTestSuite extends TestSuite {
// Update the new workflow based expectations to include [testExpectation].
var testFile = TestFile.vmUnitTest(
hasSyntaxError: false,
hasCompileError: testExpectation == Expectation.compileTimeError,
hasRuntimeError: testExpectation == Expectation.runtimeError,
hasStaticWarning: false,
hasCrash: testExpectation == Expectation.crash);
var filename = configuration.architecture == Architecture.x64 ||
configuration.architecture == Architecture.x64c
@ -464,10 +462,8 @@ class FfiTestSuite extends TestSuite {
// Update the new workflow based expectations to include [testExpectation].
final testFile = TestFile.vmUnitTest(
hasSyntaxError: false,
hasCompileError: testExpectation == Expectation.compileTimeError,
hasRuntimeError: testExpectation == Expectation.runtimeError,
hasStaticWarning: false,
hasCrash: testExpectation == Expectation.crash);
final args = [

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

@ -29,7 +29,6 @@ void main() {
testParsePackages();
testParseExperiments();
testParseMultitest();
testParseErrorFlags();
testParseErrorExpectations();
testParseContextMessages();
testIsRuntimeTest();
@ -258,44 +257,6 @@ void testParseMultitest() {
Expect.isTrue(file.isMultitest);
}
void testParseErrorFlags() {
// Not present.
var file = parseTestFile("");
Expect.isFalse(file.hasSyntaxError);
Expect.isFalse(file.hasCompileError);
Expect.isFalse(file.hasRuntimeError);
Expect.isFalse(file.hasStaticWarning);
Expect.isFalse(file.hasCrash);
file = parseTestFile("@syntax\-error");
Expect.isTrue(file.hasSyntaxError);
Expect.isTrue(file.hasCompileError); // Note: true.
Expect.isFalse(file.hasRuntimeError);
Expect.isFalse(file.hasStaticWarning);
Expect.isFalse(file.hasCrash);
file = parseTestFile("@compile\-error");
Expect.isFalse(file.hasSyntaxError);
Expect.isTrue(file.hasCompileError);
Expect.isFalse(file.hasRuntimeError);
Expect.isFalse(file.hasStaticWarning);
Expect.isFalse(file.hasCrash);
file = parseTestFile("@runtime\-error");
Expect.isFalse(file.hasSyntaxError);
Expect.isFalse(file.hasCompileError);
Expect.isTrue(file.hasRuntimeError);
Expect.isFalse(file.hasStaticWarning);
Expect.isFalse(file.hasCrash);
file = parseTestFile("@static\-warning");
Expect.isFalse(file.hasSyntaxError);
Expect.isFalse(file.hasCompileError);
Expect.isFalse(file.hasRuntimeError);
Expect.isTrue(file.hasStaticWarning);
Expect.isFalse(file.hasCrash);
}
void testParseErrorExpectations() {
// No errors.
expectParseErrorExpectations("""
@ -827,11 +788,7 @@ void testShardHash() {
Expect.equals(
0,
TestFile.vmUnitTest(
hasCompileError: false,
hasCrash: false,
hasRuntimeError: false,
hasStaticWarning: false,
hasSyntaxError: false)
hasCompileError: false, hasCrash: false, hasRuntimeError: false)
.shardHash);
}