[cloud functions] Move commit fetching to a shared cache
Change-Id: Ie7562ec9686053759db3c1fa334afd9ba3976408
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/137800
Reviewed-by: Alexander Thomas <athom@google.com>
diff --git a/functions/node/builder.dart b/functions/node/builder.dart
index c0c6b46..c7bed59 100644
--- a/functions/node/builder.dart
+++ b/functions/node/builder.dart
@@ -2,51 +2,20 @@
// 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:http/http.dart' as http;
import 'package:pool/pool.dart';
+import 'commits_cache.dart';
import 'firestore.dart';
import 'result.dart';
import 'reverted_changes.dart';
-const prefix = ")]}'\n";
-
-Iterable<int> range(int start, int end) sync* {
- for (int i = start; i < end; ++i) yield i;
-}
-
-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 http.BaseClient httpClient;
+ final CommitsCache commitsCache;
final String commitHash;
final Map<String, dynamic> firstResult;
final int countChunks;
@@ -66,8 +35,8 @@
List<String> approvalMessages = [];
int countApprovalsCopied = 0;
- Build(this.commitHash, this.firstResult, this.countChunks, this.firestore,
- this.httpClient)
+ Build(this.commitHash, this.firstResult, this.countChunks, this.commitsCache,
+ this.firestore)
: builderName = firstResult['builder_name'],
buildNumber = int.parse(firstResult['build_number']);
@@ -106,13 +75,9 @@
/// 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.
- endCommit = await firestore.getCommit(commitHash);
+ endCommit = await commitsCache.getCommit(commitHash);
if (endCommit == null) {
- await getMissingCommits();
- endCommit = await firestore.getCommit(commitHash);
- if (endCommit == null) {
- throw 'Result received with unknown commit hash $commitHash';
- }
+ throw 'Result received with unknown commit hash $commitHash';
}
endIndex = endCommit[fIndex];
// If this is a new builder, use the current commit as a trivial blamelist.
@@ -120,7 +85,7 @@
startIndex = endIndex;
} else {
final startCommit =
- await firestore.getCommit(firstResult['previous_commit_hash']);
+ await commitsCache.getCommit(firstResult['previous_commit_hash']);
startIndex = startCommit[fIndex] + 1;
if (startIndex > endIndex) {
throw ArgumentError("Results received with empty blamelist\n"
@@ -135,7 +100,7 @@
Future<void> fetchReviewsAndReverts() => _awaitCommits ??= () async {
commits = [
for (var index = startIndex; index < endIndex; ++index)
- await firestore.getCommitByIndex(index),
+ await commitsCache.getCommitByIndex(index),
endCommit
];
for (final commit in commits) {
@@ -155,81 +120,6 @@
}
}();
- /// This function is idempotent, so every call of it should write the
- /// same info to new Firestore documents. It is save to call multiple
- /// times simultaneously.
- Future<void> getMissingCommits() async {
- final lastCommit = await firestore.getLastCommit();
- // Unit tests api uses 'hash', FirestoreService uses 'id'.
- // FirestoreService is changed to 'hash' in follow-up CL.
- final lastHash = lastCommit['id'] ?? lastCommit['hash'];
- final lastIndex = lastCommit[fIndex];
-
- 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 httpClient.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) {
- print('Found no new commits between $lastHash and master');
- return;
- }
- commitsFetched = commits.length;
- final first = commits.last as Map<String, dynamic>;
- if (first['parents'].first != lastHash) {
- throw 'First new commit $first is not'
- ' a child of last known commit $lastHash when fetching new commits';
- }
- if (!commits.any((commit) => commit['commit'] == commitHash)) {
- throw 'Did not find commit $commitHash when fetching new commits';
- }
- var index = lastIndex + 1;
- for (Map<String, dynamic> commit in commits.reversed) {
- final review = _review(commit);
- var reverted = _revert(commit);
- var relanded = _reland(commit);
- if (relanded != null) {
- reverted = null;
- }
- if (reverted != null) {
- final revertedCommit = await firestore.getCommit(reverted);
- if (revertedCommit != null && revertedCommit[fRevertOf] != null) {
- reverted = null;
- relanded = revertedCommit[fRevertOf];
- }
- }
- await firestore.addCommit(commit['commit'], {
- fAuthor: commit['author']['email'],
- fCreated: parseGitilesDateTime(commit['committer']['time']),
- fIndex: index,
- fTitle: commit['message'].split('\n').first,
- if (review != null) fReview: review,
- if (reverted != null) fRevertOf: reverted,
- if (relanded != null) fRelandOf: relanded,
- });
- if (review != null) {
- await landReview(commit, index);
- }
- ++index;
- }
- }
-
- /// This function is idempotent and may be called multiple times
- /// concurrently.
- Future<void> landReview(Map<String, dynamic> commit, int index) async {
- int review = _review(commit);
- // Optimization to avoid duplicate work: if another instance has linked
- // the review to its landed commit, do nothing.
- if (await firestore.reviewIsLanded(review)) return;
- await firestore.linkReviewToCommit(review, index);
- await firestore.linkCommentsToCommit(review, index);
- }
-
Future<void> storeConfigurationsInfo(Iterable<String> configurations) async {
for (final configuration in configurations) {
await firestore.updateConfiguration(configuration, builderName);
@@ -287,28 +177,6 @@
}
}
-final reviewRegExp = RegExp(
- '^Reviewed-on: https://dart-review.googlesource.com/c/sdk/\\+/(\\d+)\$',
- multiLine: true);
-
-int _review(Map<String, dynamic> commit) {
- final match = reviewRegExp.firstMatch(commit['message']);
- if (match != null) return int.parse(match.group(1));
- return null;
-}
-
-final revertRegExp =
- RegExp('^This reverts commit ([\\da-f]+)\\.\$', multiLine: true);
-
-String _revert(Map<String, dynamic> commit) =>
- revertRegExp.firstMatch(commit['message'])?.group(1);
-
-final relandRegExp =
- RegExp('^This is a reland of ([\\da-f]+)\\.?\$', multiLine: true);
-
-String _reland(Map<String, dynamic> commit) =>
- relandRegExp.firstMatch(commit['message'])?.group(1);
-
Map<String, dynamic> constructResult(
Map<String, dynamic> change, int startIndex, int endIndex,
{bool approved, int landedReviewIndex, bool failure}) =>
@@ -325,3 +193,10 @@
if (failure) fActive: true,
if (failure) fActiveConfigurations: <String>[change['configuration']]
};
+
+String testResult(Map<String, dynamic> change) => [
+ change['name'],
+ change['result'],
+ change['previous_result'] ?? 'new test',
+ change['expected']
+ ].join(' ');
diff --git a/functions/node/commits_cache.dart b/functions/node/commits_cache.dart
new file mode 100644
index 0000000..8477215
--- /dev/null
+++ b/functions/node/commits_cache.dart
@@ -0,0 +1,216 @@
+// Copyright (c) 2020, 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:async';
+import 'dart:convert';
+
+import 'package:http/http.dart' as http;
+import 'firestore.dart';
+import 'result.dart';
+
+/// Contains data about the commits on the master branch of the sdk repo.
+/// An instance of this class is stored in a top-level variable, and is
+/// shared between cloud function invocations.
+///
+/// The class fetches commits from Firestore if they are present,
+/// and fetches them from gitiles if not, and saves them to Firestore.
+class CommitsCache {
+ FirestoreService firestore;
+ final http.Client httpClient;
+ Map<String, Map<String, dynamic>> byHash = {};
+ Map<int, Map<String, dynamic>> byIndex = {};
+ int startIndex;
+ int endIndex;
+
+ CommitsCache(this.firestore, this.httpClient);
+
+ Future<Map<String, dynamic>> getCommit(String hash) async {
+ return byHash[hash] ??
+ await fetchByHash(hash) ??
+ await getNewCommits() ??
+ await fetchByHash(hash);
+ }
+
+ Future<Map<String, dynamic>> getCommitByIndex(int index) async {
+ return byIndex[index] ??
+ await fetchByIndex(index) ??
+ await getNewCommits() ??
+ await fetchByIndex(index);
+ }
+
+ /// Add a commit to the cache. The cache must be empty, or the commit
+ /// must be immediately before or after the current cached commits.
+ /// Otherwise, do nothing.
+ void cacheCommit(Map<String, dynamic> commit) {
+ final index = commit['index'];
+ if (startIndex == null || startIndex == index + 1) {
+ startIndex = index;
+ if (endIndex == null) {
+ endIndex = index;
+ }
+ } else if (endIndex + 1 == index) {
+ endIndex = index;
+ } else
+ return;
+ byHash[commit['hash']] = commit;
+ byIndex[index] = commit;
+ }
+
+ Future<Map<String, dynamic>> fetchByHash(String hash) async {
+ final commit = await firestore.getCommit(hash);
+ if (commit == null) return null;
+ final index = commit['index'];
+ if (startIndex == null) {
+ cacheCommit(commit);
+ } else if (index < startIndex) {
+ for (int fetchIndex = startIndex - 1; fetchIndex > index; --fetchIndex) {
+ // Other invocations may be fetching simultaneously.
+ if (fetchIndex < startIndex) {
+ final infillCommit = await firestore.getCommitByIndex(fetchIndex);
+ cacheCommit(infillCommit);
+ }
+ }
+ cacheCommit(commit);
+ } else if (index > endIndex) {
+ for (int fetchIndex = endIndex + 1; fetchIndex < index; ++fetchIndex) {
+ // Other invocations may be fetching simultaneously.
+ if (fetchIndex > endIndex) {
+ final infillCommit = await firestore.getCommitByIndex(fetchIndex);
+ cacheCommit(infillCommit);
+ }
+ }
+ cacheCommit(commit);
+ }
+ return commit;
+ }
+
+ Future<Map<String, dynamic>> fetchByIndex(int index) => firestore
+ .getCommitByIndex(index)
+ .then((commit) => fetchByHash(commit['hash']));
+
+ /// This function is idempotent, so every call of it should write the
+ /// same info to new Firestore documents. It is save to call multiple
+ /// times simultaneously.
+ Future<Null> getNewCommits() async {
+ const prefix = ")]}'\n";
+ final lastCommit = await firestore.getLastCommit();
+ final lastHash = lastCommit['hash'];
+ 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 response = await httpClient.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) {
+ print('Found no new commits between $lastHash and master');
+ return;
+ }
+ final first = commits.last as Map<String, dynamic>;
+ if (first['parents'].first != lastHash) {
+ throw 'First new commit ${first['parents'].first} is not'
+ ' a child of last known commit $lastHash when fetching new commits';
+ }
+ var index = lastIndex + 1;
+ for (Map<String, dynamic> commit in commits.reversed) {
+ final review = _review(commit);
+ var reverted = _revert(commit);
+ var relanded = _reland(commit);
+ if (relanded != null) {
+ reverted = null;
+ }
+ if (reverted != null) {
+ final revertedCommit = await firestore.getCommit(reverted);
+ if (revertedCommit != null && revertedCommit[fRevertOf] != null) {
+ reverted = null;
+ relanded = revertedCommit[fRevertOf];
+ }
+ }
+ await firestore.addCommit(commit['commit'], {
+ fAuthor: commit['author']['email'],
+ fCreated: parseGitilesDateTime(commit['committer']['time']),
+ fIndex: index,
+ fTitle: commit['message'].split('\n').first,
+ if (review != null) fReview: review,
+ if (reverted != null) fRevertOf: reverted,
+ if (relanded != null) fRelandOf: relanded,
+ });
+ if (review != null) {
+ await landReview(commit, index);
+ }
+ ++index;
+ }
+ }
+
+ /// This function is idempotent and may be called multiple times
+ /// concurrently.
+ Future<void> landReview(Map<String, dynamic> commit, int index) async {
+ int review = _review(commit);
+ // Optimization to avoid duplicate work: if another instance has linked
+ // the review to its landed commit, do nothing.
+ if (await firestore.reviewIsLanded(review)) return;
+ await firestore.linkReviewToCommit(review, index);
+ await firestore.linkCommentsToCommit(review, index);
+ }
+}
+
+class TestingCommitsCache extends CommitsCache {
+ TestingCommitsCache(firestore, httpClient) : super(firestore, httpClient);
+
+ Future<Null> getNewCommits() async {
+ if ((await firestore.isStaging())) {
+ return super.getNewCommits();
+ }
+ }
+}
+
+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]}');
+}
+
+final reviewRegExp = RegExp(
+ '^Reviewed-on: https://dart-review.googlesource.com/c/sdk/\\+/(\\d+)\$',
+ multiLine: true);
+
+int _review(Map<String, dynamic> commit) {
+ final match = reviewRegExp.firstMatch(commit['message']);
+ if (match != null) return int.parse(match.group(1));
+ return null;
+}
+
+final revertRegExp =
+ RegExp('^This reverts commit ([\\da-f]+)\\.\$', multiLine: true);
+
+String _revert(Map<String, dynamic> commit) =>
+ revertRegExp.firstMatch(commit['message'])?.group(1);
+
+final relandRegExp =
+ RegExp('^This is a reland of ([\\da-f]+)\\.?\$', multiLine: true);
+
+String _reland(Map<String, dynamic> commit) =>
+ relandRegExp.firstMatch(commit['message'])?.group(1);
diff --git a/functions/node/firestore.dart b/functions/node/firestore.dart
index adf47d7..27bce78 100644
--- a/functions/node/firestore.dart
+++ b/functions/node/firestore.dart
@@ -7,6 +7,8 @@
/// 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> isStaging();
+
Future<bool> hasPatchset(String review, String patchset);
Future<Map<String, dynamic>> getCommit(String hash);
diff --git a/functions/node/firestore_impl.dart b/functions/node/firestore_impl.dart
index 64336f8..541b3ff 100644
--- a/functions/node/firestore_impl.dart
+++ b/functions/node/firestore_impl.dart
@@ -20,21 +20,31 @@
}
class FirestoreServiceImpl implements FirestoreService {
+ Future<bool> isStaging() =>
+ firestore.collection('staging').get().then((s) => s.isNotEmpty);
+
Future<bool> hasPatchset(String review, String patchset) => firestore
.document('reviews/$review/patchsets/$patchset')
.get()
.then((s) => s.exists);
+ Map<String, dynamic> _commit(DocumentSnapshot document) {
+ if (document.exists) {
+ return document.data.toMap()..['hash'] = document.documentID;
+ }
+ return null;
+ }
+
Future<Map<String, dynamic>> getCommit(String hash) => firestore
.document('commits/$hash')
.get()
- .then((s) => s.exists ? s.data.toMap() : null);
+ .then((document) => _commit(document));
Future<Map<String, dynamic>> getCommitByIndex(int index) => firestore
.collection('commits')
.where('index', isEqualTo: index)
.get()
- .then((s) => s.documents.first.data.toMap());
+ .then((s) => _commit(s.documents.first));
Future<Map<String, dynamic>> getLastCommit() async {
QuerySnapshot lastCommit = await firestore
@@ -42,9 +52,7 @@
.orderBy('index', descending: true)
.limit(1)
.get();
- final result = lastCommit.documents.first.data.toMap();
- result['id'] = lastCommit.documents.first.documentID;
- return result;
+ return _commit(lastCommit.documents.first);
}
Future<void> addCommit(String id, Map<String, dynamic> data) async {
diff --git a/functions/node/index.dart b/functions/node/index.dart
index 0dc7766..b680190 100644
--- a/functions/node/index.dart
+++ b/functions/node/index.dart
@@ -7,9 +7,12 @@
import 'package:node_interop/node.dart';
import 'builder.dart';
+import 'commits_cache.dart';
import 'firestore_impl.dart';
import 'tryjob.dart';
+final commits = CommitsCache(FirestoreServiceImpl(), http.NodeClient());
+
void main() {
functions['receiveChanges'] =
functions.pubsub.topic('results').onPublish(receiveChanges);
@@ -23,15 +26,15 @@
? int.parse(message.attributes['num_chunks'])
: null;
final String buildbucketID = message.attributes['buildbucket_id'];
+ final String baseRevision = message.attributes['base_revision'];
try {
var firestore = FirestoreServiceImpl();
if (commit.startsWith('refs/changes')) {
- return await Tryjob(
- commit, countChunks, buildbucketID, firestore, http.NodeClient())
+ return await Tryjob(commit, countChunks, buildbucketID, baseRevision,
+ commits, firestore, http.NodeClient())
.process(results);
} else {
- return await Build(
- commit, first, countChunks, firestore, http.NodeClient())
+ return await Build(commit, first, countChunks, commits, firestore)
.process(results);
}
} catch (e, trace) {
diff --git a/functions/node/test/commits_cache_test_nodejs.dart b/functions/node/test/commits_cache_test_nodejs.dart
new file mode 100644
index 0000000..5e09679
--- /dev/null
+++ b/functions/node/test/commits_cache_test_nodejs.dart
@@ -0,0 +1,102 @@
+// Copyright (c) 2020, 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:firebase_admin_interop/firebase_admin_interop.dart';
+import 'package:firebase_functions_interop/firebase_functions_interop.dart';
+import 'package:node_http/node_http.dart' as http;
+import 'package:test/test.dart';
+
+import '../firestore_impl.dart' as fs;
+import '../commits_cache.dart';
+
+// These tests read and write data from the Firestore database.
+// If they are run against the production database, they will not
+// write data to the database.
+// Requires the environment variable GOOGLE_APPLICATION_CREDENTIALS
+// to point to a json key to a service account.
+// To run against the staging database, use a service account.
+// with write access to dart_ci_staging datastore.
+// Set the database with 'firebase use --add dart-ci-staging'
+// The test must be compiled with nodejs, and run using the 'node' command.
+
+void main() async {
+ final firestore = fs.FirestoreServiceImpl();
+ // create commits cache
+ final commits = TestingCommitsCache(firestore, http.NodeClient());
+ test('Test fetch first commit', () async {
+ Future<void> fetchAndTestCommit(Map<String, dynamic> commit) async {
+ final fetched = await commits.getCommit(commit['hash']);
+ final copied = Map.from(fetched)..remove('author');
+ expect(copied, commit);
+ }
+ Future<void> fetchAndTestCommitByIndex(Map<String, dynamic> commit) async {
+ final fetched = await commits.getCommitByIndex(commit['index']);
+ final copied = Map.from(fetched)..remove('author');
+ expect(copied, commit);
+ }
+ expect(commits.startIndex, isNull);
+ await fetchAndTestCommit(commit68900);
+ expect(commits.startIndex, 68900);
+ expect(commits.endIndex, 68900);
+ await fetchAndTestCommit(commit68900);
+ expect(commits.startIndex, 68900);
+ expect(commits.endIndex, 68900);
+ await fetchAndTestCommitByIndex(commit68910);
+ expect(commits.startIndex, 68900);
+ expect(commits.endIndex, 68910);
+ await fetchAndTestCommitByIndex(commit68905);
+ expect(commits.startIndex, 68900);
+ expect(commits.endIndex, 68910);
+ await fetchAndTestCommit(commit68905);
+ expect(commits.startIndex, 68900);
+ expect(commits.endIndex, 68910);
+ await fetchAndTestCommitByIndex(commit68890);
+ expect(commits.startIndex, 68890);
+ expect(commits.endIndex, 68910);
+ await fetchAndTestCommitByIndex(commit68889);
+ expect(commits.startIndex, 68889);
+ expect(commits.endIndex, 68910);
+ });
+}
+
+final commit68889 = <String, dynamic>{
+ 'review': 136974,
+ 'title': '[Cleanup] Removes deprecated --gc_at_instance_allocation.',
+ 'index': 68889,
+ 'created': Timestamp.fromDateTime(DateTime.parse("2020-02-26 16:00:26.000")),
+ 'hash': '9c05fde96b62556944befd18ec834c56d6854fda'
+};
+
+final commit68890 = <String, dynamic>{
+ 'review': 136854,
+ 'title':
+ 'Add analyzer run support to steamroller and minor QOL improvements.',
+ 'index': 68890,
+ 'created': Timestamp.fromDateTime(DateTime.parse("2020-02-26 17:57:46.000")),
+ 'hash': '31053a8c0180b663858aadce1ff6c0eefcf78623'
+};
+
+final commit68900 = <String, dynamic>{
+ 'review': 137322,
+ 'title': 'Remove unused SdkPatcher.',
+ 'index': 68900,
+ 'created': Timestamp.fromDateTime(DateTime.parse("2020-02-26 21:20:31.000")),
+ 'hash': '118d220bfa7dc0f065b441e4edd584c2b9c0edc8',
+};
+
+final commit68905 = <String, dynamic>{
+ 'review': 137286,
+ 'title': '[dart2js] switch bot to use hostaserts once again',
+ 'index': 68905,
+ 'created': Timestamp.fromDateTime(DateTime.parse("2020-02-26 22:41:47.000")),
+ 'hash': '5055c98beeacb3996c256e37148b4dc3561735ee'
+};
+
+final commit68910 = <String, dynamic>{
+ 'review': 137424,
+ 'title': 'corpus index updates',
+ 'index': 68910,
+ 'created': Timestamp.fromDateTime(DateTime.parse("2020-02-27 00:19:11.000")),
+ 'hash': '8fb0e62babb213c98f4051f544fc80527bcecc18',
+};
diff --git a/functions/node/test/fakes.dart b/functions/node/test/fakes.dart
index bf125a1..c283739 100644
--- a/functions/node/test/fakes.dart
+++ b/functions/node/test/fakes.dart
@@ -10,6 +10,7 @@
import 'package:http/http.dart';
import '../builder.dart';
+import '../commits_cache.dart';
import '../firestore.dart';
import '../result.dart';
import 'test_data.dart';
@@ -17,12 +18,14 @@
class BuilderTest {
final client = HttpClientMock();
final firestore = FirestoreServiceFake();
+ CommitsCache commitsCache;
Build builder;
String commitHash;
Map<String, dynamic> firstChange;
BuilderTest(this.commitHash, this.firstChange) {
- builder = Build(commitHash, firstChange, null, firestore, client);
+ commitsCache = CommitsCache(firestore, client);
+ builder = Build(commitHash, firstChange, null, commitsCache, firestore);
}
Future<void> update() async {
@@ -94,7 +97,7 @@
results[id][fActiveConfigurations] != null &&
results[id][fActiveConfigurations]
.contains(change['configuration']))
- results[id]..['id'] = id
+ Map.from(results[id])..['id'] = id
];
}
diff --git a/functions/node/test/firestore_test_nodejs.dart b/functions/node/test/firestore_test_nodejs.dart
index 52b1415..cd37ea3 100644
--- a/functions/node/test/firestore_test_nodejs.dart
+++ b/functions/node/test/firestore_test_nodejs.dart
@@ -21,7 +21,7 @@
void main() async {
final firestore = fs.FirestoreServiceImpl();
- if ((await fs.firestore.collection('staging').get()).isEmpty) {
+ if (!await firestore.isStaging()) {
console
.error('Error: firestore_test_nodejs.dart is being run on production');
throw (TestFailure(
@@ -163,7 +163,8 @@
..addAll(
{'commit_hash': testPreviousPatchsetPath, 'build_number': '307'});
final buildID0 = 'test buildbucket id 0';
- await Tryjob(testPreviousPatchsetPath, 1, buildID0, firestore, null)
+ await Tryjob(testPreviousPatchsetPath, 1, buildID0, null, null, firestore,
+ null)
.process([previousFailingChange]);
var snapshot = await fs.firestore
.collection('try_results')
@@ -180,10 +181,10 @@
{'number': testPreviousPatchset});
// Send first chunk with a previously approved result and a passing result
final buildID1 = 'test buildbucket id 1';
- await Tryjob(testReviewPath, null, buildID1, firestore, null)
+ await Tryjob(testReviewPath, null, buildID1, null, null, firestore, null)
.process([tryjobPassingChange, tryjobFailingChange]);
// Send second & final chunk with an unchanged failure.
- await Tryjob(testReviewPath, 2, buildID1, firestore, null)
+ await Tryjob(testReviewPath, 2, buildID1, null, null, firestore, null)
.process([tryjobExistingFailure, tryjobFailingChange]);
// Verify state
snapshot = await fs.firestore
@@ -215,7 +216,7 @@
// Send first chunk of second run on the same patchset, with an approved
// failure and an unapproved failure.
final buildID2 = 'test buildbucket id 2';
- await Tryjob(testReviewPath, null, buildID2, firestore, null)
+ await Tryjob(testReviewPath, null, buildID2, null, null, firestore, null)
.process([tryjob2OtherFailingChange, tryjob2FailingChange]);
final reference = fs.firestore
.document('try_builds/$testBuilder:$testReview:$testPatchset');
@@ -227,7 +228,7 @@
expect(document.data.getInt('num_chunks'), isNull);
expect(document.data.getInt('processed_chunks'), 1);
// Send second chunk.
- await Tryjob(testReviewPath, 3, buildID2, firestore, null)
+ await Tryjob(testReviewPath, 3, buildID2, null, null, firestore, null)
.process([tryjob2ExistingFailure]);
document = await reference.get();
expect(document.data.getBool('success'), isFalse);
@@ -236,7 +237,7 @@
expect(document.data.getInt('num_chunks'), 3);
expect(document.data.getInt('processed_chunks'), 2);
// Send third and final chunk.
- await Tryjob(testReviewPath, null, buildID2, firestore, null)
+ await Tryjob(testReviewPath, null, buildID2, null, null, firestore, null)
.process([tryjob2PassingChange]);
document = await reference.get();
expect(document.data.getBool('success'), isFalse);
@@ -247,7 +248,7 @@
// Send first chunk of a third run, with only one chunk.
final buildID3 = 'test buildbucket id 3';
- await Tryjob(testReviewPath, 1, buildID3, firestore, null)
+ await Tryjob(testReviewPath, 1, buildID3, null, null, firestore, null)
.process([tryjob3PassingChange]);
document = await reference.get();
expect(document.data.getBool('success'), isTrue);
diff --git a/functions/node/test/test.dart b/functions/node/test/test.dart
index d8dbd56..5924b5a 100644
--- a/functions/node/test/test.dart
+++ b/functions/node/test/test.dart
@@ -58,5 +58,13 @@
Map.from(activeResult)..remove(fActiveConfigurations)..remove(fActive));
expect(builderTest.builder.countApprovalsCopied, 1);
expect(builderTest.builder.countChanges, 2);
+ expect(
+ builderTest.firestore.results[await builderTest.firestore.findResult(
+ landedCommitChange, landedCommitIndex, landedCommitIndex)],
+ landedResult);
+ expect(
+ (await builderTest.firestore.findActiveResults(landedCommitChange))
+ ..single.remove('id'),
+ [landedResult]);
});
}
diff --git a/functions/node/test/test_data.dart b/functions/node/test/test_data.dart
index 30aeaff..a941d70 100644
--- a/functions/node/test/test_data.dart
+++ b/functions/node/test/test_data.dart
@@ -71,11 +71,15 @@
/// Changes
/// These are single lines from a result.json, representing the output
/// of a single test on a single configuration on a single build.
+const String sampleTestName = "local_function_signatures_strong_test/none";
+const String sampleTestSuite = "dart2js_extra";
+const String sampleTest = "$sampleTestSuite/$sampleTestName";
+
const Map<String, dynamic> existingCommitChange = {
- "name": "dart2js_extra/local_function_signatures_strong_test/none",
+ "name": sampleTest,
"configuration": "dart2js-new-rti-linux-x64-d8",
- "suite": "dart2js_extra",
- "test_name": "local_function_signatures_strong_test/none",
+ "suite": sampleTestSuite,
+ "test_name": sampleTestName,
"time_ms": 2384,
"result": "Pass",
"expected": "Pass",
@@ -95,10 +99,10 @@
};
const Map<String, dynamic> landedCommitChange = {
- "name": "dart2js_extra/local_function_signatures_strong_test/none",
+ "name": sampleTest,
"configuration": "dart2js-new-rti-linux-x64-d8",
- "suite": "dart2js_extra",
- "test_name": "local_function_signatures_strong_test/none",
+ "suite": sampleTestSuite,
+ "test_name": sampleTestName,
"time_ms": 2384,
"result": "RuntimeError",
"expected": "Pass",
@@ -135,17 +139,17 @@
// A result on existingCommit that is overridden by the new result in
// landedCommitChange.
Map<String, dynamic> activeResult = {
- "name": landedCommitChange["name"],
+ "name": sampleTest,
"blamelist_start_index": existingCommitIndex,
"blamelist_end_index": existingCommitIndex,
"configurations": [
landedCommitChange["configuration"],
'another configuration'
- ],
+ ]..sort(),
"active_configurations": [
landedCommitChange["configuration"],
'another configuration'
- ],
+ ]..sort(),
"active": true,
"approved": false,
"result": "RuntimeError",
@@ -153,6 +157,26 @@
"previous_result": "Pass",
};
+Map<String, dynamic> landedResult = {
+ "name": sampleTest,
+ "blamelist_start_index": existingCommitIndex + 1,
+ "blamelist_end_index": landedCommitIndex,
+ "pinned_index": landedCommitIndex,
+ "configurations": [
+ 'another configuration',
+ landedCommitChange["configuration"],
+ ]..sort(),
+ "active_configurations": [
+ landedCommitChange["configuration"],
+ 'another configuration'
+ ]..sort(),
+ "active": true,
+ "approved": true,
+ "result": "RuntimeError",
+ "expected": "Pass",
+ "previous_result": "Pass",
+};
+
/// Try results
/// These are documents from the try_results table in Firestore.
const Map<String, dynamic> review44445Result = {
@@ -162,7 +186,7 @@
"dartk-reload-rollback-linux-debug-x64",
"dartk-reload-linux-debug-x64"
],
- "name": "dart2js_extra/local_function_signatures_strong_test/none",
+ "name": sampleTest,
"patchset": 1,
"result": "RuntimeError",
"expected": "Pass",
diff --git a/functions/node/tryjob.dart b/functions/node/tryjob.dart
index 3dfd943..d40fc80 100644
--- a/functions/node/tryjob.dart
+++ b/functions/node/tryjob.dart
@@ -5,6 +5,7 @@
import 'package:http/http.dart' as http show BaseClient;
import 'package:pool/pool.dart';
+import 'commits_cache.dart';
import 'firestore.dart';
import 'gerrit_change.dart';
@@ -15,7 +16,9 @@
static final changeRefRegExp = RegExp(r'refs/changes/(\d*)/(\d*)');
final http.BaseClient httpClient;
final FirestoreService firestore;
+ final CommitsCache commits;
String builderName;
+ String baseRevision;
int buildNumber;
int review;
int patchset;
@@ -25,8 +28,8 @@
int countChanges = 0;
int countUnapproved = 0;
- Tryjob(String changeRef, this.countChunks, this.buildbucketID, this.firestore,
- this.httpClient) {
+ Tryjob(String changeRef, this.countChunks, this.buildbucketID,
+ this.baseRevision, this.commits, this.firestore, this.httpClient) {
final match = changeRefRegExp.matchAsPrefix(changeRef);
review = int.parse(match[1]);
patchset = int.parse(match[2]);
