Skip to content

Commit 361d962

Browse files
munificentcommit-bot@chromium.org
munificent
authored and
commit-bot@chromium.org
committed
Remove support for negative tests from the test runner.
- Remove the `retry` field from `BrowserTestCommand` since it isn't read and the only criteria to assign it was whether a test was a negative test Change-Id: I4ff431ccf3fcd4c754fccbfdc6c89fc1ce5fa66f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/125841 Reviewed-by: William Hesse <whesse@google.com> Commit-Queue: Bob Nystrom <rnystrom@google.com> Auto-Submit: Bob Nystrom <rnystrom@google.com>
1 parent ec81b3e commit 361d962

File tree

6 files changed

+19
-74
lines changed

6 files changed

+19
-74
lines changed

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

+3-6
Original file line numberDiff line numberDiff line change
@@ -388,28 +388,25 @@ class BrowserTestCommand extends Command {
388388
Runtime get browser => configuration.runtime;
389389
final String url;
390390
final TestConfiguration configuration;
391-
final bool retry;
392391

393-
BrowserTestCommand(this.url, this.configuration, {this.retry, int index = 0})
392+
BrowserTestCommand(this.url, this.configuration, {int index = 0})
394393
: super._(configuration.runtime.name, index: index);
395394

396395
BrowserTestCommand indexedCopy(int index) =>
397-
BrowserTestCommand(url, configuration, retry: retry, index: index);
396+
BrowserTestCommand(url, configuration, index: index);
398397

399398
void _buildHashCode(HashCodeBuilder builder) {
400399
super._buildHashCode(builder);
401400
builder.addJson(browser.name);
402401
builder.addJson(url);
403402
builder.add(configuration);
404-
builder.add(retry);
405403
}
406404

407405
bool _equal(BrowserTestCommand other) =>
408406
super._equal(other) &&
409407
browser == other.browser &&
410408
url == other.url &&
411-
identical(configuration, other.configuration) &&
412-
retry == other.retry;
409+
identical(configuration, other.configuration);
413410

414411
String get reproductionCommand {
415412
var parts = [

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

+13-42
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class CommandOutput {
4747
Expectation result(TestCase testCase) {
4848
if (hasCrashed) return Expectation.crash;
4949
if (hasTimedOut) return Expectation.timeout;
50-
if (hasFailed(testCase)) return Expectation.fail;
50+
if (_didFail(testCase)) return Expectation.fail;
5151
if (hasNonUtf8) return Expectation.nonUtf8Error;
5252

5353
return Expectation.pass;
@@ -111,23 +111,8 @@ class CommandOutput {
111111
return !hasTimedOut && exitCode == 0;
112112
}
113113

114-
// Reverse result of a negative test.
115-
bool hasFailed(TestCase testCase) {
116-
return testCase.isNegative ? !_didFail(testCase) : _didFail(testCase);
117-
}
118-
119114
bool get hasNonUtf8 => exitCode == nonUtfFakeExitCode;
120115

121-
Expectation _negateOutcomeIfNegativeTest(
122-
Expectation outcome, bool isNegative) {
123-
if (!isNegative) return outcome;
124-
if (outcome == Expectation.ignore) return outcome;
125-
if (outcome.canBeOutcomeOf(Expectation.fail)) {
126-
return Expectation.pass;
127-
}
128-
return Expectation.fail;
129-
}
130-
131116
/// Called when producing output for a test failure to describe this output.
132117
void describe(TestCase testCase, Progress progress, OutputWriter output) {
133118
output.subsection("exit code");
@@ -282,7 +267,7 @@ class BrowserCommandOutput extends CommandOutput
282267
with _UnittestSuiteMessagesMixin {
283268
final BrowserTestJsonResult _jsonResult;
284269
final BrowserTestOutput _result;
285-
final Expectation _rawOutcome;
270+
final Expectation _outcome;
286271

287272
factory BrowserCommandOutput(Command command, BrowserTestOutput result) {
288273
Expectation outcome;
@@ -317,7 +302,7 @@ class BrowserCommandOutput extends CommandOutput
317302
}
318303

319304
BrowserCommandOutput._internal(Command command, BrowserTestOutput result,
320-
this._rawOutcome, this._jsonResult, List<int> stdout, List<int> stderr)
305+
this._outcome, this._jsonResult, List<int> stdout, List<int> stderr)
321306
: _result = result,
322307
super(command, 0, result.didTimeout, stdout, stderr, result.duration,
323308
false, 0);
@@ -332,11 +317,11 @@ class BrowserCommandOutput extends CommandOutput
332317

333318
// Multitests are handled specially.
334319
if (testCase.hasRuntimeError) {
335-
if (_rawOutcome == Expectation.runtimeError) return Expectation.pass;
320+
if (_outcome == Expectation.runtimeError) return Expectation.pass;
336321
return Expectation.missingRuntimeError;
337322
}
338323

339-
return _negateOutcomeIfNegativeTest(_rawOutcome, testCase.isNegative);
324+
return _outcome;
340325
}
341326

342327
/// Cloned code from member result(), with changes.
@@ -348,7 +333,7 @@ class BrowserCommandOutput extends CommandOutput
348333
}
349334

350335
if (hasNonUtf8) return Expectation.nonUtf8Error;
351-
return _rawOutcome;
336+
return _outcome;
352337
}
353338

354339
void describe(TestCase testCase, Progress progress, OutputWriter output) {
@@ -509,13 +494,6 @@ class AnalysisCommandOutput extends CommandOutput with _StaticErrorOutput {
509494
return _validateExpectedErrors(testCase);
510495
}
511496

512-
// Handle negative.
513-
if (testCase.isNegative) {
514-
return errors.isNotEmpty
515-
? Expectation.pass
516-
: Expectation.missingCompileTimeError;
517-
}
518-
519497
// Handle errors / missing errors.
520498
if (testCase.hasCompileError) {
521499
if (errors.isNotEmpty) {
@@ -727,8 +705,7 @@ class VMCommandOutput extends CommandOutput with _UnittestSuiteMessagesMixin {
727705
outcome = Expectation.fail;
728706
}
729707

730-
outcome = _negateOutcomeIfIncompleteAsyncTest(outcome, decodeUtf8(stdout));
731-
return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
708+
return _negateOutcomeIfIncompleteAsyncTest(outcome, decodeUtf8(stdout));
732709
}
733710

734711
/// Cloned code from member result(), with changes.
@@ -835,9 +812,7 @@ class CompilationCommandOutput extends CommandOutput {
835812
return Expectation.compileTimeError;
836813
}
837814

838-
var outcome =
839-
exitCode == 0 ? Expectation.pass : Expectation.compileTimeError;
840-
return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
815+
return exitCode == 0 ? Expectation.pass : Expectation.compileTimeError;
841816
}
842817
}
843818

@@ -866,9 +841,7 @@ class DevCompilerCommandOutput extends CommandOutput {
866841
: Expectation.pass;
867842
}
868843

869-
var outcome =
870-
exitCode == 0 ? Expectation.pass : Expectation.compileTimeError;
871-
return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
844+
return exitCode == 0 ? Expectation.pass : Expectation.compileTimeError;
872845
}
873846

874847
/// Cloned code from member result(), with changes.
@@ -930,16 +903,15 @@ class VMKernelCompilationCommandOutput extends CompilationCommandOutput {
930903
}
931904

932905
// The actual outcome depends on the exitCode.
933-
var outcome = Expectation.pass;
934906
if (exitCode == VMCommandOutput._compileErrorExitCode ||
935907
exitCode == kBatchModeCompileTimeErrorExit) {
936-
outcome = Expectation.compileTimeError;
908+
return Expectation.compileTimeError;
937909
} else if (exitCode != 0) {
938910
// This is a general fail, in case we get an unknown nonzero exitcode.
939-
outcome = Expectation.fail;
911+
return Expectation.fail;
940912
}
941913

942-
return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
914+
return Expectation.pass;
943915
}
944916

945917
/// Cloned code from member result(), with changes.
@@ -1003,8 +975,7 @@ class JSCommandLineOutput extends CommandOutput
1003975
}
1004976

