[functions] Dependency-inject HttpClient to enable unit testing with mocks
Change-Id: I5e509f9ae27b7b2d31f10d9f21ba2fdaed6562c7
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/127020
Reviewed-by: Jonas Termansen <sortie@google.com>
diff --git a/functions/node/builder.dart b/functions/node/builder.dart
index ccb97be..03ca510 100644
--- a/functions/node/builder.dart
+++ b/functions/node/builder.dart
@@ -3,9 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
import 'dart:convert';
-import 'package:node_http/node_http.dart' as http;
-// For testing, use instead: import 'package:http/http.dart' as http;
-// TODO(whesse): Inject http Client() through dependency injection.
+
+import 'package:http/http.dart' as http;
import 'firestore.dart';
@@ -51,6 +50,7 @@
/// Tryjob builds are represented by a subclass Tryjob of this class.
class Build {
final FirestoreService firestore;
+ final http.BaseClient httpClient;
final String commitHash;
final Map<String, dynamic> firstResult;
String builderName;
@@ -61,7 +61,7 @@
Statistics stats = Statistics();
- Build(this.commitHash, this.firstResult, this.firestore)
+ Build(this.commitHash, this.firstResult, this.firestore, this.httpClient)
: builderName = firstResult['builder_name'],
buildNumber = int.parse(firstResult['build_number']);
@@ -121,9 +121,7 @@
final range = '$lastHash..master';
final parameters = ['format=JSON', 'topo-order', 'n=1000'];
final url = '$logUrl$range?${parameters.join('&')}';
- final client = http.NodeClient();
- // For testing, use http.Client(); TODO(whesse): Inject correct http.
- final response = await client.get(url);
+ final response = await httpClient.get(url);
final protectedJson = response.body;
if (!protectedJson.startsWith(prefix))
throw Exception('Gerrit response missing prefix $prefix: $protectedJson');
diff --git a/functions/node/gerrit_change.dart b/functions/node/gerrit_change.dart
index 5bb6de0..e9ab4ba 100644
--- a/functions/node/gerrit_change.dart
+++ b/functions/node/gerrit_change.dart
@@ -4,7 +4,7 @@
import 'dart:convert';
-import 'package:node_http/node_http.dart' as http;
+import 'package:http/http.dart' as http;
import 'firestore.dart';
@@ -17,11 +17,12 @@
};
static const prefix = ")]}'\n";
+ http.BaseClient httpClient;
FirestoreService firestore;
String review;
String patchset;
- GerritInfo(int review, int patchset, this.firestore) {
+ GerritInfo(int review, int patchset, this.firestore, this.httpClient) {
this.review = review.toString();
this.patchset = patchset.toString();
}
@@ -29,10 +30,9 @@
// Fetch the owner, changeId, message, and date of a Gerrit change.
Future<void> update() async {
if (await firestore.hasPatchset(review, patchset)) return;
- final client = http.NodeClient();
// Get the Gerrit change's commit from the Gerrit API.
final url = '$gerritUrl/$review?o=ALL_REVISIONS&o=DETAILED_ACCOUNTS';
- final response = await client.get(url);
+ final response = await httpClient.get(url);
final protectedJson = response.body;
if (!protectedJson.startsWith(prefix))
throw Exception('Gerrit response missing prefix $prefix: $protectedJson');
diff --git a/functions/node/index.dart b/functions/node/index.dart
index 4b7735d..1242414 100644
--- a/functions/node/index.dart
+++ b/functions/node/index.dart
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
import 'package:firebase_functions_interop/firebase_functions_interop.dart';
+import 'package:node_http/node_http.dart' as http;
import 'builder.dart';
import 'firestore_impl.dart';
@@ -19,8 +20,10 @@
final String commit = first['commit_hash'];
if (commit.startsWith('refs/changes')) {
- return Tryjob(commit, FirestoreServiceImpl()).process(results);
+ return Tryjob(commit, FirestoreServiceImpl(), http.NodeClient())
+ .process(results);
} else {
- return Build(commit, first, FirestoreServiceImpl()).process(results);
+ return Build(commit, first, FirestoreServiceImpl(), http.NodeClient())
+ .process(results);
}
}
diff --git a/functions/node/test/http_mock.dart b/functions/node/test/http_mock.dart
new file mode 100644
index 0000000..0ce904e
--- /dev/null
+++ b/functions/node/test/http_mock.dart
@@ -0,0 +1,10 @@
+// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:http/http.dart';
+import 'package:mockito/mockito.dart';
+
+import 'test_data.dart';
+
+class HttpClientMock extends Mock implements BaseClient {}
diff --git a/functions/node/test/test.dart b/functions/node/test/test.dart
index dcbe90a..912007f 100644
--- a/functions/node/test/test.dart
+++ b/functions/node/test/test.dart
@@ -7,14 +7,16 @@
import '../builder.dart';
import 'firestore_mock.dart';
+import 'http_mock.dart';
import 'test_data.dart';
void main() async {
test("Store already existing commit", () async {
+ final client = HttpClientMock();
final firestore = FirestoreServiceMock();
final change = alreadyExistingCommitChange;
- final builder = Build(existingCommitHash, change, firestore);
+ final builder = Build(existingCommitHash, change, firestore, client);
await builder.storeBuildCommitsInfo();
expect(builder.endIndex, existingCommit['index']);
expect(builder.startIndex, previousCommit['index'] + 1);
diff --git a/functions/node/tryjob.dart b/functions/node/tryjob.dart
index 7158d7f..f72504e 100644
--- a/functions/node/tryjob.dart
+++ b/functions/node/tryjob.dart
@@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
+import 'package:http/http.dart' as http show BaseClient;
+
import 'firestore.dart';
import 'gerrit_change.dart';
@@ -10,18 +12,19 @@
class Tryjob {
static final changeRefRegExp = RegExp(r'refs/changes/(\d*)/(\d*)');
+ http.BaseClient httpClient;
FirestoreService firestore;
int review;
int patchset;
- Tryjob(String changeRef, this.firestore) {
+ Tryjob(String changeRef, this.firestore, this.httpClient) {
final match = changeRefRegExp.matchAsPrefix(changeRef);
review = int.parse(match[1]);
patchset = int.parse(match[2]);
}
Future<void> update() {
- return GerritInfo(review, patchset, firestore).update();
+ return GerritInfo(review, patchset, firestore, httpClient).update();
}
Future<void> process(List<Map<String, dynamic>> results) async {
