Skip to content

Commit

Permalink
test [nfc]: Introduce "finish" helper for async checks
Browse files Browse the repository at this point in the history
This brings the file test/api/core_test.dart up to being clean
against the `unawaited_futures` lint rule, toward zulip#731.

It does so with a smaller diff, and simpler resulting code,
than by adding `await` on each of the futures.  For comparison:
  zulip#934 (comment)
  • Loading branch information
gnprice committed Oct 8, 2024
1 parent 8c440fd commit 701763d
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 19 deletions.
39 changes: 20 additions & 19 deletions test/api/core_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:zulip/model/localizations.dart';

import '../model/binding.dart';
import '../stdlib_checks.dart';
import '../test_async.dart';
import 'exception_checks.dart';
import 'fake_api.dart';
import '../example_data.dart' as eg;
Expand All @@ -19,8 +20,8 @@ void main() {
TestZulipBinding.ensureInitialized();

test('ApiConnection.get', () async {
Future<void> checkRequest(Map<String, dynamic>? params, String expectedRelativeUrl) {
return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
void checkRequest(Map<String, dynamic>? params, String expectedRelativeUrl) {
finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
connection.prepare(json: {});
await connection.get(kExampleRouteName, (json) => json, 'example/route', params);
check(connection.lastRequest!).isA<http.Request>()
Expand All @@ -31,7 +32,7 @@ void main() {
...kFallbackUserAgentHeader,
})
..body.equals('');
});
}));
}

checkRequest(null, '/api/v1/example/route');
Expand All @@ -50,8 +51,8 @@ void main() {
});

test('ApiConnection.post', () async {
Future<void> checkRequest(Map<String, dynamic>? params, String expectedBody, {bool expectContentType = true}) {
return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
void checkRequest(Map<String, dynamic>? params, String expectedBody, {bool expectContentType = true}) {
finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
connection.prepare(json: {});
await connection.post(kExampleRouteName, (json) => json, 'example/route', params);
check(connection.lastRequest!).isA<http.Request>()
Expand All @@ -64,7 +65,7 @@ void main() {
'content-type': 'application/x-www-form-urlencoded; charset=utf-8',
})
..body.equals(expectedBody);
});
}));
}

checkRequest(null, '', expectContentType: false);
Expand All @@ -81,9 +82,9 @@ void main() {
});

test('ApiConnection.postFileFromStream', () async {
Future<void> checkRequest(List<List<int>> content, int length,
void checkRequest(List<List<int>> content, int length,
{String? filename, String? contentType, bool isContentTypeInvalid = false}) {
return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
connection.prepare(json: {});
await connection.postFileFromStream(
kExampleRouteName, (json) => json, 'example/route',
Expand All @@ -108,7 +109,7 @@ void main() {
..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents')
.completes((it) => it.deepEquals(content.expand((l) => l)))
);
});
}));
}

checkRequest([], 0, filename: null);
Expand All @@ -126,8 +127,8 @@ void main() {
});

test('ApiConnection.delete', () async {
Future<void> checkRequest(Map<String, dynamic>? params, String expectedBody, {bool expectContentType = true}) {
return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
void checkRequest(Map<String, dynamic>? params, String expectedBody, {bool expectContentType = true}) {
finish(FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
connection.prepare(json: {});
await connection.delete(kExampleRouteName, (json) => json, 'example/route', params);
check(connection.lastRequest!).isA<http.Request>()
Expand All @@ -140,7 +141,7 @@ void main() {
'content-type': 'application/x-www-form-urlencoded; charset=utf-8',
})
..body.equals(expectedBody);
});
}));
}

checkRequest(null, '', expectContentType: false);
Expand All @@ -166,13 +167,13 @@ void main() {
});

test('API network errors', () async {
Future<void> checkRequest<T extends Object>(
void checkRequest<T extends Object>(
T exception, Condition<NetworkException> condition) {
return check(tryRequest(exception: exception))
finish(check(tryRequest(exception: exception))
.throws<NetworkException>((it) => it
..routeName.equals(kExampleRouteName)
..cause.equals(exception)
..which(condition));
..which(condition)));
}

final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
Expand Down Expand Up @@ -219,14 +220,14 @@ void main() {
});

test('API 4xx errors, malformed', () async {
Future<void> checkMalformed({
int httpStatus = 400, Map<String, dynamic>? json, String? body}) async {
void checkMalformed({
int httpStatus = 400, Map<String, dynamic>? json, String? body}) {
assert((json == null) != (body == null));
await check(tryRequest(httpStatus: httpStatus, json: json, body: body))
finish(check(tryRequest(httpStatus: httpStatus, json: json, body: body))
.throws<MalformedServerResponseException>((it) => it
..routeName.equals(kExampleRouteName)
..httpStatus.equals(httpStatus)
..data.deepEquals(json));
..data.deepEquals(json)));
}

await check(
Expand Down
18 changes: 18 additions & 0 deletions test/test_async.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'package:test_api/hooks.dart';

/// Ensure the test runner will wait for the given future to complete
/// before considering the current test complete.
///
/// Consider using this function, instead of `await`, when a test invokes
/// a check which is asynchronous and has no interaction with other tasks
/// the test will do later.
///
/// Use `await`, instead of this function, when it matters what order the
/// rest of the test's logic runs in relative to the asynchronous work
/// represented by the given future. In particular, when calling a function
/// that performs setup for later logic in the test, the returned future
/// should always be awaited.
void finish(Future<void> future) {
final outstandingWork = TestHandle.current.markPending();
future.whenComplete(outstandingWork.complete);
}

0 comments on commit 701763d

Please sign in to comment.