[cloud functions] Refactor cloud functions to allow unit testing with mocks
This change factors out all the access to the Firestore database into a
FirestoreService class, which is replaced by a mock during unit testing.
It also groups related functions with shared state into classes.
Change-Id: I1ce40a91135a5ddb4befcd985c6f6ba87990ff4b
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/126740
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/functions/node/builder.dart b/functions/node/builder.dart
new file mode 100644
index 0000000..75e095c
--- /dev/null
+++ b/functions/node/builder.dart
@@ -0,0 +1,176 @@
+// 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 '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 'firestore.dart';
+
+void info(Object message) {
+ print("Info: $message");
+}
+
+void error(Object message) {
+ print("Error: $message");
+}
+
+const prefix = ")]}'\n";
+
+bool isChangedResult(Map<String, dynamic> result) =>
+ result['changed'] && !result['flaky'] && !result['previous_flaky'];
+
+const months = const {
+ 'Jan': '01',
+ 'Feb': '02',
+ 'Mar': '03',
+ 'Apr': '04',
+ 'May': '05',
+ 'Jun': '06',
+ 'Jul': '07',
+ 'Aug': '08',
+ 'Sep': '09',
+ 'Oct': '10',
+ 'Nov': '11',
+ 'Dec': '12'
+};
+
+DateTime parseGitilesDateTime(String gitiles) {
+ final parts = gitiles.split(' ');
+ final year = parts[4];
+ final month = months[parts[1]];
+ final day = parts[2].padLeft(2, '0');
+ return DateTime.parse('$year-$month-$day ${parts[3]} ${parts[5]}');
+}
+
+/// A Builder holds information about a CI build, and can
+/// store the changed results from that build, using an injected
+/// Firestore() object.
+/// Tryjob builds are represented by a subclass Tryjob of this class.
+class Build {
+ final FirestoreService firestore;
+ final String commitHash;
+ final Map<String, dynamic> firstResult;
+ String builderName;
+ int buildNumber;
+ int startIndex;
+ int endIndex;
+
+ Statistics stats = Statistics();
+
+ Build(this.commitHash, this.firstResult, this.firestore)
+ : builderName = firstResult['builder_name'],
+ buildNumber = int.parse(firstResult['build_number']);
+
+ Future<void> process(List<Map<String, dynamic>> results) async {
+ final configurations =
+ results.map((change) => change['configuration'] as String).toSet();
+ await update(configurations);
+ await Future.forEach(
+ results.where(isChangedResult), (result) => storeChange(result));
+ stats.report();
+ }
+
+ Future<void> update(Iterable<String> configurations) async {
+ await storeBuildCommitsInfo();
+ await storeConfigurationsInfo(configurations);
+ await firestore.updateBuildInfo(builderName, buildNumber, endIndex);
+ }
+
+ /// Stores the commit info for the blamelist of result.
+ /// If the info is already there does nothing.
+ /// Saves the commit indices of the start and end of the blamelist.
+ Future<void> storeBuildCommitsInfo() async {
+ // Get indices of change. Range includes startIndex and endIndex.
+ var commit = await firestore.getCommit(commitHash);
+ if (commit == null) {
+ await getMissingCommits();
+ commit = await firestore.getCommit(commitHash);
+ if (commit == null) {
+ error('Result received with unknown commit hash $commitHash');
+ }
+ }
+ endIndex = commit['index'];
+ // If this is a new builder, use the current commit as a trivial blamelist.
+ if (firstResult['previous_commit_hash'] == null) {
+ startIndex = endIndex;
+ } else {
+ commit = await firestore.getCommit(firstResult['previous_commit_hash']);
+ startIndex = commit['index'] + 1;
+ }
+ }
+
+ Future<void> getMissingCommits() async {
+ final lastCommit = await firestore.getLastCommit(commitHash);
+ final lastHash = lastCommit['id'];
+ final lastIndex = lastCommit['index'];
+
+ final logUrl = 'https://dart.googlesource.com/sdk/+log/';
+ 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 protectedJson = response.body;
+ if (!protectedJson.startsWith(prefix))
+ throw Exception('Gerrit response missing prefix $prefix: $protectedJson');
+ final commits = jsonDecode(protectedJson.substring(prefix.length))['log']
+ as List<dynamic>;
+ if (commits.isEmpty) {
+ info('Found no new commits between $lastHash and master');
+ return;
+ }
+ stats.commitsFetched = commits.length;
+ final first = commits.last as Map<String, dynamic>;
+ if (first['parents'].first != lastHash) {
+ error('First new commit ${first['parents'].first} is not'
+ ' a child of last known commit $lastHash when fetching new commits');
+ throw ('First new commit ${first['parents'].first} is not'
+ ' a child of last known commit $lastHash when fetching new commits');
+ }
+ if (!commits.any((commit) => commit['commit'] == commitHash)) {
+ info('Did not find commit $commitHash when fetching new commits');
+ return;
+ }
+ var index = lastIndex + 1;
+ for (Map<String, dynamic> commit in commits.reversed) {
+ await firestore.addCommit(commit['commit'], {
+ 'author': commit['author']['email'],
+ 'created': parseGitilesDateTime(commit['committer']['time']),
+ 'index': index,
+ 'title': commit['message'].split('\n').first
+ });
+ ++index;
+ }
+ }
+
+ Future<void> storeConfigurationsInfo(Iterable<String> configurations) async {
+ for (final configuration in configurations) {
+ await firestore.updateConfiguration(configuration, builderName);
+ }
+ }
+
+ Future<void> storeChange(Map<String, dynamic> change) async {
+ firestore.storeChange(change, startIndex, endIndex);
+ stats.changes++;
+ }
+}
+
+class Statistics {
+ int results = 0;
+ int changes = 0;
+ int newRecords = 0;
+ int modifiedRecords = 0;
+ int commitsFetched = 0;
+
+ void report() {
+ info("Number of changed results processed: $changes");
+ info("Number of results processed: $results");
+ info("Number of firestore records produced: $newRecords");
+ info("Number of firestore records modified: $modifiedRecords");
+ info("Number of commits fetched: $commitsFetched");
+ }
+}
diff --git a/functions/node/firestore.dart b/functions/node/firestore.dart
new file mode 100644
index 0000000..36c99fd
--- /dev/null
+++ b/functions/node/firestore.dart
@@ -0,0 +1,31 @@
+// 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.
+
+/// FirestoreService is implemented by FirestoreServiceImpl, for production
+/// use, and by FirestoreServiceMock, for testing.
+/// The implementation must be in a separate file, so that testing can
+/// run on Dart native, not just on the nodeJS platform.
+abstract class FirestoreService {
+ Future<bool> hasPatchset(String review, String patchset);
+
+ Future<Map<String, dynamic>> getCommit(String hash);
+
+ Future<Map<String, dynamic>> getLastCommit(String hash);
+
+ Future<void> addCommit(String id, Map<String, dynamic> data);
+
+ Future<void> updateConfiguration(String configuration, String builder);
+
+ Future<void> updateBuildInfo(String builder, int buildNumber, int index);
+
+ Future<void> storeChange(
+ Map<String, dynamic> change, int startIndex, int endIndex);
+
+ Future<void> storeTryChange(
+ Map<String, dynamic> change, int review, int patchset);
+
+ Future<void> storeReview(String review, Map<String, dynamic> data);
+ Future<void> storePatchset(
+ String review, String patchset, Map<String, dynamic> data);
+}
diff --git a/functions/node/firestore_impl.dart b/functions/node/firestore_impl.dart
new file mode 100644
index 0000000..2f8b457
--- /dev/null
+++ b/functions/node/firestore_impl.dart
@@ -0,0 +1,203 @@
+// 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 'dart:math' show max, min;
+import 'package:firebase_functions_interop/firebase_functions_interop.dart';
+
+import 'firestore.dart';
+
+void info(Object message) {
+ print("Info: $message");
+}
+
+void error(Object message) {
+ print("Error: $message");
+}
+
+// Cloud functions run the cloud function many times in the same isolate.
+// Use static initializer to run global initialization once.
+Firestore firestore = createFirestore();
+
+Firestore createFirestore() {
+ final app = FirebaseAdmin.instance.initializeApp();
+ return app.firestore()
+ ..settings(FirestoreSettings(timestampsInSnapshots: true));
+}
+
+class FirestoreServiceImpl implements FirestoreService {
+ Future<bool> hasPatchset(String review, String patchset) => firestore
+ .document('reviews/$review/patchsets/$patchset')
+ .get()
+ .then((s) => s.exists);
+
+ Future<Map<String, dynamic>> getCommit(String hash) => firestore
+ .document('commits/$hash')
+ .get()
+ .then((s) => s.exists ? s.data.toMap() : null);
+
+ Future<Map<String, dynamic>> getLastCommit(String hash) async {
+ QuerySnapshot lastCommit = await firestore
+ .collection('commits')
+ .orderBy('index', descending: true)
+ .limit(1)
+ .get();
+ final result = lastCommit.documents.first.data.toMap();
+ result['id'] = lastCommit.documents.first.documentID;
+ return result;
+ }
+
+ Future<void> addCommit(String id, Map<String, dynamic> data) async {
+ data['created'] = Timestamp.fromDateTime(data['created']);
+ final docRef = firestore.document('commits/$id');
+ await docRef.setData(DocumentData.fromMap(data));
+ }
+
+ Future<void> updateConfiguration(String configuration, String builder) async {
+ final record =
+ await firestore.document('configurations/$configuration').get();
+ if (!record.exists || record.data.getString('builder') != builder) {
+ await firestore
+ .document('configurations/$configuration')
+ .setData(DocumentData.fromMap({'builder': builder}));
+ if (!record.exists) {
+ info('Configuration document $configuration -> $builder created');
+ } else {
+ info('Configuration document changed: $configuration -> $builder '
+ '(was ${record.data.getString("builder")}');
+ }
+ }
+ }
+
+ Future<void> updateBuildInfo(
+ String builder, int buildNumber, int index) async {
+ final documentRef = firestore.document('builds/$builder:$index');
+ final record = await documentRef.get();
+ if (!record.exists) {
+ await documentRef.setData(DocumentData.fromMap(
+ {'builder': builder, 'build_number': buildNumber, 'index': index}));
+ info('Created build record: '
+ 'builder: $builder, build_number: $buildNumber, index: $index');
+ } else if (record.data.getInt('index') != index) {
+ error(
+ 'Build $buildNumber of $builder had commit index ${record.data.getInt('index')},'
+ 'should be $index.');
+ }
+ }
+
+ Future<void> storeChange(
+ Map<String, dynamic> change, int startIndex, int endIndex) async {
+ String name = change['name'];
+ String result = change['result'];
+ String previousResult = change['previous_result'] ?? 'new test';
+ QuerySnapshot snapshot = await firestore
+ .collection('results')
+ .orderBy('blamelist_end_index', descending: true)
+ .where('name', isEqualTo: name)
+ .where('result', isEqualTo: result)
+ .where('previous_result', isEqualTo: previousResult)
+ .where('expected', isEqualTo: change['expected'])
+ .limit(5) // We will pick the right one, probably the latest.
+ .get();
+
+ // Find an existing change group with a blamelist that intersects this change.
+
+ bool blamelistIncludesChange(DocumentSnapshot groupDocument) {
+ var group = groupDocument.data;
+ var groupStart = group.getInt('blamelist_start_index');
+ var groupEnd = group.getInt('blamelist_end_index');
+ return startIndex <= groupEnd && endIndex >= groupStart;
+ }
+
+ DocumentSnapshot group = snapshot.documents
+ .firstWhere(blamelistIncludesChange, orElse: () => null);
+
+ if (group == null) {
+ info("Adding group for $name");
+ return firestore.collection('results').add(DocumentData.fromMap({
+ 'name': name,
+ 'result': result,
+ 'previous_result': previousResult,
+ 'expected': change['expected'],
+ 'blamelist_start_index': startIndex,
+ 'blamelist_end_index': endIndex,
+ 'trivial_blamelist': startIndex == endIndex,
+ 'configurations': <String>[change['configuration']]
+ }));
+ }
+
+ // Update the change group in a transaction.
+ // Add new configuration and narrow the blamelist.
+ Future<void> updateGroup(Transaction transaction) async {
+ final DocumentSnapshot groupSnapshot =
+ await transaction.get(group.reference);
+ final data = groupSnapshot.data;
+ final newStart = max(startIndex, data.getInt('blamelist_start_index'));
+ final newEnd = min(endIndex, data.getInt('blamelist_end_index'));
+ final updateMap = <String, dynamic>{
+ 'blamelist_start_index':
+ max(startIndex, data.getInt('blamelist_start_index')),
+ 'blamelist_end_index': min(endIndex, data.getInt('blamelist_end_index'))
+ };
+ updateMap['trivial_blamelist'] = (updateMap['blamelist_start_index'] ==
+ updateMap['blamelist_end_index']);
+ final update = UpdateData.fromMap({
+ 'blamelist_start_index': newStart,
+ 'blamelist_end_index': newEnd,
+ 'trivial_blamelist': newStart == newEnd
+ });
+ update.setFieldValue('configurations',
+ Firestore.fieldValues.arrayUnion([change['configuration']]));
+ group.reference.updateData(update);
+ }
+
+ return firestore.runTransaction(updateGroup);
+ }
+
+ Future<void> storeTryChange(
+ Map<String, dynamic> change, int review, int patchset) async {
+ String name = change['name'];
+ String result = change['result'];
+ String expected = change['expected'];
+ String reviewPath = change['commit_hash'];
+ String previousResult = change['previous_result'] ?? 'new test';
+ QuerySnapshot snapshot = await firestore
+ .collection('try_results')
+ .where('review_path', isEqualTo: reviewPath)
+ .where('name', isEqualTo: name)
+ .where('result', isEqualTo: result)
+ .where('previous_result', isEqualTo: previousResult)
+ .where('expected', isEqualTo: expected)
+ .limit(1)
+ .get();
+
+ if (snapshot.isEmpty) {
+ info("Adding group for $name");
+
+ return firestore.collection('try_results').add(DocumentData.fromMap({
+ 'name': name,
+ 'result': result,
+ 'previous_result': previousResult,
+ 'expected': expected,
+ 'review_path': reviewPath,
+ 'review': review,
+ 'patchset': patchset,
+ 'configurations': <String>[change['configuration']]
+ }));
+ } else {
+ final update = UpdateData()
+ ..setFieldValue('configurations',
+ Firestore.fieldValues.arrayUnion([change['configuration']]));
+ snapshot.documents.first.reference.updateData(update);
+ }
+ }
+
+ Future<void> storeReview(String review, Map<String, dynamic> data) =>
+ firestore.document('reviews/$review').setData(DocumentData.fromMap(data));
+
+ Future<void> storePatchset(
+ String review, String patchset, Map<String, dynamic> data) =>
+ firestore
+ .document('reviews/$review/patchsets/$patchset')
+ .setData(DocumentData.fromMap(data));
+}
diff --git a/functions/node/gerrit_change.dart b/functions/node/gerrit_change.dart
index b061dec..5bb6de0 100644
--- a/functions/node/gerrit_change.dart
+++ b/functions/node/gerrit_change.dart
@@ -4,12 +4,12 @@
import 'dart:convert';
-import 'package:firebase_functions_interop/firebase_functions_interop.dart';
import 'package:node_http/node_http.dart' as http;
+import 'firestore.dart';
+
class GerritInfo {
static const gerritUrl = 'https://dart-review.googlesource.com/changes';
- static final reviewRefRegExp = RegExp(r'refs/changes/(\d*)/(\d*)');
static const trivialKinds = const {
'TRIVIAL_REBASE',
'NO_CHANGE',
@@ -17,23 +17,19 @@
};
static const prefix = ")]}'\n";
- Firestore firestore;
+ FirestoreService firestore;
String review;
String patchset;
- GerritInfo(String changeRef, this.firestore) {
- final match = reviewRefRegExp.matchAsPrefix(changeRef);
- review = match[1];
- patchset = match[2];
+ GerritInfo(int review, int patchset, this.firestore) {
+ this.review = review.toString();
+ this.patchset = patchset.toString();
}
// 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();
- var patchsetRef = firestore.document('reviews/$review/patchsets/$patchset');
- DocumentSnapshot patchsetDoc = await patchsetRef.get();
- if (patchsetDoc.exists) return;
-
// 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);
@@ -42,9 +38,7 @@
throw Exception('Gerrit response missing prefix $prefix: $protectedJson');
final reviewInfo = jsonDecode(protectedJson.substring(prefix.length))
as Map<String, dynamic>;
- final reviewRef = firestore.document('reviews/$review');
- await reviewRef
- .setData(DocumentData.fromMap({'subject': reviewInfo['subject']}));
+ await firestore.storeReview(review, {'subject': reviewInfo['subject']});
// Add the patchset information to the patchsets subcollection.
final revisions = reviewInfo['revisions'].values.toList()
@@ -55,14 +49,12 @@
if (!trivialKinds.contains(revision['kind'])) {
patchsetGroupFirst = number;
}
- patchsetRef = firestore.document('reviews/$review/patchsets/$number');
-
- await patchsetRef.setData(DocumentData.fromMap({
+ await firestore.storePatchset(review, number, {
'number': number,
'patchset_group': patchsetGroupFirst,
'description': revision['description'],
'kind': revision['kind']
- }));
+ });
}
}
}
diff --git a/functions/node/index.dart b/functions/node/index.dart
index a1ee928..4b7735d 100644
--- a/functions/node/index.dart
+++ b/functions/node/index.dart
@@ -2,334 +2,25 @@
// 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 'dart:convert';
-import 'dart:math' show max, min;
-
import 'package:firebase_functions_interop/firebase_functions_interop.dart';
-import 'package:node_http/node_http.dart' as http;
-import 'gerrit_change.dart';
-
-const prefix = ")]}'\n";
-
-void info(Object message) {
- print("Info: $message");
-}
-
-void error(Object message) {
- print("Error: $message");
-}
+import 'builder.dart';
+import 'firestore_impl.dart';
+import 'tryjob.dart';
void main() {
functions['receiveChanges'] =
functions.pubsub.topic('results').onPublish(receiveChanges);
}
-// Cloud functions run the cloud function many times in the same isolate.
-// Use static initializer to run global initialization once.
-Firestore firestore = createFirestore();
-
-Firestore createFirestore() {
- final app = FirebaseAdmin.instance.initializeApp();
- return app.firestore()
- ..settings(FirestoreSettings(timestampsInSnapshots: true));
-}
-
-bool isChangedResult(Map<String, dynamic> result) =>
- result['changed'] && !result['flaky'] && !result['previous_flaky'];
-
Future<void> receiveChanges(Message message, EventContext context) async {
- final stats = Statistics();
final results = (message.json as List).cast<Map<String, dynamic>>();
final first = results.first;
final String commit = first['commit_hash'];
- final String builder = first['builder_name'];
- final int buildNumber = int.parse(first['build_number']);
if (commit.startsWith('refs/changes')) {
- await GerritInfo(commit, firestore).update();
- await Future.forEach(
- results.where(isChangedResult), (result) => storeTryChange(result));
+ return Tryjob(commit, FirestoreServiceImpl()).process(results);
} else {
- var blamelist = await storeBuildCommitsInfo(first, stats);
- final configurations =
- results.map((change) => change['configuration'] as String).toSet();
- await storeConfigurationsInfo(builder, configurations);
- await storeBuildInfo(builder, buildNumber, blamelist['endIndex']);
- await Future.forEach(results.where(isChangedResult),
- (result) => storeChange(result, blamelist, stats));
+ return Build(commit, first, FirestoreServiceImpl()).process(results);
}
- stats.report();
-}
-
-class Statistics {
- int results = 0;
- int changes = 0;
- int newRecords = 0;
- int modifiedRecords = 0;
- int commitsFetched = 0;
- String builder;
- int buildNumber;
-
- void report() {
- info("Number of changed results processed: $changes");
- info("Number of results processed: $results");
- info("Number of firestore records produced: $newRecords");
- info("Number of firestore records modified: $modifiedRecords");
- info("Number of commits fetched: $commitsFetched");
- }
-}
-
-/// Stores the commit info for the blamelist of result.
-/// If the info is already there does nothing.
-/// Returns the commit indices of the start and end of the blamelist.
-Future<Map<String, int>> storeBuildCommitsInfo(
- Map<String, dynamic> result, Statistics stats) async {
- // Get indices of change. Range includes startIndex and endIndex.
- final hash = result['commit_hash'] as String;
- final docRef = await firestore.document('commits/$hash');
- var docSnapshot = await docRef.get();
- if (!docSnapshot.exists) {
- await getMissingCommits(hash, stats);
- docSnapshot = await docRef.get();
- if (!docSnapshot.exists) {
- error('Result received with unknown commit hash $hash');
- }
- }
- final endIndex = docSnapshot.data.getInt('index');
- // If this is a new builder, use the current commit as a trivial blamelist.
- var startIndex = endIndex;
- if (result['previous_commit_hash'] != null) {
- final commit = await firestore
- .document('commits/${result['previous_commit_hash']}')
- .get();
- startIndex = commit.data.getInt('index') + 1;
- }
- return {"startIndex": startIndex, "endIndex": endIndex};
-}
-
-Future<void> storeConfigurationsInfo(
- String builder, Iterable<String> configurations) async {
- for (final configuration in configurations) {
- final record =
- await firestore.document('configurations/$configuration').get();
- if (!record.exists || record.data.getString('builder') != builder) {
- await firestore
- .document('configurations/$configuration')
- .setData(DocumentData.fromMap({'builder': builder}));
- if (!record.exists) {
- info('Configuration document $configuration -> $builder created');
- } else {
- info('Configuration document changed: $configuration -> $builder '
- '(was ${record.data.getString("builder")}');
- }
- }
- }
-}
-
-Future<void> storeBuildInfo(
- String builder, int buildNumber, int commitIndex) async {
- final documentRef = firestore.document('builds/$builder:$commitIndex');
- final record = await documentRef.get();
- if (!record.exists) {
- await documentRef.setData(DocumentData.fromMap({
- 'builder': builder,
- 'build_number': buildNumber,
- 'index': commitIndex
- }));
- info('Created build record: '
- 'builder: $builder, build_number: $buildNumber, index: $commitIndex');
- } else if (record.data.getInt('index') != commitIndex) {
- error(
- 'Build $buildNumber of $builder had commit index ${record.data.getInt('index')},'
- 'should be $commitIndex.');
- }
-}
-
-Future<void> storeChange(Map<String, dynamic> change,
- Map<String, int> buildInfo, Statistics stats) async {
- stats.changes++;
- String name = change['name'];
- String result = change['result'];
- String previousResult = change['previous_result'] ?? 'new test';
- QuerySnapshot snapshot = await firestore
- .collection('results')
- .orderBy('blamelist_end_index', descending: true)
- .where('name', isEqualTo: name)
- .where('result', isEqualTo: result)
- .where('previous_result', isEqualTo: previousResult)
- .where('expected', isEqualTo: change['expected'])
- .limit(5) // We will pick the right one, probably the latest.
- .get();
-
- // Find an existing change group with a blamelist that intersects this change.
- final startIndex = buildInfo['startIndex'];
- final endIndex = buildInfo['endIndex'];
-
- bool blamelistIncludesChange(DocumentSnapshot groupDocument) {
- var group = groupDocument.data;
- var groupStart = group.getInt('blamelist_start_index');
- var groupEnd = group.getInt('blamelist_end_index');
- return startIndex <= groupEnd && endIndex >= groupStart;
- }
-
- DocumentSnapshot group = snapshot.documents
- .firstWhere(blamelistIncludesChange, orElse: () => null);
-
- if (group == null) {
- info("Adding group for $name");
- return firestore.collection('results').add(DocumentData.fromMap({
- 'name': name,
- 'result': result,
- 'previous_result': previousResult,
- 'expected': change['expected'],
- 'blamelist_start_index': startIndex,
- 'blamelist_end_index': endIndex,
- 'trivial_blamelist': startIndex == endIndex,
- 'configurations': <String>[change['configuration']]
- }));
- }
-
- // Update the change group in a transaction.
- // Add new configuration and narrow the blamelist.
- Future<void> updateGroup(Transaction transaction) async {
- final DocumentSnapshot groupSnapshot =
- await transaction.get(group.reference);
- final data = groupSnapshot.data;
- final newStart = max(startIndex, data.getInt('blamelist_start_index'));
- final newEnd = min(endIndex, data.getInt('blamelist_end_index'));
- final updateMap = <String, dynamic>{
- 'blamelist_start_index':
- max(startIndex, data.getInt('blamelist_start_index')),
- 'blamelist_end_index': min(endIndex, data.getInt('blamelist_end_index'))
- };
- updateMap['trivial_blamelist'] = (updateMap['blamelist_start_index'] ==
- updateMap['blamelist_end_index']);
- final update = UpdateData.fromMap({
- 'blamelist_start_index': newStart,
- 'blamelist_end_index': newEnd,
- 'trivial_blamelist': newStart == newEnd
- });
- update.setFieldValue('configurations',
- Firestore.fieldValues.arrayUnion([change['configuration']]));
- group.reference.updateData(update);
- }
-
- return firestore.runTransaction(updateGroup);
-}
-
-Future<void> storeTryChange(Map<String, dynamic> change) async {
- String name = change['name'];
- String result = change['result'];
- String expected = change['expected'];
- String reviewPath = change['commit_hash'];
- String previousResult = change['previous_result'] ?? 'new test';
- QuerySnapshot snapshot = await firestore
- .collection('try_results')
- .where('review_path', isEqualTo: reviewPath)
- .where('name', isEqualTo: name)
- .where('result', isEqualTo: result)
- .where('previous_result', isEqualTo: previousResult)
- .where('expected', isEqualTo: expected)
- .limit(1)
- .get();
-
- if (snapshot.isEmpty) {
- info("Adding group for $name");
-
- final reviewInfo = GerritInfo(reviewPath, firestore);
- int review = int.parse(reviewInfo.review);
- int patchset = int.parse(reviewInfo.patchset);
-
- return firestore.collection('try_results').add(DocumentData.fromMap({
- 'name': name,
- 'result': result,
- 'previous_result': previousResult,
- 'expected': expected,
- 'review_path': reviewPath,
- 'review': review,
- 'patchset': patchset,
- 'configurations': <String>[change['configuration']]
- }));
- } else {
- final update = UpdateData()
- ..setFieldValue('configurations',
- Firestore.fieldValues.arrayUnion([change['configuration']]));
- snapshot.documents.first.reference.updateData(update);
- }
-}
-
-Future<void> getMissingCommits(String hash, Statistics stats) async {
- final client = http.NodeClient();
- final QuerySnapshot lastCommit = await firestore
- .collection('commits')
- .orderBy('index', descending: true)
- .limit(1)
- .get();
- final lastHash = lastCommit.documents.first.documentID;
- final lastIndex = lastCommit.documents.first.data.getInt('index');
-
- final logUrl = 'https://dart.googlesource.com/sdk/+log/';
- final range = '$lastHash..master';
- final parameters = ['format=JSON', 'topo-order', 'n=1000'];
- final url = '$logUrl$range?${parameters.join('&')}';
- final response = await client.get(url);
- final protectedJson = response.body;
- if (!protectedJson.startsWith(prefix))
- throw Exception('Gerrit response missing prefix $prefix: $protectedJson');
- final commits = jsonDecode(protectedJson.substring(prefix.length))['log']
- as List<dynamic>;
- if (commits.isEmpty) {
- info('Found no new commits between $lastHash and master');
- return;
- }
- stats.commitsFetched = commits.length;
- final first = commits.last as Map<String, dynamic>;
- if (first['parents'].first != lastHash) {
- error('First new commit ${first['parents'].first} is not'
- ' a child of last known commit $lastHash when fetching new commits');
- throw ('First new commit ${first['parents'].first} is not'
- ' a child of last known commit $lastHash when fetching new commits');
- }
- if (!commits.any((commit) => commit['commit'] == hash)) {
- info('Did not find commit $hash when fetching new commits');
- return;
- }
- var index = lastIndex + 1;
- for (Map<String, dynamic> commit in commits.reversed) {
- // Create new commit document for this commit.
- final docRef = firestore.document('commits/${commit['commit']}');
- final data = DocumentData.fromMap({
- 'author': commit['author']['email'],
- 'created': Timestamp.fromDateTime(
- parseGitilesDateTime(commit['committer']['time'])),
- 'index': index,
- 'title': commit['message'].split('\n').first
- });
- await docRef.setData(data);
- ++index;
- }
-}
-
-const months = const {
- 'Jan': '01',
- 'Feb': '02',
- 'Mar': '03',
- 'Apr': '04',
- 'May': '05',
- 'Jun': '06',
- 'Jul': '07',
- 'Aug': '08',
- 'Sep': '09',
- 'Oct': '10',
- 'Nov': '11',
- 'Dec': '12'
-};
-
-DateTime parseGitilesDateTime(String gitiles) {
- final parts = gitiles.split(' ');
- final year = parts[4];
- final month = months[parts[1]];
- final day = parts[2].padLeft(2, '0');
- return DateTime.parse('$year-$month-$day ${parts[3]} ${parts[5]}');
}
diff --git a/functions/node/test/firestore_mock.dart b/functions/node/test/firestore_mock.dart
new file mode 100644
index 0000000..1bbd969
--- /dev/null
+++ b/functions/node/test/firestore_mock.dart
@@ -0,0 +1,17 @@
+// 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:mockito/mockito.dart';
+
+import '../firestore.dart';
+import 'test_data.dart';
+
+class FirestoreServiceMock extends Mock implements FirestoreService {
+ FirestoreServiceMock() {
+ when(this.getCommit(existingCommitHash))
+ .thenAnswer((_) => Future.value(existingCommit));
+ when(this.getCommit(previousCommitHash))
+ .thenAnswer((_) => Future.value(previousCommit));
+ }
+}
diff --git a/functions/node/test/test.dart b/functions/node/test/test.dart
index 8c99f6d..85a83fe 100644
--- a/functions/node/test/test.dart
+++ b/functions/node/test/test.dart
@@ -4,68 +4,26 @@
import 'dart:convert';
-import '../index.dart';
-import '../gerrit_change.dart';
+import 'package:mockito/mockito.dart';
+import 'package:test/test.dart';
-String resultJson = '''
-{
-"name":"dart2js_extra/local_function_signatures_strong_test/none",
-"configuration":"dart2js-new-rti-linux-x64-d8",
-"suite":"dart2js_extra",
-"test_name":"local_function_signatures_strong_test/none",
-"time_ms":2384,
-"result":"Pass",
-"expected":"Pass",
-"matches":true,
-"bot_name":"luci-dart-try-xenial-70-8fkh",
-"commit_hash":"9cd47ac2e6ac024e5a0fd1d5667d94a8c2fd2d5e",
-"commit_time":1563576771,
-"build_number":"307",
-"builder_name":"dart2js-rti-linux-x64-d8",
-"flaky":false,
-"previous_flaky":false,
-"previous_result":"RuntimeError",
-"previous_commit_hash":"ebc180be95efd89b8745ddffcedf970af6e36dc1",
-"previous_commit_time":1563576211,
-"previous_build_number":"306",
-"changed":true
-}
-''';
-
-String tryResultJson = '''
-{
-"name":"dart2js_extra/local_function_signatures_strong_test/none",
-"configuration":"dart2js-new-rti-linux-x64-d8",
-"suite":"dart2js_extra",
-"test_name":"local_function_signatures_strong_test/none",
-"time_ms":2384,
-"result":"Pass",
-"expected":"Pass",
-"matches":true,
-"bot_name":"luci-dart-try-xenial-70-8fkh",
-"commit_hash":"refs/changes/12345/2",
-"commit_time":1563576771,
-"build_number":"307",
-"builder_name":"dart2js-rti-linux-x64-d8",
-"flaky":false,
-"previous_flaky":false,
-"previous_result":"RuntimeError",
-"previous_commit_hash":"ebc180be95efd89b8745ddffcedf970af6e36dc1",
-"previous_commit_time":1563576211,
-"previous_build_number":"306",
-"changed":true
-}
-''';
+import '../builder.dart';
+import 'firestore_mock.dart';
+import 'test_data.dart';
void main() async {
- print("starting test");
- Map<String, dynamic> result = jsonDecode(resultJson);
- final stats = Statistics();
- final info = await storeBuildCommitsInfo(result, stats);
- print("info: $info");
- stats.report();
- await storeChange(result, info, stats);
- stats.report();
- await storeTryChange(jsonDecode(tryResultJson) as Map<String, dynamic>);
- await GerritInfo('refs/changes/119860/5', firestore).update();
+ test("Store already existing commit", () async {
+ final firestore = FirestoreServiceMock();
+ final change = alreadyExistingCommitChange;
+ final hash = change['commit_hash'];
+
+ final builder = Build(existingCommitHash, change, firestore);
+ await builder.storeBuildCommitsInfo();
+ expect(builder.endIndex, existingCommit['index']);
+ expect(builder.startIndex, previousCommit['index'] + 1);
+ verifyInOrder([
+ firestore.getCommit(existingCommitHash),
+ firestore.getCommit(previousCommitHash)
+ ]);
+ });
}
diff --git a/functions/node/test/test_data.dart b/functions/node/test/test_data.dart
new file mode 100644
index 0000000..140ce6f
--- /dev/null
+++ b/functions/node/test/test_data.dart
@@ -0,0 +1,45 @@
+// 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 'dart:core';
+
+const Map<String, dynamic> alreadyExistingCommitChange = {
+ "name": "dart2js_extra/local_function_signatures_strong_test/none",
+ "configuration": "dart2js-new-rti-linux-x64-d8",
+ "suite": "dart2js_extra",
+ "test_name": "local_function_signatures_strong_test/none",
+ "time_ms": 2384,
+ "result": "Pass",
+ "expected": "Pass",
+ "matches": true,
+ "bot_name": "luci-dart-try-xenial-70-8fkh",
+ "commit_hash": existingCommitHash,
+ "commit_time": 1563576771,
+ "build_number": "307",
+ "builder_name": "dart2js-rti-linux-x64-d8",
+ "flaky": false,
+ "previous_flaky": false,
+ "previous_result": "RuntimeError",
+ "previous_commit_hash": previousCommitHash,
+ "previous_commit_time": 1563576211,
+ "previous_build_number": "306",
+ "changed": true
+};
+
+const String existingCommitHash = 'an already existing commit hash';
+
+Map<String, dynamic> existingCommit = {
+ 'author': 'test_user@example.com',
+ 'created': DateTime.parse('2019-11-22 22:19:00Z'),
+ 'index': 52,
+ 'title': 'A commit used for testing, with index 52'
+};
+
+const String previousCommitHash = 'a previous existing commit hash';
+Map<String, dynamic> previousCommit = {
+ 'author': 'previous_user@example.com',
+ 'created': DateTime.parse('2019-11-24 11:18:00Z'),
+ 'index': 49,
+ 'title': 'A commit used for testing, with index 49'
+};
diff --git a/functions/node/tryjob.dart b/functions/node/tryjob.dart
new file mode 100644
index 0000000..7158d7f
--- /dev/null
+++ b/functions/node/tryjob.dart
@@ -0,0 +1,32 @@
+// 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 'firestore.dart';
+import 'gerrit_change.dart';
+
+bool isChangedResult(Map<String, dynamic> result) =>
+ result['changed'] && !result['flaky'] && !result['previous_flaky'];
+
+class Tryjob {
+ static final changeRefRegExp = RegExp(r'refs/changes/(\d*)/(\d*)');
+ FirestoreService firestore;
+ int review;
+ int patchset;
+
+ Tryjob(String changeRef, this.firestore) {
+ final match = changeRefRegExp.matchAsPrefix(changeRef);
+ review = int.parse(match[1]);
+ patchset = int.parse(match[2]);
+ }
+
+ Future<void> update() {
+ return GerritInfo(review, patchset, firestore).update();
+ }
+
+ Future<void> process(List<Map<String, dynamic>> results) async {
+ await update();
+ await Future.forEach(results.where(isChangedResult),
+ (change) => firestore.storeTryChange(change, review, patchset));
+ }
+}
diff --git a/functions/pubspec.lock b/functions/pubspec.lock
index a99f498..5cb1a22 100644
--- a/functions/pubspec.lock
+++ b/functions/pubspec.lock
@@ -29,6 +29,13 @@
url: "https://pub.dartlang.org"
source: hosted
version: "0.1.22"
+ boolean_selector:
+ dependency: transitive
+ description:
+ name: boolean_selector
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "1.0.5"
build:
dependency: transitive
description:
@@ -134,6 +141,13 @@
url: "https://pub.dartlang.org"
source: hosted
version: "2.1.1"
+ coverage:
+ dependency: transitive
+ description:
+ name: coverage
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "0.13.3+1"
crypto:
dependency: transitive
description:
@@ -266,7 +280,7 @@
name: matcher
url: "https://pub.dartlang.org"
source: hosted
- version: "0.12.5"
+ version: "0.12.6"
meta:
dependency: transitive
description:
@@ -281,6 +295,20 @@
url: "https://pub.dartlang.org"
source: hosted
version: "0.9.6+3"
+ mockito:
+ dependency: "direct dev"
+ description:
+ name: mockito
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "4.1.1"
+ multi_server_socket:
+ dependency: transitive
+ description:
+ name: multi_server_socket
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "1.0.2"
node_http:
dependency: transitive
description:
@@ -393,6 +421,20 @@
url: "https://pub.dartlang.org"
source: hosted
version: "0.7.5"
+ shelf_packages_handler:
+ dependency: transitive
+ description:
+ name: shelf_packages_handler
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "1.0.4"
+ shelf_static:
+ dependency: transitive
+ description:
+ name: shelf_static
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "0.2.8"
shelf_web_socket:
dependency: transitive
description:
@@ -400,6 +442,20 @@
url: "https://pub.dartlang.org"
source: hosted
version: "0.2.3"
+ source_map_stack_trace:
+ dependency: transitive
+ description:
+ name: source_map_stack_trace
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "1.1.5"
+ source_maps:
+ dependency: transitive
+ description:
+ name: source_maps
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "0.10.8"
source_span:
dependency: transitive
description:
@@ -442,6 +498,27 @@
url: "https://pub.dartlang.org"
source: hosted
version: "1.1.0"
+ test:
+ dependency: "direct dev"
+ description:
+ name: test
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "1.9.4"
+ test_api:
+ dependency: transitive
+ description:
+ name: test_api
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "0.2.11"
+ test_core:
+ dependency: transitive
+ description:
+ name: test_core
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "0.2.15"
timing:
dependency: transitive
description:
@@ -456,6 +533,13 @@
url: "https://pub.dartlang.org"
source: hosted
version: "1.1.6"
+ vm_service:
+ dependency: transitive
+ description:
+ name: vm_service
+ url: "https://pub.dartlang.org"
+ source: hosted
+ version: "2.1.2"
watcher:
dependency: transitive
description:
diff --git a/functions/pubspec.yaml b/functions/pubspec.yaml
index 090ab02..defd5b8 100644
--- a/functions/pubspec.yaml
+++ b/functions/pubspec.yaml
@@ -13,4 +13,5 @@
dev_dependencies:
build_runner: ^1.7.1
build_node_compilers: ^0.2.3
-
+ mockito: ^4.1.0
+ test: ^1.9.4
