Skip to content

Commit 95d1ebd

Browse files
bwilkersoncommit-bot@chromium.org
authored andcommitted
Fix one cause of timeouts in tests
Change-Id: I8514c7a1e89c69bbbf11882fa734903cd7c7569c Reviewed-on: https://dart-review.googlesource.com/68430 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent 72b2542 commit 95d1ebd

File tree

10 files changed

+83
-31
lines changed

10 files changed

+83
-31
lines changed

pkg/analysis_server/test/analysis/get_errors_test.dart

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,12 +157,15 @@ class GetErrorsTest_UseCFE extends GetErrorsTest {
157157

158158
@failingTest
159159
@override
160-
test_errorInPart() async => super.test_errorInPart();
160+
test_errorInPart() => super.test_errorInPart();
161161

162162
@override
163-
test_fileDoesNotExist() async => super.test_fileDoesNotExist();
163+
test_fileDoesNotExist() => super.test_fileDoesNotExist();
164+
165+
@failingTest
166+
@override
167+
test_hasErrors() => super.test_hasErrors();
164168

165169
@override
166-
test_removeContextAfterRequest() async =>
167-
super.test_removeContextAfterRequest();
170+
test_removeContextAfterRequest() => super.test_removeContextAfterRequest();
168171
}

pkg/analysis_server/test/analysis_abstract.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,9 @@ class AbstractAnalysisTest extends Object with ResourceProviderMixin {
262262
* Completes with a successful [Response] for the given [request].
263263
* Otherwise fails.
264264
*/
265-
Future<Response> waitResponse(Request request) async {
266-
return serverChannel.sendRequest(request);
265+
Future<Response> waitResponse(Request request,
266+
{bool throwOnError = true}) async {
267+
return serverChannel.sendRequest(request, throwOnError: throwOnError);
267268
}
268269
}
269270

pkg/analysis_server/test/edit/postfix_completion_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,15 @@ main() {
7979
var params = new EditGetPostfixCompletionParams(testFile, key, offset);
8080
var request =
8181
new Request('0', "edit.isPostfixCompletionApplicable", params.toJson());
82-
Response response = await waitResponse(request);
82+
Response response = await waitResponse(request, throwOnError: false);
8383
var isApplicable =
8484
new EditIsPostfixCompletionApplicableResult.fromResponse(response);
8585
if (!isApplicable.value) {
8686
fail("Postfix completion not applicable at given location");
8787
}
8888
request = new EditGetPostfixCompletionParams(testFile, key, offset)
8989
.toRequest('1');
90-
response = await waitResponse(request);
90+
response = await waitResponse(request, throwOnError: false);
9191
var result = new EditGetPostfixCompletionResult.fromResponse(response);
9292
change = result.change;
9393
}

pkg/analysis_server/test/edit/refactoring_test.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,11 @@ class ExtractLocalVariableTest_UseCFE extends ExtractLocalVariableTest {
609609
@override
610610
test_analysis_onlyOneFile() => super.test_analysis_onlyOneFile();
611611

612+
@failingTest
613+
@override
614+
test_resetOnAnalysisSetChanged_overlay() =>
615+
super.test_resetOnAnalysisSetChanged_overlay();
616+
612617
@failingTest
613618
@override
614619
test_resetOnAnalysisSetChanged_watch_otherFile() =>

pkg/analysis_server/test/edit/sort_members_test.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,9 @@ class C {}
260260
class SortMembersTest_UseCFE extends SortMembersTest {
261261
@override
262262
bool get useCFE => true;
263+
264+
@failingTest
265+
@override
266+
test_OK_directives_withAnnotation() =>
267+
super.test_OK_directives_withAnnotation();
263268
}

pkg/analysis_server/test/integration/analysis/get_errors_test.dart

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'dart:async';
66

7+
import 'package:analysis_server/protocol/protocol_generated.dart';
78
import 'package:test/test.dart';
89
import 'package:test_reflective_loader/test_reflective_loader.dart';
910

@@ -18,21 +19,17 @@ main() {
1819

1920
@reflectiveTest
2021
class GetErrorsTest extends AbstractAnalysisServerIntegrationTest {
21-
test_getErrors() {
22+
test_getErrors() async {
2223
String pathname = sourcePath('test.dart');
2324
String text = r'''
2425
main() {
2526
var x // parse error: missing ';'
2627
}''';
2728
writeFile(pathname, text);
2829
standardAnalysisSetup();
29-
Future finishTest() {
30-
return sendAnalysisGetErrors(pathname).then((result) {
31-
expect(result.errors, equals(currentAnalysisErrors[pathname]));
32-
});
33-
}
34-
35-
return analysisFinished.then((_) => finishTest());
30+
await analysisFinished;
31+
AnalysisGetErrorsResult result = await sendAnalysisGetErrors(pathname);
32+
expect(result.errors, equals(currentAnalysisErrors[pathname]));
3633
}
3734
}
3835

pkg/analysis_server/test/mocks.dart

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class MockServerChannel implements ServerCommunicationChannel {
5151
new StreamController<Response>.broadcast();
5252
StreamController<Notification> notificationController =
5353
new StreamController<Notification>(sync: true);
54+
Completer<Response> errorCompleter;
5455

5556
List<Response> responsesReceived = [];
5657
List<Notification> notificationsReceived = [];
@@ -81,23 +82,34 @@ class MockServerChannel implements ServerCommunicationChannel {
8182
return;
8283
}
8384
notificationsReceived.add(notification);
85+
if (errorCompleter != null && notification.event == 'server.error') {
86+
errorCompleter.completeError(
87+
new ServerError(notification.params['message']),
88+
new StackTrace.fromString(notification.params['stackTrace']));
89+
}
8490
// Wrap send notification in future to simulate websocket
8591
// TODO(scheglov) ask Dan why and decide what to do
8692
// new Future(() => notificationController.add(notification));
8793
notificationController.add(notification);
8894
}
8995

9096
/**
91-
* Simulate request/response pair.
97+
* Send the given [request] to the server and return a future that will
98+
* complete when a response associated with the [request] has been received.
99+
* The value of the future will be the received response. If [throwOnError] is
100+
* `true` (the default) then the returned future will throw an exception if a
101+
* server error is reported before the response has been received.
92102
*/
93-
Future<Response> sendRequest(Request request) {
103+
Future<Response> sendRequest(Request request, {bool throwOnError = true}) {
104+
// TODO(brianwilkerson) Attempt to remove the `throwOnError` parameter and
105+
// have the default behavior be the only behavior.
94106
// No further requests should be sent after the connection is closed.
95107
if (_closed) {
96108
throw new Exception('sendRequest after connection closed');
97109
}
98110
// Wrap send request in future to simulate WebSocket.
99111
new Future(() => requestController.add(request));
100-
return waitForResponse(request);
112+
return waitForResponse(request, throwOnError: throwOnError);
101113
}
102114

103115
@override
@@ -111,10 +123,28 @@ class MockServerChannel implements ServerCommunicationChannel {
111123
new Future(() => responseController.add(response));
112124
}
113125

114-
Future<Response> waitForResponse(Request request) {
126+
/**
127+
* Return a future that will complete when a response associated with the
128+
* given [request] has been received. The value of the future will be the
129+
* received response. If [throwOnError] is `true` (the default) then the
130+
* returned future will throw an exception if a server error is reported
131+
* before the response has been received.
132+
*
133+
* Unlike [sendRequest], this method assumes that the [request] has already
134+
* been sent to the server.
135+
*/
136+
Future<Response> waitForResponse(Request request,
137+
{bool throwOnError = true}) {
138+
// TODO(brianwilkerson) Attempt to remove the `throwOnError` parameter and
139+
// have the default behavior be the only behavior.
115140
String id = request.id;
116-
return responseController.stream
117-
.firstWhere((response) => response.id == id);
141+
Future<Response> response =
142+
responseController.stream.firstWhere((response) => response.id == id);
143+
if (throwOnError) {
144+
errorCompleter ??= new Completer<Response>();
145+
return Future.any([response, errorCompleter.future]);
146+
}
147+
return response;
118148
}
119149
}
120150

@@ -194,6 +224,16 @@ class MockSource extends StringTypedMock implements Source {
194224
bool exists() => null;
195225
}
196226

227+
class ServerError implements Exception {
228+
final message;
229+
230+
ServerError(this.message);
231+
232+
String toString() {
233+
return "Server Error: $message";
234+
}
235+
}
236+
197237
class StringTypedMock {
198238
String _toString;
199239

pkg/analysis_server/test/search/type_hierarchy_test.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,8 +1086,7 @@ class GetTypeHierarchyTest_UseCFE extends GetTypeHierarchyTest {
10861086
@failingTest
10871087
@override
10881088
test_class_extendsTypeB() {
1089-
fail('Timeout');
1090-
// return callFailingTest(super.test_class_extendsTypeB);
1089+
return callFailingTest(super.test_class_extendsTypeB);
10911090
}
10921091

10931092
@failingTest

pkg/analysis_server/test/socket_server_test.dart

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,18 @@ class SocketServerTest {
9292
});
9393
}
9494

95-
static Future requestHandler_futureException() {
95+
static Future requestHandler_futureException() async {
9696
SocketServer server = _createSocketServer();
9797
MockServerChannel channel = new MockServerChannel();
9898
server.createAnalysisServer(channel);
9999
_MockRequestHandler handler = new _MockRequestHandler(true);
100100
server.analysisServer.handlers = [handler];
101101
var request = new ServerGetVersionParams().toRequest('0');
102-
return channel.sendRequest(request).then((Response response) {
103-
expect(response.id, equals('0'));
104-
expect(response.error, isNull);
105-
channel.expectMsgCount(responseCount: 1, notificationCount: 2);
106-
expect(channel.notificationsReceived[1].event, SERVER_NOTIFICATION_ERROR);
107-
});
102+
Response response = await channel.sendRequest(request, throwOnError: false);
103+
expect(response.id, equals('0'));
104+
expect(response.error, isNull);
105+
channel.expectMsgCount(responseCount: 1, notificationCount: 2);
106+
expect(channel.notificationsReceived[1].event, SERVER_NOTIFICATION_ERROR);
108107
}
109108

110109
static SocketServer _createSocketServer() {

pkg/pkg.status

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ status_file/test/status_expression_dnf_test: RuntimeError # not strong-clean
226226
async/test/stream_zip_test: SkipSlow # Times out. Issue 22050
227227
collection/test/unmodifiable_collection_test: SkipSlow # Times out. Issue 22050
228228

229+
[ $compiler == none && !$preview_dart_2 ]
230+
analysis_server/test/analysis/get_errors_test: Fail
231+
229232
[ $runtime == vm && $system == windows ]
230233
analysis_server/test/analysis/get_errors_test: Skip # runtime error, Issue 22180
231234
analysis_server/test/benchmarks_test: RuntimeError # Issue 32355

0 commit comments

Comments
 (0)