Completion. Don't update @FailingTest expectations.
Change-Id: I627a2b82fd663c56636f52699237380cbf98e750 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357181 Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
committed by
Commit Queue
parent
c78f973ce8
commit
e2e6ee8b02
+2
-12
@@ -20,18 +20,6 @@ class ObjectPatternTest extends AbstractCompletionDriverTest
|
||||
await super.setUp();
|
||||
printerConfiguration.withLocationName = true;
|
||||
}
|
||||
|
||||
@FailingTest(reason: 'Suggest invalid static field / getter')
|
||||
@override
|
||||
Future<void> test_pattern_first() {
|
||||
return super.test_pattern_first();
|
||||
}
|
||||
|
||||
@FailingTest(reason: 'Suggest invalid static field / getter')
|
||||
@override
|
||||
Future<void> test_pattern_second() {
|
||||
return super.test_pattern_second();
|
||||
}
|
||||
}
|
||||
|
||||
mixin ObjectPatternTestCases on AbstractCompletionDriverTest {
|
||||
@@ -509,6 +497,7 @@ suggestions
|
||||
''');
|
||||
}
|
||||
|
||||
@FailingTest(reason: 'Suggest invalid static field / getter')
|
||||
Future<void> test_pattern_first() async {
|
||||
await computeSuggestions('''
|
||||
void f1(Object x0) {
|
||||
@@ -564,6 +553,7 @@ suggestions
|
||||
''');
|
||||
}
|
||||
|
||||
@FailingTest(reason: 'Suggest invalid static field / getter')
|
||||
Future<void> test_pattern_second() async {
|
||||
await computeSuggestions('''
|
||||
void f1(Object x0) {
|
||||
|
||||
@@ -4,66 +4,110 @@
|
||||
|
||||
import 'dart:io' as io;
|
||||
|
||||
import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
|
||||
import 'package:analyzer/dart/analysis/results.dart';
|
||||
import 'package:analyzer/dart/ast/visitor.dart';
|
||||
import 'package:analyzer/file_system/physical_file_system.dart';
|
||||
import 'package:analyzer/source/line_info.dart';
|
||||
import 'package:analyzer/src/dart/ast/ast.dart';
|
||||
import 'package:analyzer/src/utilities/extensions/collection.dart';
|
||||
import 'package:test/test.dart';
|
||||
import 'package:test_reflective_loader/test_reflective_loader.dart';
|
||||
|
||||
class TextExpectationsCollector {
|
||||
static const _updatingIsEnabled = false;
|
||||
/// If this flag is `true`, we accumulate updates to expectations.
|
||||
/// This should only happen locally, to update tests or implementation.
|
||||
///
|
||||
/// This flag should be `false` during code review.
|
||||
static const updatingIsEnabled = true;
|
||||
|
||||
static const assertMethods = {
|
||||
'AbstractCompletionDriverTest.assertResponse',
|
||||
'SelectionTest._assertSelection',
|
||||
};
|
||||
static final assertMethods = [
|
||||
_AssertMethod(
|
||||
className: 'AbstractCompletionDriverTest',
|
||||
methodName: 'assertResponse',
|
||||
argument: _ArgumentIndex(0),
|
||||
),
|
||||
_AssertMethod(
|
||||
className: 'SelectionTest',
|
||||
methodName: '_assertSelection',
|
||||
argument: _ArgumentIndex(1),
|
||||
),
|
||||
];
|
||||
|
||||
static final Map<String, _File> _files = {};
|
||||
|
||||
static void add(String actual) {
|
||||
if (!_updatingIsEnabled) {
|
||||
if (!updatingIsEnabled) {
|
||||
return;
|
||||
}
|
||||
|
||||
var traceLines = '${StackTrace.current}'.split('\n');
|
||||
for (var traceIndex = 0; traceIndex < traceLines.length; traceIndex++) {
|
||||
var traceLine = traceLines[traceIndex];
|
||||
for (var assertMethod in assertMethods) {
|
||||
if (traceLine.contains(' $assertMethod ')) {
|
||||
var invocationLine = traceLines[traceIndex + 1];
|
||||
var locationMatch = RegExp(
|
||||
r'file://(.+_test.dart):(\d+):',
|
||||
).firstMatch(invocationLine);
|
||||
if (locationMatch == null) {
|
||||
fail('Cannot parse: $invocationLine');
|
||||
}
|
||||
final traceLines = '${StackTrace.current}'.split('\n');
|
||||
for (final assertMethod in assertMethods) {
|
||||
for (var traceIndex = 0; traceIndex < traceLines.length; traceIndex++) {
|
||||
final traceLine = traceLines[traceIndex];
|
||||
if (!traceLine.contains(' ${assertMethod.stackTracePattern} ')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
var path = locationMatch.group(1)!;
|
||||
var line = int.parse(locationMatch.group(2)!);
|
||||
var file = _getFile(path);
|
||||
// Find the invocation of the assert method in the stack trace.
|
||||
var invocationTraceIndex = traceIndex + 1;
|
||||
if (traceLines[invocationTraceIndex] == '<asynchronous suspension>') {
|
||||
invocationTraceIndex++;
|
||||
}
|
||||
final invocationTraceLine = traceLines[invocationTraceIndex];
|
||||
|
||||
var invocationOffset = file.lineInfo.getOffsetOfLine(line - 1);
|
||||
// Parse the invocation stack trace line.
|
||||
final locationMatch = RegExp(
|
||||
r'(file://.+_test.dart):(\d+):',
|
||||
).firstMatch(invocationTraceLine);
|
||||
if (locationMatch == null) {
|
||||
fail('Cannot parse: $invocationTraceLine');
|
||||
}
|
||||
|
||||
const String rawStringPrefix = "r'''";
|
||||
var expectationOffset = file.content.indexOf(
|
||||
rawStringPrefix,
|
||||
invocationOffset,
|
||||
);
|
||||
expectationOffset += rawStringPrefix.length;
|
||||
var expectationEnd = file.content.indexOf("'''", expectationOffset);
|
||||
final path = Uri.parse(locationMatch.group(1)!).toFilePath();
|
||||
final line = int.parse(locationMatch.group(2)!);
|
||||
final file = _getFile(path);
|
||||
|
||||
file.replacements.add(
|
||||
_Replacement(
|
||||
expectationOffset,
|
||||
expectationEnd,
|
||||
'\n$actual',
|
||||
),
|
||||
final invocation = file.findInvocation(
|
||||
invocationLine: line,
|
||||
);
|
||||
if (invocation == null) {
|
||||
fail('Cannot find MethodInvocation.');
|
||||
}
|
||||
|
||||
if (invocation.methodName.name != assertMethod.methodName) {
|
||||
fail(
|
||||
'Expected: ${assertMethod.methodName}\n'
|
||||
'Actual: ${invocation.methodName.name}\n',
|
||||
);
|
||||
}
|
||||
|
||||
if (invocation.isInFailingTest) {
|
||||
return;
|
||||
}
|
||||
|
||||
final argumentList = invocation.argumentList;
|
||||
final argument = assertMethod.argument.get(argumentList);
|
||||
if (argument is! SimpleStringLiteral) {
|
||||
fail('Not a literal: ${argument.runtimeType}');
|
||||
}
|
||||
|
||||
file.addReplacement(
|
||||
_Replacement(
|
||||
argument.contentsOffset,
|
||||
argument.contentsEnd,
|
||||
actual,
|
||||
),
|
||||
);
|
||||
|
||||
// Stop after the first (most specific) assert method.
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static void _apply() {
|
||||
for (var file in _files.values) {
|
||||
static void apply() {
|
||||
for (final file in _files.values) {
|
||||
file.applyReplacements();
|
||||
}
|
||||
_files.clear();
|
||||
@@ -77,22 +121,63 @@ class TextExpectationsCollector {
|
||||
@reflectiveTest
|
||||
class UpdateTextExpectations {
|
||||
test_applyReplacements() {
|
||||
TextExpectationsCollector._apply();
|
||||
TextExpectationsCollector.apply();
|
||||
}
|
||||
}
|
||||
|
||||
sealed class _Argument {
|
||||
Expression get(ArgumentList argumentList);
|
||||
}
|
||||
|
||||
final class _ArgumentIndex extends _Argument {
|
||||
final int index;
|
||||
|
||||
_ArgumentIndex(this.index);
|
||||
|
||||
@override
|
||||
Expression get(ArgumentList argumentList) {
|
||||
return argumentList.arguments
|
||||
.whereNotType<NamedExpression>()
|
||||
.elementAt(index);
|
||||
}
|
||||
}
|
||||
|
||||
class _AssertMethod {
|
||||
final String methodName;
|
||||
final String stackTracePattern;
|
||||
final _Argument argument;
|
||||
|
||||
const _AssertMethod({
|
||||
required String className,
|
||||
required this.methodName,
|
||||
required this.argument,
|
||||
}) : stackTracePattern = '$className.$methodName';
|
||||
}
|
||||
|
||||
class _File {
|
||||
final String path;
|
||||
final String content;
|
||||
final LineInfo lineInfo;
|
||||
final CompilationUnit unit;
|
||||
final List<_Replacement> replacements = [];
|
||||
|
||||
factory _File(String path) {
|
||||
var content = io.File(path).readAsStringSync();
|
||||
final content = io.File(path).readAsStringSync();
|
||||
|
||||
final collection = AnalysisContextCollection(
|
||||
resourceProvider: PhysicalResourceProvider.INSTANCE,
|
||||
includedPaths: [path],
|
||||
);
|
||||
final analysisContext = collection.contextFor(path);
|
||||
final analysisSession = analysisContext.currentSession;
|
||||
final parseResult = analysisSession.getParsedUnit(path);
|
||||
parseResult as ParsedUnitResult;
|
||||
|
||||
return _File._(
|
||||
path: path,
|
||||
content: content,
|
||||
lineInfo: LineInfo.fromContent(content),
|
||||
unit: parseResult.unit,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -100,18 +185,83 @@ class _File {
|
||||
required this.path,
|
||||
required this.content,
|
||||
required this.lineInfo,
|
||||
required this.unit,
|
||||
});
|
||||
|
||||
void addReplacement(_Replacement replacement) {
|
||||
// Check if there is the same replacement.
|
||||
for (final existing in replacements) {
|
||||
if (existing.offset == replacement.offset) {
|
||||
// Sanity check.
|
||||
if (existing.end != replacement.end) {
|
||||
fail(
|
||||
'At offset: ${existing.offset}\n'
|
||||
'Existing end: ${existing.end}\n'
|
||||
'New end: ${replacement.end}\n',
|
||||
);
|
||||
}
|
||||
if (existing.text != replacement.text) {
|
||||
fail(
|
||||
'At offset: ${existing.offset}\n'
|
||||
'Existing text:\n${existing.text}\n'
|
||||
'New text:\n${replacement.end}\n',
|
||||
);
|
||||
}
|
||||
// We already have the same replacement, exit.
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// This is a new replacement, add it.
|
||||
replacements.add(replacement);
|
||||
}
|
||||
|
||||
void applyReplacements() {
|
||||
replacements.sort((a, b) => b.offset - a.offset);
|
||||
var newCode = content;
|
||||
for (var replacement in replacements) {
|
||||
for (final replacement in replacements) {
|
||||
newCode = newCode.substring(0, replacement.offset) +
|
||||
replacement.text +
|
||||
newCode.substring(replacement.end);
|
||||
}
|
||||
io.File(path).writeAsStringSync(newCode);
|
||||
}
|
||||
|
||||
MethodInvocation? findInvocation({
|
||||
required int invocationLine,
|
||||
}) {
|
||||
final visitor = _InvocationVisitor(
|
||||
lineInfo: lineInfo,
|
||||
requestedLine: invocationLine,
|
||||
);
|
||||
unit.accept(visitor);
|
||||
return visitor.result;
|
||||
}
|
||||
}
|
||||
|
||||
class _InvocationVisitor extends RecursiveAstVisitor<void> {
|
||||
final LineInfo lineInfo;
|
||||
final int requestedLine;
|
||||
MethodInvocation? result;
|
||||
|
||||
_InvocationVisitor({
|
||||
required this.lineInfo,
|
||||
required this.requestedLine,
|
||||
});
|
||||
|
||||
@override
|
||||
void visitMethodInvocation(MethodInvocation node) {
|
||||
if (result != null) {
|
||||
return;
|
||||
}
|
||||
|
||||
final nodeLine = lineInfo.getLocation(node.offset).lineNumber;
|
||||
if (nodeLine == requestedLine) {
|
||||
result = node;
|
||||
}
|
||||
|
||||
super.visitMethodInvocation(node);
|
||||
}
|
||||
}
|
||||
|
||||
class _Replacement {
|
||||
@@ -121,3 +271,18 @@ class _Replacement {
|
||||
|
||||
_Replacement(this.offset, this.end, this.text);
|
||||
}
|
||||
|
||||
extension on AstNode {
|
||||
bool get isInFailingTest {
|
||||
final testMethod = thisOrAncestorOfType<MethodDeclaration>();
|
||||
if (testMethod != null && testMethod.name.lexeme.startsWith('test_')) {
|
||||
for (final annotation in testMethod.metadata) {
|
||||
var name = annotation.name.name;
|
||||
if (const {'FailingTest', 'failingTest'}.contains(name)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user