1005977
var outcome = exitCode == 0 ? Expectation.pass : Expectation.runtimeError;
1006-
outcome = _negateOutcomeIfIncompleteAsyncTest(outcome, decodeUtf8(stdout));
1007-
return _negateOutcomeIfNegativeTest(outcome, testCase.isNegative);
978+
return _negateOutcomeIfIncompleteAsyncTest(outcome, decodeUtf8(stdout));
1008979
}
1009980

1010981
/// Cloned code from member result(), with changes.

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ has been specified on the command line.''',
330330
'''
331331
Skip the compilation step, using the compilation artifacts left in
332332
the output folder from a previous run. This flag will often cause
333-
false positves and negatives, but can be useful for quick and
333+
false positives and negatives, but can be useful for quick and
334334
dirty offline testing when not making changes that affect the
335335
compiler.''',
336336
hide: true),

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

-5
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ abstract class RuntimeConfiguration {
9494

9595
List<String> dart2jsPreambles(Uri preambleDir) => [];
9696

97-
bool get shouldSkipNegativeTests => false;
98-
9997
/// Returns the path to the Dart VM executable.
10098
///
10199
/// Controlled by user with the option "--dart".
@@ -389,9 +387,6 @@ class SelfCheckRuntimeConfiguration extends DartVmRuntimeConfiguration {
389387
checked: _configuration.isChecked))
390388
.toList();
391389
}
392-
393-
@override
394-
bool get shouldSkipNegativeTests => true;
395390
}
396391

397392
/// Temporary runtime configuration for browser runtimes that haven't been

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

-7
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ class TestCase {
7575
bool get hasSyntaxError => testFile?.hasSyntaxError ?? false;
7676
bool get hasCompileError => testFile?.hasCompileError ?? false;
7777
bool get hasCrash => testFile?.hasCrash ?? false;
78-
bool get isNegative =>
79-
hasCompileError ||
80-
hasRuntimeError && configuration.runtime != Runtime.none ||
81-
displayName.contains("negative_test");
8278

8379
bool get unexpectedOutput {
8480
var outcome = result;
@@ -109,9 +105,6 @@ class TestCase {
109105
}
110106
return Expectation.pass;
111107
}
112-
if (displayName.contains("negative_test")) {
113-
return Expectation.fail;
114-
}
115108
if (configuration.compiler == Compiler.dart2analyzer && hasStaticWarning) {
116109
return Expectation.staticWarning;
117110
}

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

+2-13
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,6 @@ abstract class TestSuite {
166166
return false;
167167
}
168168

169-
if (isNegative(testFile) &&
170-
configuration.runtimeConfiguration.shouldSkipNegativeTests) {
171-
return false;
172-
}
173-
174169
// Handle sharding based on the original test path. All multitests of a
175170
// given original test belong to the same shard.
176171
if (configuration.shardCount > 1 &&
@@ -212,10 +207,6 @@ abstract class TestSuite {
212207
return false;
213208
}
214209

215-
bool isNegative(TestFile testFile) =>
216-
testFile.hasCompileError ||
217-
testFile.hasRuntimeError && configuration.runtime != Runtime.none;
218-
219210
String createGeneratedTestDirectoryHelper(
220211
String name, String dirname, Path testPath) {
221212
Path relative = testPath.relativeTo(Repository.dir);
@@ -880,8 +871,7 @@ class StandardTestSuite extends TestSuite {
880871
var htmlPathSubtest = _createUrlPathFromFile(Path(htmlPath));
881872
var fullHtmlPath = _uriForBrowserTest(htmlPathSubtest, subtestName);
882873

883-
commands.add(BrowserTestCommand(fullHtmlPath, configuration,
884-
retry: !isNegative(testFile)));
874+
commands.add(BrowserTestCommand(fullHtmlPath, configuration));
885875

886876
var fullName = testName;
887877
if (subtestName != null) fullName += "/$subtestName";
@@ -971,8 +961,7 @@ class PackageTestSuite extends StandardTestSuite {
971961
super._enqueueBrowserTest(testFile, expectations, onTest);
972962
} else {
973963
var fullPath = _createUrlPathFromFile(customHtmlPath);
974-
var command = BrowserTestCommand(fullPath, configuration,
975-
retry: !isNegative(testFile));
964+
var command = BrowserTestCommand(fullPath, configuration);
976965
_addTestCase(testFile, testFile.name, [command],
977966
expectations[testFile.name], onTest);
978967
}

0 commit comments

Comments
 (0)