api: Write tests for ApiConnection.get, post, postFileFromStream

lib/api/core.dart

@@ -85,7 +85,8 @@ class ApiConnection {
   }
 
   Future<Map<String, dynamic>> postFileFromStream(String route, Stream<List<int>> content, int length, { String? filename }) async {
-    http.MultipartRequest request = http.MultipartRequest('POST', Uri.parse("$realmUrl/api/v1/$route"))
+    final url = realmUrl.replace(path: "/api/v1/$route");
+    final request = http.MultipartRequest('POST', url)
       ..files.add(http.MultipartFile('file', content, length, filename: filename));
     return send(request);
   }

test/api/core_test.dart

@@ -0,0 +1,111 @@
+import 'dart:convert';
+
+import 'package:checks/checks.dart';
+import 'package:http/http.dart' as http;
+import 'package:test/scaffolding.dart';
+import 'package:zulip/api/core.dart';
+
+import '../stdlib_checks.dart';
+import 'fake_api.dart';
+import '../example_data.dart' as eg;
+
+void main() {
+  test('ApiConnection.get', () async {
+    Future<void> checkRequest(Map<String, dynamic>? params, String expectedRelativeUrl) {
+      return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
+        connection.prepare(body: jsonEncode({}));
+        await connection.get('example/route', params);
+        check(connection.lastRequest!).isA<http.Request>()
+          ..method.equals('GET')
+          ..url.asString.equals('${eg.realmUrl.origin}$expectedRelativeUrl')
+          ..headers.deepEquals(authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey))
+          ..body.equals('');
+      });
+    }
+
+    checkRequest(null,             '/api/v1/example/route');
+    checkRequest({},               '/api/v1/example/route?');
+    checkRequest({'x': 3},         '/api/v1/example/route?x=3');
+    checkRequest({'x': 3, 'y': 4}, '/api/v1/example/route?x=3&y=4');
+    checkRequest({'x': null},      '/api/v1/example/route?x=null');
+    checkRequest({'x': true},      '/api/v1/example/route?x=true');
+    checkRequest({'x': 'foo'},     '/api/v1/example/route?x=%22foo%22');
+    checkRequest({'x': [1, 2]},    '/api/v1/example/route?x=%5B1%2C2%5D');
+    checkRequest({'x': {'y': 1}},  '/api/v1/example/route?x=%7B%22y%22%3A1%7D');
+    checkRequest({'x': RawParameter('foo')},
+                                   '/api/v1/example/route?x=foo');
+    checkRequest({'x': RawParameter('foo'), 'y': 'bar'},
+                                   '/api/v1/example/route?x=foo&y=%22bar%22');
+  });
+
+  test('ApiConnection.post', () async {
+    Future<void> checkRequest(Map<String, dynamic>? params, String expectedBody, {bool expectContentType = true}) {
+      return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
+        connection.prepare(body: jsonEncode({}));
+        await connection.post('example/route', params);
+        check(connection.lastRequest!).isA<http.Request>()
+          ..method.equals('POST')
+          ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route')
+          ..headers.deepEquals({
+            ...authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey),
+            if (expectContentType)
+              'content-type': 'application/x-www-form-urlencoded; charset=utf-8',
+          })
+          ..body.equals(expectedBody);
+      });
+    }
+
+    checkRequest(null,                                   '', expectContentType: false);
+    checkRequest({},                                     '');
+    checkRequest({'x': 3},                               'x=3');
+    checkRequest({'x': 3, 'y': 4},                       'x=3&y=4');
+    checkRequest({'x': null},                            'x=null');
+    checkRequest({'x': true},                            'x=true');
+    checkRequest({'x': 'foo'},                           'x=%22foo%22');
+    checkRequest({'x': [1, 2]},                          'x=%5B1%2C2%5D');
+    checkRequest({'x': {'y': 1}},                        'x=%7B%22y%22%3A1%7D');
+    checkRequest({'x': RawParameter('foo')},             'x=foo');
+    checkRequest({'x': RawParameter('foo'), 'y': 'bar'}, 'x=foo&y=%22bar%22');
+  });
+
+  test('ApiConnection.postFileFromStream', () async {
+    Future<void> checkRequest(List<List<int>> content, int length, String? filename) {
+      return FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
+        connection.prepare(body: jsonEncode({}));
+        await connection.postFileFromStream(
+          'example/route',
+          Stream.fromIterable(content), length, filename: filename);
+        check(connection.lastRequest!).isA<http.MultipartRequest>()
+          ..method.equals('POST')
+          ..url.asString.equals('${eg.realmUrl.origin}/api/v1/example/route')
+          ..headers.deepEquals(authHeader(email: eg.selfAccount.email, apiKey: eg.selfAccount.apiKey))
+          ..fields.deepEquals({})
+          ..files.single.which(it()
+            ..field.equals('file')
+            ..length.equals(length)
+            ..filename.equals(filename)
+            ..has<Future<List<int>>>((f) => f.finalize().toBytes(), 'contents')
+              .completes(it()..deepEquals(content.expand((l) => l)))
+          );
+      });
+    }
+
+    checkRequest([], 0, null);
+    checkRequest(['asdf'.codeUnits], 4, null);
+    checkRequest(['asd'.codeUnits, 'f'.codeUnits], 4, null);
+
+    checkRequest(['asdf'.codeUnits], 4, 'info.txt');
+
+    checkRequest(['asdf'.codeUnits], 1, null); // nothing on client side catches a wrong length
+    checkRequest(['asdf'.codeUnits], 100, null);
+  });
+
+  test('API success result', () async {
+    await FakeApiConnection.with_(account: eg.selfAccount, (connection) async {
+      connection.prepare(body: jsonEncode({'result': 'success', 'x': 3}));
+      final result = await connection.get(
+        'example/route', {'y': 'z'});
+      check(result).deepEquals({'result': 'success', 'x': 3});
+    });
+  });
+}

