Skip to content

Commit 496c5ca

Browse files
DanTupCommit Queue
DanTup
authored and
Commit Queue
committed
[analysis_server] Minor CodeAction test cleanup
This is some more final (🙃) cleanup before having CodeActions work for LSP-over-Legacy and run all the same tests. Includes: - Adding some additional signatures to the SharedTestInterface - Moving some setup code (that uses those methods) into the shared test mixins - Add a field for `testPackageName` to allow tests to handle differences between the test package name between LSP/Legacy (something that should probably consolidated but is probably not for this CL) - Move plugin tests back out of the shared code action tests (these require some additional abstraction to work the same across both kinds of tests - TODO added) Change-Id: Iad25f1ff1e141d3315e9b972a6d0850e71d4a4bd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/427443 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 9dd284e commit 496c5ca

11 files changed

+206
-169
lines changed

‎pkg/analysis_server/test/analysis_server_base.dart‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -209,12 +209,14 @@ class PubPackageAnalysisServerTest extends ContextResolutionTest
209209
'$testPackageLibPath/test.dart',
210210
);
211211

212-
late String pubspecFilePath = resourceProvider.convertPath(
213-
'$testPackageLibPath/pubspec.yaml',
212+
late String pubspecFilePath = pathContext.normalize(
213+
resourceProvider.convertPath('$testPackageRootPath/pubspec.yaml'),
214214
);
215215

216216
late TestCode parsedTestCode;
217217

218+
final String testPackageName = 'test';
219+
218220
/// Return a list of the experiments that are to be enabled for tests in this
219221
/// class, an empty list if there are no experiments that should be enabled.
220222
List<String> get experiments => experimentsForTests;
@@ -237,7 +239,7 @@ class PubPackageAnalysisServerTest extends ContextResolutionTest
237239
Folder get testPackageRoot => getFolder(testPackageRootPath);
238240

239241
@override
240-
String get testPackageRootPath => '$workspaceRootPath/test';
242+
String get testPackageRootPath => '$workspaceRootPath/$testPackageName';
241243

242244
String get testPackageTestPath => '$testPackageRootPath/test';
243245

@@ -267,7 +269,7 @@ class PubPackageAnalysisServerTest extends ContextResolutionTest
267269
@override
268270
void createDefaultFiles() {
269271
writeTestPackageConfig();
270-
writeTestPackagePubspecYamlFile('name: test');
272+
writeTestPackagePubspecYamlFile('name: $testPackageName');
271273

272274
writeTestPackageAnalysisOptionsFile(
273275
analysisOptionsContent(experiments: experiments),

‎pkg/analysis_server/test/lsp/code_actions_assists_test.dart‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import 'dart:async';
77
import 'package:analysis_server/lsp_protocol/protocol.dart';
88
import 'package:analysis_server/src/analysis_server.dart';
99
import 'package:analysis_server/src/lsp/extensions/code_action.dart';
10-
import 'package:analysis_server/src/services/correction/assist_internal.dart';
1110
import 'package:analyzer/src/test_utilities/test_code_format.dart';
1211
import 'package:analyzer_plugin/protocol/protocol_common.dart' as plugin;
1312
import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin;
@@ -30,20 +29,8 @@ class AssistsCodeActionsTest extends AbstractLspAnalysisServerTest
3029
with
3130
LspSharedTestMixin,
3231
CodeActionsTestMixin,
32+
// Most tests are defined in a shared mixin.
3333
SharedAssistsCodeActionsTests {
34-
@override
35-
void setUp() {
36-
super.setUp();
37-
38-
setApplyEditSupport();
39-
setDocumentChangesSupport();
40-
setSupportedCodeActionKinds([CodeActionKind.Refactor]);
41-
42-
registerBuiltInAssistGenerators();
43-
44-
writeTestPackageConfig(flutter: true);
45-
}
46-
4734
Future<void> test_plugin() async {
4835
failTestOnErrorDiagnostic = false;
4936

‎pkg/analysis_server/test/lsp/code_actions_refactor_test.dart‎

Lines changed: 100 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ void main() {
2727
@reflectiveTest
2828
class ConvertGetterToMethodCodeActionsTest extends RefactorCodeActionsTest
2929
with
30-
// Most tests are defined in a shared mixin.
30+
// Tests are defined in a shared mixin.
3131
SharedConvertGetterToMethodRefactorCodeActionsTests {}
3232

3333
@reflectiveTest
3434
class ConvertMethodToGetterCodeActionsTest extends RefactorCodeActionsTest
3535
with
36-
// Most tests are defined in a shared mixin.
36+
// Tests are defined in a shared mixin.
3737
SharedConvertMethodToGetterRefactorCodeActionsTests {}
3838

3939
@reflectiveTest
@@ -42,6 +42,99 @@ class ExtractMethodRefactorCodeActionsTest extends RefactorCodeActionsTest
4242
LspProgressNotificationsMixin,
4343
// Most tests are defined in a shared mixin.
4444
SharedExtractMethodRefactorCodeActionsTests {
45+
Future<void> test_progress_clientProvided() async {
46+
const content = '''
47+
void f() {
48+
print('Test!');
49+
[!print('Test!');!]
50+
}
51+
''';
52+
const expectedContent = '''
53+
void f() {
54+
print('Test!');
55+
newMethod();
56+
}
57+
58+
void newMethod() {
59+
print('Test!');
60+
}
61+
''';
62+
63+
// Expect begin/end progress updates without a create, since the
64+
// token was supplied by us (the client).
65+
expect(progressUpdates, emitsInOrder(['BEGIN', 'END']));
66+
67+
await verifyCodeActionLiteralEdits(
68+
content,
69+
expectedContent,
70+
command: Commands.performRefactor,
71+
title: extractMethodTitle,
72+
commandWorkDoneToken: clientProvidedTestWorkDoneToken,
73+
);
74+
}
75+
76+
Future<void> test_progress_notSupported() async {
77+
const content = '''
78+
void f() {
79+
print('Test!');
80+
[!print('Test!');!]
81+
}
82+
''';
83+
const expectedContent = '''
84+
void f() {
85+
print('Test!');
86+
newMethod();
87+
}
88+
89+
void newMethod() {
90+
print('Test!');
91+
}
92+
''';
93+
94+
var didGetProgressNotifications = false;
95+
progressUpdates.listen((_) => didGetProgressNotifications = true);
96+
97+
await verifyCodeActionLiteralEdits(
98+
content,
99+
expectedContent,
100+
command: Commands.performRefactor,
101+
title: extractMethodTitle,
102+
);
103+
104+
expect(didGetProgressNotifications, isFalse);
105+
}
106+
107+
Future<void> test_progress_serverGenerated() async {
108+
const content = '''
109+
void f() {
110+
print('Test!');
111+
[!print('Test!');!]
112+
}
113+
''';
114+
const expectedContent = '''
115+
void f() {
116+
print('Test!');
117+
newMethod();
118+
}
119+
120+
void newMethod() {
121+
print('Test!');
122+
}
123+
''';
124+
125+
// Expect create/begin/end progress updates, because in this case the server
126+
// generates the token.
127+
expect(progressUpdates, emitsInOrder(['CREATE', 'BEGIN', 'END']));
128+
129+
setWorkDoneProgressSupport();
130+
await verifyCodeActionLiteralEdits(
131+
content,
132+
expectedContent,
133+
command: Commands.performRefactor,
134+
title: extractMethodTitle,
135+
);
136+
}
137+
45138
/// Test if the client does not call refactor.validate it still gets a
46139
/// sensible `showMessage` call and not a failed request.
47140
Future<void> test_validLocation_failsInitialValidation_noValidation() async {
@@ -84,31 +177,25 @@ void doFoo(void Function() a) => a();
84177
@reflectiveTest
85178
class ExtractVariableRefactorCodeActionsTest extends RefactorCodeActionsTest
86179
with
87-
// Most tests are defined in a shared mixin.
180+
// Tests are defined in a shared mixin.
88181
SharedExtractVariableRefactorCodeActionsTests {}
89182

90183
@reflectiveTest
91184
class ExtractWidgetRefactorCodeActionsTest extends RefactorCodeActionsTest
92185
with
93-
// Most tests are defined in a shared mixin.
94-
SharedExtractWidgetRefactorCodeActionsTests {
95-
@override
96-
void setUp() {
97-
super.setUp();
98-
writeTestPackageConfig(flutter: true);
99-
}
100-
}
186+
// Tests are defined in a shared mixin.
187+
SharedExtractWidgetRefactorCodeActionsTests {}
101188

102189
@reflectiveTest
103190
class InlineLocalVariableRefactorCodeActionsTest extends RefactorCodeActionsTest
104191
with
105-
// Most tests are defined in a shared mixin.
192+
// Tests are defined in a shared mixin.
106193
SharedInlineLocalVariableRefactorCodeActionsTests {}
107194

108195
@reflectiveTest
109196
class InlineMethodRefactorCodeActionsTest extends RefactorCodeActionsTest
110197
with
111-
// Most tests are defined in a shared mixin.
198+
// Tests are defined in a shared mixin.
112199
SharedInlineMethodRefactorCodeActionsTests {}
113200

114201
abstract class RefactorCodeActionsTest extends AbstractLspAnalysisServerTest

‎pkg/analysis_server/test/lsp/code_actions_source_test.dart‎

Lines changed: 10 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,43 +25,16 @@ void main() {
2525

2626
abstract class AbstractSourceCodeActionsTest
2727
extends AbstractLspAnalysisServerTest
28-
with LspSharedTestMixin, CodeActionsTestMixin {
29-
/// For convenience since source code actions do not rely on a position (but
30-
/// one must be provided), uses [startOfDocPos] to avoid every test needing
31-
/// to include a '^' marker.
32-
@override
33-
Future<List<CodeAction>> getCodeActions(
34-
Uri fileUri, {
35-
Range? range,
36-
Position? position,
37-
List<CodeActionKind>? kinds,
38-
CodeActionTriggerKind? triggerKind,
39-
ProgressToken? workDoneToken,
40-
}) {
41-
return super.getCodeActions(
42-
fileUri,
43-
position: startOfDocPos,
44-
kinds: kinds,
45-
triggerKind: triggerKind,
46-
workDoneToken: workDoneToken,
47-
);
48-
}
49-
50-
@override
51-
void setUp() {
52-
super.setUp();
53-
54-
setApplyEditSupport();
55-
setDocumentChangesSupport();
56-
setSupportedCodeActionKinds([CodeActionKind.Source]);
57-
}
58-
}
28+
with
29+
LspSharedTestMixin,
30+
SharedSourceCodeActionsTestMixin,
31+
CodeActionsTestMixin {}
5932

6033
@reflectiveTest
6134
class FixAllSourceCodeActionsTest extends AbstractSourceCodeActionsTest {
6235
@override
63-
void setUp() {
64-
super.setUp();
36+
Future<void> setUp() async {
37+
await super.setUp();
6538

6639
// Fix tests are likely to have diagnostics that need fixing.
6740
failTestOnErrorDiagnostic = false;
@@ -362,11 +335,13 @@ int? a;
362335
@reflectiveTest
363336
class OrganizeImportsSourceCodeActionsTest extends AbstractSourceCodeActionsTest
364337
with
365-
// Most tests are defined in a shared mixin.
338+
SharedSourceCodeActionsTestMixin,
339+
// Tests are defined in a shared mixin.
366340
SharedOrganizeImportsSourceCodeActionsTests {}
367341

368342
@reflectiveTest
369343
class SortMembersSourceCodeActionsTest extends AbstractSourceCodeActionsTest
370344
with
371-
// Most tests are defined in a shared mixin.
345+
SharedSourceCodeActionsTestMixin,
346+
// Tests are defined in a shared mixin.
372347
SharedSortMembersSourceCodeActionsTests {}

‎pkg/analysis_server/test/lsp/server_abstract.dart‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,10 @@ mixin LspAnalysisServerTestMixin on LspRequestHelpersMixin, LspEditHelpersMixin
792792
pubspecFilePath,
793793
analysisOptionsPath;
794794

795-
final String simplePubspecContent = 'name: my_project';
795+
/// The name of the test package (and the folder where the package lives).
796+
final String testPackageName = 'my_project';
797+
798+
late final String simplePubspecContent = 'name: $testPackageName';
796799

797800
/// The client capabilities sent to the server during initialization.
798801
///

‎pkg/analysis_server/test/lsp_over_legacy/abstract_lsp_over_legacy.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ abstract class LspOverLegacyTest extends PubPackageAnalysisServerTest
400400
@override
401401
Future<void> setUp() async {
402402
super.setUp();
403-
await setRoots(included: [workspaceRootPath], excluded: []);
403+
await setRoots(included: [testPackageRootPath], excluded: []);
404404
}
405405

406406
Future<void> updateOverlay(String filePath, SourceEdit edit) {

‎pkg/analysis_server/test/shared/shared_code_actions_assists_tests.dart‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'dart:convert';
88
import 'package:analysis_server/lsp_protocol/protocol.dart';
99
import 'package:analysis_server/src/lsp/constants.dart';
1010
import 'package:analysis_server/src/lsp/extensions/code_action.dart';
11+
import 'package:analysis_server/src/services/correction/assist_internal.dart';
1112
import 'package:analyzer/src/test_utilities/test_code_format.dart';
1213
import 'package:test/test.dart';
1314

@@ -27,6 +28,19 @@ mixin SharedAssistsCodeActionsTests
2728
LspEditHelpersMixin,
2829
LspVerifyEditHelpersMixin,
2930
ClientCapabilitiesHelperMixin {
31+
@override
32+
Future<void> setUp() async {
33+
await super.setUp();
34+
35+
setApplyEditSupport();
36+
setDocumentChangesSupport();
37+
setSupportedCodeActionKinds([CodeActionKind.Refactor]);
38+
39+
registerBuiltInAssistGenerators();
40+
41+
writeTestPackageConfig(flutter: true);
42+
}
43+
3044
Future<void> test_appliesCorrectEdits_withDocumentChangesSupport() async {
3145
// This code should get an assist to add a show combinator.
3246
const content = '''

‎pkg/analysis_server/test/shared/shared_code_actions_fixes_tests.dart‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ mixin SharedFixesCodeActionsTests
3333
pathContext.join(projectFolderPath, 'analysis_options.yaml');
3434

3535
@override
36-
void setUp() {
37-
super.setUp();
36+
Future<void> setUp() async {
37+
await super.setUp();
3838

3939
// Fix tests are likely to have diagnostics that need fixing.
4040
failTestOnErrorDiagnostic = false;
@@ -579,10 +579,6 @@ var a = [Test, Test, Te[!!]st];
579579
}
580580

581581
Future<void> test_noDuplicates_withDocumentChangesSupport() async {
582-
setApplyEditSupport();
583-
setDocumentChangesSupport();
584-
setSupportedCodeActionKinds([CodeActionKind.QuickFix]);
585-
586582
var code = TestCode.parse('''
587583
var a = [Test, Test, Te[!!]st];
588584
''');
@@ -655,8 +651,8 @@ ProcessInfo b;
655651
Future<void> test_pubspec() async {
656652
const content = '^';
657653

658-
const expectedContent = r'''
659-
name: my_project
654+
var expectedContent = '''
655+
name: $testPackageName
660656
''';
661657

662658
await verifyCodeActionLiteralEdits(

0 commit comments

Comments
 (0)