test/api/fake_api.dart

@@ -4,42 +4,78 @@ import 'package:http/http.dart' as http;
 import 'package:zulip/api/core.dart';
 import 'package:zulip/model/store.dart';
 
+import '../example_data.dart' as eg;
+
 /// An [http.Client] that accepts and replays canned responses, for testing.
 class FakeHttpClient extends http.BaseClient {
 
+  http.BaseRequest? lastRequest;
+
   List<int>? _nextResponseBytes;
 
-  // TODO: This mocking API will need to get richer to support all the tests we need.
+  // Please add more features to this mocking API as needed.  For example:
+  //  * preparing an HTTP status other than 200
+  //  * preparing an exception instead of an [http.StreamedResponse]
+  //  * preparing more than one request, and logging more than one request
 
-  void prepare(String response) {
+  void prepare({String? body}) {
     assert(_nextResponseBytes == null,
-        'FakeApiConnection.prepare was called while already expecting a request');
-    _nextResponseBytes = utf8.encode(response);
+      'FakeApiConnection.prepare was called while already expecting a request');
+    _nextResponseBytes = utf8.encode(body ?? '');
   }
 
   @override
   Future<http.StreamedResponse> send(http.BaseRequest request) {
     final responseBytes = _nextResponseBytes!;
     _nextResponseBytes = null;
+    lastRequest = request;
     final byteStream = http.ByteStream.fromBytes(responseBytes);
     return Future.value(http.StreamedResponse(byteStream, 200, request: request));
   }
 }
 
 /// An [ApiConnection] that accepts and replays canned responses, for testing.
 class FakeApiConnection extends ApiConnection {
-  FakeApiConnection({required Uri realmUrl})
-    : this._(realmUrl: realmUrl, client: FakeHttpClient());
+  FakeApiConnection({Uri? realmUrl})
+    : this._(realmUrl: realmUrl ?? eg.realmUrl, client: FakeHttpClient());
 
   FakeApiConnection.fromAccount(Account account)
-    : this(realmUrl: account.realmUrl);
+    : this._(
+        realmUrl: account.realmUrl,
+        email: account.email,
+        apiKey: account.apiKey,
+        client: FakeHttpClient());
 
-  FakeApiConnection._({required Uri realmUrl, required this.client})
-    : super(client: client, realmUrl: realmUrl);
+  FakeApiConnection._({
+    required super.realmUrl,
+    super.email,
+    super.apiKey,
+    required this.client,
+  }) : super(client: client);
 
   final FakeHttpClient client;
 
-  void prepare(String response) {
-    client.prepare(response);
+  /// Run the given callback on a fresh [FakeApiConnection], then close it,
+  /// using try/finally.
+  static Future<T> with_<T>(
+    Future<T> Function(FakeApiConnection connection) fn, {
+    Uri? realmUrl,
+    Account? account,
+  }) async {
+    assert((account == null) || (realmUrl == null));
+    final connection = (account != null)
+      ? FakeApiConnection.fromAccount(account)
+      : FakeApiConnection(realmUrl: realmUrl);
+    try {
+      return fn(connection);
+    } finally {
+      connection.close();
+    }
+  }
+
+  http.BaseRequest? get lastRequest => client.lastRequest;
+
+  void prepare({String? body}) {
+    client.prepare(body: body);
   }
 }

test/api/route/messages_test.dart

@@ -11,15 +11,15 @@ void main() {
   test('sendMessage accepts fixture realm', () async {
     final connection = FakeApiConnection(
         realmUrl: Uri.parse('https://chat.zulip.org/'));
-    connection.prepare(jsonEncode(SendMessageResult(id: 42).toJson()));
+    connection.prepare(body: jsonEncode(SendMessageResult(id: 42).toJson()));
     check(sendMessage(connection, content: 'hello', topic: 'world'))
         .completes(it()..id.equals(42));
   });
 
   test('sendMessage rejects unexpected realm', () async {
     final connection = FakeApiConnection(
         realmUrl: Uri.parse('https://chat.example/'));
-    connection.prepare(jsonEncode(SendMessageResult(id: 42).toJson()));
+    connection.prepare(body: jsonEncode(SendMessageResult(id: 42).toJson()));
     check(sendMessage(connection, content: 'hello', topic: 'world'))
         .throws();
   });

test/stdlib_checks.dart

@@ -0,0 +1,59 @@
+/// `package:checks`-related extensions for the Dart standard library.
+///
+/// Use this file for types in the Dart SDK, as well as in other
+/// packages published by the Dart team that function as
+/// part of the Dart standard library.
+
+import 'dart:convert';
+
+import 'package:checks/checks.dart';
+import 'package:http/http.dart' as http;
+
+extension UriChecks on Subject<Uri> {
+  Subject<String> get asString => has((u) => u.toString(), 'toString'); // TODO(checks): what's a good convention for this?
+
+  Subject<String> get scheme => has((u) => u.scheme, 'scheme');
+  Subject<String> get authority => has((u) => u.authority, 'authority');
+  Subject<String> get userInfo => has((u) => u.userInfo, 'userInfo');
+  Subject<String> get host => has((u) => u.host, 'host');
+  Subject<int> get port => has((u) => u.port, 'port');
+  Subject<String> get path => has((u) => u.path, 'path');
+  Subject<String> get query => has((u) => u.query, 'query');
+  Subject<String> get fragment => has((u) => u.fragment, 'fragment');
+  Subject<List<String>> get pathSegments => has((u) => u.pathSegments, 'pathSegments');
+  Subject<Map<String, String>> get queryParameters => has((u) => u.queryParameters, 'queryParameters');
+  Subject<Map<String, List<String>>> get queryParametersAll => has((u) => u.queryParametersAll, 'queryParametersAll');
+  Subject<bool> get isAbsolute => has((u) => u.isAbsolute, 'isAbsolute');
+  Subject<String> get origin => has((u) => u.origin, 'origin');
+  // TODO hasScheme, other has*, data
+}
+
+extension HttpBaseRequestChecks on Subject<http.BaseRequest> {
+  Subject<String> get method => has((r) => r.method, 'method');
+  Subject<Uri> get url => has((r) => r.url, 'url');
+  Subject<int?> get contentLength => has((r) => r.contentLength, 'contentLength');
+  Subject<Map<String, String>> get headers => has((r) => r.headers, 'headers');
+  // TODO persistentConnection, followRedirects, maxRedirects, finalized
+}
+
+extension HttpRequestChecks on Subject<http.Request> {
+  Subject<int> get contentLength => has((r) => r.contentLength, 'contentLength');
+  Subject<Encoding> get encoding => has((r) => r.encoding, 'encoding');
+  Subject<List<int>> get bodyBytes => has((r) => r.bodyBytes, 'bodyBytes'); // TODO or Uint8List?
+  Subject<String> get body => has((r) => r.body, 'body');
+  Subject<Map<String, String>> get bodyFields => has((r) => r.bodyFields, 'bodyFields');
+}
+
+extension HttpMultipartRequestChecks on Subject<http.MultipartRequest> {
+  Subject<Map<String, String>> get fields => has((r) => r.fields, 'fields');
+  Subject<List<http.MultipartFile>> get files => has((r) => r.files, 'files');
+  Subject<int> get contentLength => has((r) => r.contentLength, 'contentLength');
+}
+
+extension HttpMultipartFileChecks on Subject<http.MultipartFile> {
+  Subject<String> get field => has((f) => f.field, 'field');
+  Subject<int> get length => has((f) => f.length, 'length');
+  Subject<String?> get filename => has((f) => f.filename, 'filename');
+  // TODO Subject<MediaType> get contentType => has((f) => f.contentType, 'contentType');
+  Subject<bool> get isFinalized => has((f) => f.isFinalized, 'isFinalized');
+}

test/widgets/login_test.dart

@@ -2,6 +2,8 @@ import 'package:checks/checks.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/widgets/login.dart';
 
+import '../stdlib_checks.dart';
+
 void main() {
   group('ServerUrlTextEditingController.tryParse', () {
     final controller = ServerUrlTextEditingController();
@@ -11,9 +13,7 @@ void main() {
         controller.text = text;
         final result = controller.tryParse();
         check(result.error).isNull();
-        check(result.url)
-          .isNotNull() // catch `null` here instead of by its .toString()
-          .has((url) => url.toString(), 'toString()').equals(expectedUrl);
+        check(result.url).isNotNull().asString.equals(expectedUrl);
       });
     }