Use datastore to determine if a checkrun rerun should trigger a presubmit or postsubmit build (#2387)
* Use GoB to determine if a checkrun should trigger a presubmit or postsubmit rerun
* Apply suggestions from code review
Co-authored-by: Casey Hillers <caseyhillers@gmail.com>
* Simplify getCommit's arguments by passing project name through slug
* Remove GoB stuff
* Use datastore instead of GoB for postsubmit check
* Fix tests and change ScheduleBuildRequest's properties map to have Objects as values instead of dynamic
* Add datastore tests
* Add logging about presubmit / postsubmit determination
* Add tests for processCheckRun
* Fix GerritCommit deserialization of committer field
* Fix gitBranch TODO in processCheckRun
* Add entity lookup methods from PR #2391
* Use entity methods
* Fix licenses test
* Rename Commit's key method, fix tests, and rewrite TODO
* Fix log message for Commit lookup
Co-authored-by: Casey Hillers <caseyhillers@gmail.com>
diff --git a/app_dart/lib/src/model/appengine/commit.dart b/app_dart/lib/src/model/appengine/commit.dart
index 4fa9eb4..592934e 100644
--- a/app_dart/lib/src/model/appengine/commit.dart
+++ b/app_dart/lib/src/model/appengine/commit.dart
@@ -6,6 +6,8 @@
import 'package:github/github.dart';
import 'package:json_annotation/json_annotation.dart';
+import '../../service/datastore.dart';
+import '../../service/logging.dart';
import 'key_converter.dart';
part 'commit.g.dart';
@@ -28,6 +30,28 @@
id = key?.id;
}
+ /// Create a [Key] that can be used to lookup a [Commit] from Datastore.
+ static Key<String> createKey({
+ required DatastoreDB db,
+ required RepositorySlug slug,
+ required String gitBranch,
+ required String sha,
+ }) {
+ return db.emptyKey.append(
+ Commit,
+ id: '${slug.fullName}/$gitBranch/$sha',
+ );
+ }
+
+ /// Lookup [Commit] from Datastore.
+ static Future<Commit> fromDatastore({
+ required DatastoreService datastore,
+ required Key<String> key,
+ }) async {
+ log.fine('Looking up commit by key with id: ${key.id}');
+ return datastore.lookupByValue<Commit>(key);
+ }
+
/// The timestamp (in milliseconds since the Epoch) of when the commit
/// landed.
@IntProperty(propertyName: 'CreateTimestamp', required: true)
diff --git a/app_dart/lib/src/model/appengine/task.dart b/app_dart/lib/src/model/appengine/task.dart
index cc4b48c..002f32f 100644
--- a/app_dart/lib/src/model/appengine/task.dart
+++ b/app_dart/lib/src/model/appengine/task.dart
@@ -6,6 +6,7 @@
import 'package:json_annotation/json_annotation.dart';
import '../../request_handling/exceptions.dart';
+import '../../service/datastore.dart';
import '../../service/logging.dart';
import '../ci_yaml/target.dart';
import '../luci/push_message.dart';
@@ -70,6 +71,64 @@
);
}
+ /// Lookup [Task] from Datastore from its parent key and name.
+ static Future<Task> fromCommitKey({
+ required DatastoreService datastore,
+ required Key<String> commitKey,
+ required String name,
+ }) async {
+ if (name.isEmpty) {
+ throw const BadRequestException('task name is null');
+ }
+ final Query<Task> query = datastore.db.query<Task>(ancestorKey: commitKey)..filter('name =', name);
+ final List<Task> tasks = await query.run().toList();
+ if (tasks.length != 1) {
+ log.severe('Found ${tasks.length} entries for builder $name');
+ throw InternalServerError('Expected to find 1 task for $name, but found ${tasks.length}');
+ }
+ return tasks.single;
+ }
+
+ /// Lookup [Task] from its [key].
+ ///
+ /// This is the fastest way to lookup [Task], but requires [id] to be passed
+ /// as it is generated from Datastore.
+ static Future<Task> fromKey({
+ required DatastoreService datastore,
+ required Key<String> commitKey,
+ required int id,
+ }) {
+ log.fine('Looking up key...');
+ final Key<int> key = Key<int>(commitKey, Task, id);
+ return datastore.lookupByValue<Task>(key);
+ }
+
+ /// Lookup [Task] from Datastore.
+ ///
+ /// Either name or id must be given to lookup [Task].
+ ///
+ /// Prefer passing [id] when possible as it is a faster lookup.
+ static Future<Task> fromDatastore({
+ required DatastoreService datastore,
+ required Key<String> commitKey,
+ String? name,
+ String? id,
+ }) {
+ if (id == null) {
+ return Task.fromCommitKey(
+ datastore: datastore,
+ commitKey: commitKey,
+ name: name!,
+ );
+ }
+
+ return Task.fromKey(
+ datastore: datastore,
+ commitKey: commitKey,
+ id: int.parse(id),
+ );
+ }
+
/// The task was cancelled.
static const String statusCancelled = 'Cancelled';
diff --git a/app_dart/lib/src/model/gerrit/commit.dart b/app_dart/lib/src/model/gerrit/commit.dart
index 8044ec8..e5b7cc7 100644
--- a/app_dart/lib/src/model/gerrit/commit.dart
+++ b/app_dart/lib/src/model/gerrit/commit.dart
@@ -20,7 +20,7 @@
this.commit,
this.tree,
this.author,
- this.comitter,
+ this.committer,
this.message,
});
@@ -29,7 +29,7 @@
final String? commit;
final String? tree;
final GerritUser? author;
- final GerritUser? comitter;
+ final GerritUser? committer;
final String? message;
@override
diff --git a/app_dart/lib/src/model/gerrit/commit.g.dart b/app_dart/lib/src/model/gerrit/commit.g.dart
index a25363c..aaec6bd 100644
--- a/app_dart/lib/src/model/gerrit/commit.g.dart
+++ b/app_dart/lib/src/model/gerrit/commit.g.dart
@@ -12,7 +12,7 @@
commit: json['commit'] as String?,
tree: json['tree'] as String?,
author: json['author'] == null ? null : GerritUser.fromJson(json['author'] as Map<String, dynamic>),
- comitter: json['comitter'] == null ? null : GerritUser.fromJson(json['comitter'] as Map<String, dynamic>),
+ committer: json['committer'] == null ? null : GerritUser.fromJson(json['committer'] as Map<String, dynamic>),
message: json['message'] as String?,
);
@@ -20,7 +20,7 @@
'commit': instance.commit,
'tree': instance.tree,
'author': instance.author,
- 'comitter': instance.comitter,
+ 'committer': instance.committer,
'message': instance.message,
};
diff --git a/app_dart/lib/src/model/luci/buildbucket.dart b/app_dart/lib/src/model/luci/buildbucket.dart
index 88bebad..903639e 100644
--- a/app_dart/lib/src/model/luci/buildbucket.dart
+++ b/app_dart/lib/src/model/luci/buildbucket.dart
@@ -591,7 +591,7 @@
/// * ["blamelist""]
/// * ["$recipe_engine/runtime", "is_luci"]
/// * ["$recipe_engine/runtime", "is_experimental"]
- final Map<String, dynamic>? properties;
+ final Map<String, Object>? properties;
/// The value for Build.input.gitiles_commit.
///
@@ -809,7 +809,7 @@
static Input fromJson(Map<String, dynamic> json) => _$InputFromJson(json);
/// The build properties of a build.
- final Map<String, dynamic>? properties;
+ final Map<String, Object>? properties;
/// The [GitilesCommit] information for a build.
final GitilesCommit? gitilesCommit;
diff --git a/app_dart/lib/src/model/luci/buildbucket.g.dart b/app_dart/lib/src/model/luci/buildbucket.g.dart
index 06683d7..6cb274c 100644
--- a/app_dart/lib/src/model/luci/buildbucket.g.dart
+++ b/app_dart/lib/src/model/luci/buildbucket.g.dart
@@ -327,7 +327,9 @@
experimental: $enumDecodeNullable(_$TrinaryEnumMap, json['experimental']),
gitilesCommit:
json['gitilesCommit'] == null ? null : GitilesCommit.fromJson(json['gitilesCommit'] as Map<String, dynamic>),
- properties: json['properties'] as Map<String, dynamic>?,
+ properties: (json['properties'] as Map<String, dynamic>?)?.map(
+ (k, e) => MapEntry(k, e as Object),
+ ),
dimensions: (json['dimensions'] as List<dynamic>?)
?.map((e) => RequestedDimension.fromJson(e as Map<String, dynamic>))
.toList(),
@@ -461,7 +463,9 @@
value == null ? null : toJson(value);
Input _$InputFromJson(Map<String, dynamic> json) => Input(
- properties: json['properties'] as Map<String, dynamic>?,
+ properties: (json['properties'] as Map<String, dynamic>?)?.map(
+ (k, e) => MapEntry(k, e as Object),
+ ),
gitilesCommit:
json['gitilesCommit'] == null ? null : GitilesCommit.fromJson(json['gitilesCommit'] as Map<String, dynamic>),
experimental: json['experimental'] as bool?,
diff --git a/app_dart/lib/src/request_handlers/reset_prod_task.dart b/app_dart/lib/src/request_handlers/reset_prod_task.dart
index 6a7b9cd..85c3e40 100644
--- a/app_dart/lib/src/request_handlers/reset_prod_task.dart
+++ b/app_dart/lib/src/request_handlers/reset_prod_task.dart
@@ -19,7 +19,6 @@
import '../request_handling/exceptions.dart';
import '../service/config.dart';
import '../service/datastore.dart';
-import '../service/logging.dart';
import '../service/luci_build_service.dart';
import '../service/scheduler.dart';
@@ -119,50 +118,17 @@
final Key<int> key = config.keyHelper.decode(encodedKey) as Key<int>;
return datastore.lookupByValue<Task>(key);
}
-
- final Key<String> commitKey = await _constructCommitKey(
- datastore: datastore,
+ final Key<String> commitKey = Commit.createKey(
+ db: datastore.db,
+ slug: slug!,
gitBranch: gitBranch!,
sha: sha!,
- slug: slug!,
);
-
- final Query<Task> query = datastore.db.query<Task>(ancestorKey: commitKey);
- final List<Task> initialTasks = await query.run().toList();
- log.fine('Found ${initialTasks.length} tasks for commit');
- final List<Task> tasks = <Task>[];
- log.fine('Searching for task with name=$name');
- for (Task task in initialTasks) {
- if (task.name == name) {
- tasks.add(task);
- }
- }
-
- if (tasks.length != 1) {
- log.severe('Found ${tasks.length} entries for builder $name');
- throw InternalServerError('Expected to find 1 task for $name, but found ${tasks.length}');
- }
-
- return tasks.single;
- }
-
- /// Construct the Datastore key for [Commit] that is the ancestor to this [Task].
- ///
- /// Throws [BadRequestException] if the given git branch does not exist in [CocoonConfig].
- Future<Key<String>> _constructCommitKey({
- required DatastoreService datastore,
- required String gitBranch,
- required String sha,
- required RepositorySlug slug,
- }) async {
- gitBranch = gitBranch.trim();
- sha = sha.trim();
- final String id = '${slug.fullName}/$gitBranch/$sha';
- final Key<String> commitKey = datastore.db.emptyKey.append<String>(Commit, id: id);
- log.fine('Constructed commit key=$id');
- // Return the official key from Datastore for task lookups.
- final Commit commit = await datastore.lookupByValue<Commit>(commitKey);
- return commit.key;
+ return Task.fromDatastore(
+ datastore: datastore,
+ commitKey: commitKey,
+ name: name!,
+ );
}
/// Returns the [Commit] associated with [Task].
diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart
index a67e7b5..b1526a8 100644
--- a/app_dart/lib/src/service/luci_build_service.dart
+++ b/app_dart/lib/src/service/luci_build_service.dart
@@ -362,10 +362,10 @@
);
}
- /// Sends [ScheduleBuildRequest] for [pullRequest] using [checkRunEvent].
+ /// Sends presubmit [ScheduleBuildRequest] for a pull request using [checkRunEvent].
///
/// Returns the [Build] returned by scheduleBuildRequest.
- Future<Build> rescheduleUsingCheckRunEvent(cocoon_checks.CheckRunEvent checkRunEvent) async {
+ Future<Build> reschedulePresubmitBuildUsingCheckRunEvent(cocoon_checks.CheckRunEvent checkRunEvent) async {
final github.RepositorySlug slug = checkRunEvent.repository!.slug();
final String sha = checkRunEvent.checkRun!.headSha!;
@@ -379,36 +379,62 @@
);
final Iterable<Build> builds = await getTryBuilds(slug, sha, checkName);
-
- if (builds.isNotEmpty) {
- final Build build = builds.first;
-
- final String prString = build.tags!['buildset']!.firstWhere((String? element) => element!.startsWith('pr/git/'))!;
- final String cipdVersion = build.tags!['cipd_version']![0]!;
- final int prNumber = int.parse(prString.split('/')[2]);
-
- final Map<String, dynamic> userData = <String, dynamic>{'check_run_id': githubCheckRun.id};
- final Map<String, dynamic>? properties = build.input!.properties;
- log.info('input ${build.input!} properties $properties');
-
- final ScheduleBuildRequest scheduleBuildRequest = _createPresubmitScheduleBuild(
- slug: slug,
- sha: sha,
- checkName: checkName,
- pullRequestNumber: prNumber,
- cipdVersion: cipdVersion,
- properties: properties,
- userData: userData,
- );
-
- final Build scheduleBuild = await buildBucketClient.scheduleBuild(scheduleBuildRequest);
-
- final String buildUrl = 'https://ci.chromium.org/ui/b/${scheduleBuild.id}';
- await githubChecksUtil.updateCheckRun(config, slug, githubCheckRun, detailsUrl: buildUrl);
- return scheduleBuild;
- } else {
+ if (builds.isEmpty) {
throw NoBuildFoundException('Unable to find try build.');
}
+
+ final Build build = builds.first;
+ final String prString = build.tags!['buildset']!.firstWhere((String? element) => element!.startsWith('pr/git/'))!;
+ final String cipdVersion = build.tags!['cipd_version']![0]!;
+ final int prNumber = int.parse(prString.split('/')[2]);
+
+ final Map<String, dynamic> userData = <String, dynamic>{'check_run_id': githubCheckRun.id};
+ final Map<String, Object>? properties = build.input!.properties;
+ log.info('input ${build.input!} properties $properties');
+
+ final ScheduleBuildRequest scheduleBuildRequest = _createPresubmitScheduleBuild(
+ slug: slug,
+ sha: sha,
+ checkName: checkName,
+ pullRequestNumber: prNumber,
+ cipdVersion: cipdVersion,
+ properties: properties,
+ userData: userData,
+ );
+
+ final Build scheduleBuild = await buildBucketClient.scheduleBuild(scheduleBuildRequest);
+
+ final String buildUrl = 'https://ci.chromium.org/ui/b/${scheduleBuild.id}';
+ await githubChecksUtil.updateCheckRun(config, slug, githubCheckRun, detailsUrl: buildUrl);
+ return scheduleBuild;
+ }
+
+ /// Sends postsubmit [ScheduleBuildRequest] for a commit using [checkRunEvent], [Commit], [Task], and [Target].
+ ///
+ /// Returns the [Build] returned by scheduleBuildRequest.
+ Future<Build> reschedulePostsubmitBuildUsingCheckRunEvent(
+ cocoon_checks.CheckRunEvent checkRunEvent, {
+ required Commit commit,
+ required Task task,
+ required Target target,
+ }) async {
+ final github.RepositorySlug slug = checkRunEvent.repository!.slug();
+ final String sha = checkRunEvent.checkRun!.headSha!;
+ final String checkName = checkRunEvent.checkRun!.name!;
+
+ final Iterable<Build> builds = await getProdBuilds(slug, sha, checkName);
+ if (builds.isEmpty) {
+ throw NoBuildFoundException('Unable to find prod build.');
+ }
+
+ final Build build = builds.first;
+ final Map<String, Object>? properties = build.input!.properties;
+ log.info('input ${build.input!} properties $properties');
+
+ final ScheduleBuildRequest scheduleBuildRequest =
+ await _createPostsubmitScheduleBuild(commit: commit, target: target, task: task, properties: properties);
+ final Build scheduleBuild = await buildBucketClient.scheduleBuild(scheduleBuildRequest);
+ return scheduleBuild;
}
/// Gets [Build] using its [id] and passing the additional
@@ -474,15 +500,15 @@
required String checkName,
required int pullRequestNumber,
required String cipdVersion,
- Map<String, dynamic>? properties,
+ Map<String, Object>? properties,
Map<String, List<String>>? tags,
Map<String, dynamic>? userData,
List<RequestedDimension>? dimensions,
}) {
- final Map<String, dynamic> processedProperties = <String, dynamic>{};
- processedProperties.addAll(properties ?? <String, dynamic>{});
+ final Map<String, Object> processedProperties = <String, Object>{};
+ processedProperties.addAll(properties ?? <String, Object>{});
processedProperties.addEntries(
- <String, dynamic>{
+ <String, Object>{
'git_url': 'https://github.com/${slug.owner}/${slug.name}',
'git_ref': 'refs/pull/$pullRequestNumber/head',
'exe_cipd_version': cipdVersion,
diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart
index 64365de..e52bbc1 100644
--- a/app_dart/lib/src/service/scheduler.dart
+++ b/app_dart/lib/src/service/scheduler.dart
@@ -455,7 +455,7 @@
final String? name = checkRunEvent.checkRun!.name;
bool success = false;
if (name == kCiYamlCheckName) {
- // The [CheckRunEvent.checkRun.pullRequests] array is empty for this
+ // The CheckRunEvent.checkRun.pullRequests array is empty for this
// event, so we need to find the matching pull request.
final RepositorySlug slug = checkRunEvent.repository!.slug();
final String headSha = checkRunEvent.checkRun!.headSha!;
@@ -471,7 +471,42 @@
}
} else {
try {
- await luciBuildService.rescheduleUsingCheckRunEvent(checkRunEvent);
+ final String sha = checkRunEvent.checkRun!.headSha!;
+ final String checkName = checkRunEvent.checkRun!.name!;
+ final RepositorySlug slug = checkRunEvent.repository!.slug();
+
+ // TODO(nehalvpatel): Use head_branch from checkRunEvent.checkRun.checkSuite, https://github.com/flutter/flutter/issues/119171
+ final String gitBranch = Config.defaultBranch(slug);
+
+ // Only merged commits are added to the datastore. If a matching commit is found, this must be a postsubmit checkrun.
+ datastore = datastoreProvider(config.db);
+ final Key<String> commitKey =
+ Commit.createKey(db: datastore.db, slug: slug, gitBranch: gitBranch, sha: sha);
+ Commit? commit;
+ try {
+ commit = await Commit.fromDatastore(datastore: datastore, key: commitKey);
+ log.fine('Commit found in datastore.');
+ } on KeyNotFoundException {
+ log.fine('Commit not found in datastore.');
+ }
+
+ if (commit == null) {
+ log.fine('Rescheduling presubmit build.');
+ await luciBuildService.reschedulePresubmitBuildUsingCheckRunEvent(checkRunEvent);
+ } else {
+ log.fine('Rescheduling postsubmit build.');
+ final Task task = await Task.fromDatastore(datastore: datastore, commitKey: commitKey, name: checkName);
+ final CiYaml ciYaml = await getCiYaml(commit);
+ final Target target =
+ ciYaml.postsubmitTargets.singleWhere((Target target) => target.value.name == task.name);
+ await luciBuildService.reschedulePostsubmitBuildUsingCheckRunEvent(
+ checkRunEvent,
+ commit: commit,
+ task: task,
+ target: target,
+ );
+ }
+
success = true;
} on NoBuildFoundException {
log.warning('No build found to reschedule.');
diff --git a/app_dart/test/model/commit_test.dart b/app_dart/test/model/commit_test.dart
new file mode 100644
index 0000000..3907e77
--- /dev/null
+++ b/app_dart/test/model/commit_test.dart
@@ -0,0 +1,61 @@
+// Copyright 2023 The Flutter Authors. 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:cocoon_service/src/model/appengine/commit.dart';
+import 'package:cocoon_service/src/service/datastore.dart';
+import 'package:gcloud/db.dart';
+import 'package:github/github.dart';
+import 'package:test/test.dart';
+
+import '../src/datastore/fake_config.dart';
+import '../src/datastore/fake_datastore.dart';
+import '../src/utilities/entity_generators.dart';
+
+void main() {
+ group('Commit.composeKey', () {
+ test('creates valid key', () {
+ final FakeDatastoreDB db = FakeDatastoreDB();
+ final RepositorySlug slug = RepositorySlug('flutter', 'flutter');
+ const String gitBranch = 'main';
+ const String sha = 'abc';
+ final key = Commit.createKey(db: db, slug: slug, gitBranch: gitBranch, sha: sha);
+ expect(key.id, equals('flutter/flutter/main/abc'));
+ });
+ });
+
+ group('Commit.fromDatastore', () {
+ late FakeConfig config;
+ late Commit expectedCommit;
+
+ setUp(() {
+ config = FakeConfig();
+ expectedCommit = generateCommit(1);
+ config.db.values[expectedCommit.key] = expectedCommit;
+ });
+
+ test('look up by id', () async {
+ final Commit commit = await Commit.fromDatastore(
+ datastore: DatastoreService(config.db, 5),
+ key: expectedCommit.key,
+ );
+ expect(commit, expectedCommit);
+ });
+
+ test('look up by id fails if cannot be found', () async {
+ final datastore = DatastoreService(config.db, 5);
+ expect(
+ Commit.fromDatastore(
+ datastore: datastore,
+ key: Commit.createKey(
+ db: datastore.db,
+ slug: RepositorySlug('abc', 'test'),
+ gitBranch: 'main',
+ sha: 'def',
+ ),
+ ),
+ throwsA(isA<KeyNotFoundException>()),
+ );
+ });
+ });
+}
diff --git a/app_dart/test/model/task_test.dart b/app_dart/test/model/task_test.dart
index 87381b8..b2e9d3f 100644
--- a/app_dart/test/model/task_test.dart
+++ b/app_dart/test/model/task_test.dart
@@ -2,11 +2,15 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'package:cocoon_service/src/model/appengine/commit.dart';
import 'package:cocoon_service/src/model/appengine/task.dart';
import 'package:cocoon_service/src/model/luci/push_message.dart';
+import 'package:cocoon_service/src/request_handling/exceptions.dart';
+import 'package:cocoon_service/src/service/datastore.dart';
import 'package:gcloud/db.dart';
import 'package:test/test.dart';
+import '../src/datastore/fake_config.dart';
import '../src/utilities/entity_generators.dart';
void main() {
@@ -174,9 +178,89 @@
});
});
});
-}
-void validateModel(Task task) {
- // Throws an exception when property validation fails.
- ModelDBImpl().toDatastoreEntity(task);
+ // TODO(chillers): There is a bug where `dart test` does not work in offline mode.
+ // Need to file issue and get traces.
+ group('Task.fromDatastore', () {
+ late FakeConfig config;
+ late Commit commit;
+ late Task expectedTask;
+
+ setUp(() {
+ config = FakeConfig();
+ commit = generateCommit(1);
+ expectedTask = generateTask(1, parent: commit);
+ config.db.values[commit.key] = commit;
+ config.db.values[expectedTask.key] = expectedTask;
+ });
+
+ test('look up by id', () async {
+ final Task task = await Task.fromDatastore(
+ datastore: DatastoreService(config.db, 5),
+ commitKey: commit.key,
+ id: '${expectedTask.id}',
+ );
+ expect(task, expectedTask);
+ });
+
+ test('look up by id fails if cannot be found', () async {
+ expect(
+ Task.fromDatastore(
+ datastore: DatastoreService(config.db, 5),
+ commitKey: commit.key,
+ id: '12345',
+ ),
+ throwsA(isA<KeyNotFoundException>()),
+ );
+ });
+
+ test('look up by name', () async {
+ final Task task = await Task.fromDatastore(
+ datastore: DatastoreService(config.db, 5),
+ commitKey: commit.key,
+ name: expectedTask.name,
+ );
+ expect(task, expectedTask);
+ });
+
+ test('look up by name fails if cannot be found', () async {
+ try {
+ await Task.fromDatastore(
+ datastore: DatastoreService(config.db, 5),
+ commitKey: commit.key,
+ name: 'Linux not_found',
+ );
+ } catch (e) {
+ expect(e, isA<InternalServerError>());
+ expect(
+ e.toString(),
+ equals(
+ 'HTTP 500: Expected to find 1 task for Linux not_found, but found 0',
+ ),
+ );
+ }
+ });
+
+ test('look up by name fails if multiple Tasks with the same name are found', () async {
+ final DatastoreService datastore = DatastoreService(config.db, 5);
+ final String taskName = expectedTask.name!;
+ final Task duplicatedTask = generateTask(2, parent: commit, name: taskName);
+ config.db.values[duplicatedTask.key] = duplicatedTask;
+ try {
+ await Task.fromDatastore(
+ datastore: datastore,
+ commitKey: commit.key,
+ name: taskName,
+ );
+ } catch (e) {
+ expect(e, isA<InternalServerError>());
+ expect(
+ e.toString(),
+ equals(
+ 'HTTP 500: Expected to find 1 task for $taskName, but found 2',
+ ),
+ );
+ }
+ });
+ });
}
diff --git a/app_dart/test/service/gerrit_service_test.dart b/app_dart/test/service/gerrit_service_test.dart
index 80edcfd..cc94816 100644
--- a/app_dart/test/service/gerrit_service_test.dart
+++ b/app_dart/test/service/gerrit_service_test.dart
@@ -74,6 +74,9 @@
expect(commit.author?.email, 'dash@flutter.dev');
expect(commit.author?.name, 'Dash');
expect(commit.author?.time, isNotNull);
+ expect(commit.committer?.email, 'flutter-scoped@luci-project-accounts.iam.gserviceaccount.com');
+ expect(commit.committer?.name, 'CQ Bot Account');
+ expect(commit.committer?.time, isNotNull);
final DateTime time = commit.author!.time!;
final DateTime expectedTime = DateTime.utc(2022, 7, 12, 17, 21, 25);
expect(time, expectedTime);
diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart
index af50931..19a6459 100644
--- a/app_dart/test/service/luci_build_service_test.dart
+++ b/app_dart/test/service/luci_build_service_test.dart
@@ -557,8 +557,15 @@
final Map<String, dynamic> jsonSubMap = json.decode(jsonMap['2']);
final cocoon_checks.CheckRunEvent checkRunEvent = cocoon_checks.CheckRunEvent.fromJson(jsonSubMap);
- expect(() async => await service.rescheduleUsingCheckRunEvent(checkRunEvent),
- throwsA(const TypeMatcher<NoBuildFoundException>()));
+ expect(
+ () async => await service.reschedulePostsubmitBuildUsingCheckRunEvent(
+ checkRunEvent,
+ commit: generateCommit(0),
+ task: generateTask(0),
+ target: generateTarget(0),
+ ),
+ throwsA(const TypeMatcher<NoBuildFoundException>()),
+ );
});
test('do not create postsubmit checkrun for bringup: true target', () async {
diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart
index 78c1054..807b7cb 100644
--- a/app_dart/test/service/scheduler_test.dart
+++ b/app_dart/test/service/scheduler_test.dart
@@ -32,6 +32,7 @@
import '../src/datastore/fake_datastore.dart';
import '../src/service/fake_build_status_provider.dart';
import '../src/request_handling/fake_pubsub.dart';
+import '../src/service/fake_buildbucket.dart';
import '../src/service/fake_gerrit_service.dart';
import '../src/service/fake_github_service.dart';
import '../src/service/fake_luci_build_service.dart';
@@ -450,18 +451,105 @@
verify(mockGithubChecksUtil.createCheckRun(any, any, any, any)).called(1);
});
- test('rerequested triggers a single luci build', () async {
- when(mockGithubChecksUtil.createCheckRun(any, any, any, any)).thenAnswer((_) async {
- return CheckRun.fromJson(const <String, dynamic>{
- 'id': 1,
- 'started_at': '2020-05-10T02:49:31Z',
- 'check_suite': <String, dynamic>{'id': 2},
- });
+ test('rerequested presubmit check triggers presubmit build', () async {
+ // Note that we're not inserting any commits into the db, because
+ // only postsubmit commits are stored in the datastore.
+ config = FakeConfig(dbValue: db);
+ db = FakeDatastoreDB();
+
+ // Set up mock buildbucket to validate which bucket is requested.
+ final MockBuildBucketClient mockBuildbucket = MockBuildBucketClient();
+ when(mockBuildbucket.batch(any)).thenAnswer((i) async {
+ return FakeBuildBucketClient().batch(i.positionalArguments[0]);
});
+ when(mockBuildbucket.scheduleBuild(any, buildBucketUri: anyNamed('buildBucketUri')))
+ .thenAnswer((realInvocation) async {
+ final ScheduleBuildRequest scheduleBuildRequest = realInvocation.positionalArguments[0];
+ // Ensure this is an attempt to schedule a presubmit build by
+ // verifying that bucket == 'try'.
+ expect(scheduleBuildRequest.builderId.bucket, equals('try'));
+ return const Build(builderId: BuilderId(), id: '');
+ });
+
+ scheduler = Scheduler(
+ cache: cache,
+ config: config,
+ githubChecksService: GithubChecksService(config, githubChecksUtil: mockGithubChecksUtil),
+ luciBuildService: FakeLuciBuildService(
+ config: config,
+ githubChecksUtil: mockGithubChecksUtil,
+ buildbucket: mockBuildbucket,
+ ),
+ );
final cocoon_checks.CheckRunEvent checkRunEvent = cocoon_checks.CheckRunEvent.fromJson(
jsonDecode(checkRunString) as Map<String, dynamic>,
);
expect(await scheduler.processCheckRun(checkRunEvent), true);
+ verify(mockBuildbucket.scheduleBuild(any, buildBucketUri: anyNamed('buildBucketUri'))).called(1);
+ verify(mockGithubChecksUtil.createCheckRun(any, any, any, any)).called(1);
+ });
+
+ test('rerequested postsubmit check triggers postsubmit build', () async {
+ // Set up datastore with postsubmit entities matching [checkRunString].
+ db = FakeDatastoreDB();
+ config = FakeConfig(dbValue: db, postsubmitSupportedReposValue: {RepositorySlug('abc', 'cocoon')});
+ final Commit commit = generateCommit(
+ 1,
+ sha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c',
+ branch: 'master',
+ owner: 'abc',
+ repo: 'cocoon',
+ );
+ config.db.values[commit.key] = commit;
+ final Task task = generateTask(1, name: "test1", parent: commit);
+ config.db.values[task.key] = task;
+
+ // Set up ci.yaml with task name from [checkRunString].
+ httpClient = MockClient((http.Request request) async {
+ if (request.url.path.contains('.ci.yaml')) {
+ return http.Response(
+ r'''
+enabled_branches:
+ - master
+targets:
+ - name: test1
+''',
+ 200,
+ );
+ }
+ throw Exception('Failed to find ${request.url.path}');
+ });
+
+ // Set up mock buildbucket to validate which bucket is requested.
+ final MockBuildBucketClient mockBuildbucket = MockBuildBucketClient();
+ when(mockBuildbucket.batch(any)).thenAnswer((i) async {
+ return FakeBuildBucketClient().batch(i.positionalArguments[0]);
+ });
+ when(mockBuildbucket.scheduleBuild(any, buildBucketUri: anyNamed('buildBucketUri')))
+ .thenAnswer((realInvocation) async {
+ final ScheduleBuildRequest scheduleBuildRequest = realInvocation.positionalArguments[0];
+ // Ensure this is an attempt to schedule a postsubmit build by
+ // verifying that bucket == 'prod'.
+ expect(scheduleBuildRequest.builderId.bucket, equals('prod'));
+ return const Build(builderId: BuilderId(), id: '');
+ });
+
+ scheduler = Scheduler(
+ cache: cache,
+ config: config,
+ githubChecksService: GithubChecksService(config, githubChecksUtil: mockGithubChecksUtil),
+ httpClientProvider: () => httpClient,
+ luciBuildService: FakeLuciBuildService(
+ config: config,
+ githubChecksUtil: mockGithubChecksUtil,
+ buildbucket: mockBuildbucket,
+ ),
+ );
+ final cocoon_checks.CheckRunEvent checkRunEvent = cocoon_checks.CheckRunEvent.fromJson(
+ jsonDecode(checkRunString) as Map<String, dynamic>,
+ );
+ expect(await scheduler.processCheckRun(checkRunEvent), true);
+ verify(mockBuildbucket.scheduleBuild(any, buildBucketUri: anyNamed('buildBucketUri'))).called(1);
verify(mockGithubChecksUtil.createCheckRun(any, any, any, any)).called(1);
});
diff --git a/app_dart/test/src/datastore/fake_config.dart b/app_dart/test/src/datastore/fake_config.dart
index 7233616..057a31c 100644
--- a/app_dart/test/src/datastore/fake_config.dart
+++ b/app_dart/test/src/datastore/fake_config.dart
@@ -234,10 +234,7 @@
@override
bool githubPostsubmitSupportedRepo(gh.RepositorySlug slug) {
- return <gh.RepositorySlug>[
- Config.packagesSlug,
- Config.pluginsSlug,
- ].contains(slug);
+ return postsubmitSupportedRepos.contains(slug);
}
@override
@@ -279,7 +276,7 @@
@override
Set<gh.RepositorySlug> get postsubmitSupportedRepos =>
- postsubmitSupportedReposValue ?? <gh.RepositorySlug>{Config.packagesSlug};
+ postsubmitSupportedReposValue ?? <gh.RepositorySlug>{Config.packagesSlug, Config.pluginsSlug};
@override
Future<Iterable<Branch>> getBranches(gh.RepositorySlug slug) async {
diff --git a/app_dart/test/src/datastore/fake_datastore.dart b/app_dart/test/src/datastore/fake_datastore.dart
index 3b961ae..f749dd4 100644
--- a/app_dart/test/src/datastore/fake_datastore.dart
+++ b/app_dart/test/src/datastore/fake_datastore.dart
@@ -144,8 +144,8 @@
/// A query that will return all values of type `T` that exist in the
/// [FakeDatastoreDB.values] map.
///
-/// This fake query ignores any [filter] or [order] directives, though it does
-/// respect [limit] and [offset] directives.
+/// This fake query respects [order], [limit], and [offset]. However, [filter]
+/// may require local additions here to respect new filters.
class FakeQuery<T extends Model<dynamic>> implements Query<T> {
FakeQuery._(this.db, this.results);
@@ -189,14 +189,14 @@
Stream<T> run() {
Iterable<T> resultsView = results;
- // This considers only the special case when there exists [branch] or [pr] filter.
for (FakeFilterSpec filter in filters) {
final String filterString = filter.filterString;
final Object? value = filter.comparisonObject;
if (filterString.contains('branch =') ||
filterString.contains('head =') ||
filterString.contains('pr =') ||
- filterString.contains('repository =')) {
+ filterString.contains('repository =') ||
+ filterString.contains('name =')) {
resultsView = resultsView.where((T result) => result.toString().contains(value.toString()));
}
}
@@ -214,6 +214,9 @@
final String filterString;
final Object? comparisonObject;
+
+ @override
+ String toString() => 'FakeFilterSpec($filterString, $comparisonObject)';
}
class FakeOrderSpec {
diff --git a/app_dart/test/src/service/fake_buildbucket.dart b/app_dart/test/src/service/fake_buildbucket.dart
index b5b4bad..17ee23a 100644
--- a/app_dart/test/src/service/fake_buildbucket.dart
+++ b/app_dart/test/src/service/fake_buildbucket.dart
@@ -62,7 +62,7 @@
'cipd_version': <String>['refs/heads/main']
},
input: Input(
- properties: <String, dynamic>{'bringup': 'true'},
+ properties: <String, Object>{'bringup': 'true'},
),
),
],
diff --git a/app_dart/test/src/utilities/entity_generators.dart b/app_dart/test/src/utilities/entity_generators.dart
index 383b24e..c3dba7e 100644
--- a/app_dart/test/src/utilities/entity_generators.dart
+++ b/app_dart/test/src/utilities/entity_generators.dart
@@ -15,23 +15,24 @@
import '../service/fake_scheduler.dart';
-Key<T> generateKey<T>(Type type, T id) => Key<T>.emptyKey(Partition('flutter-dashboard')).append<T>(type, id: id);
+Key<T> generateKey<T>(Type type, T id) => Key<T>.emptyKey(Partition(null)).append<T>(type, id: id);
Commit generateCommit(
int i, {
String? sha,
String branch = 'master',
+ String? owner = 'flutter',
String repo = 'flutter',
int? timestamp,
}) =>
Commit(
sha: sha ?? '$i',
timestamp: timestamp ?? i,
- repository: 'flutter/$repo',
+ repository: '$owner/$repo',
branch: branch,
key: generateKey<String>(
Commit,
- 'flutter/$repo/$branch/${sha ?? '$i'}',
+ '$owner/$repo/$branch/${sha ?? '$i'}',
),
);
diff --git a/app_dart/test/src/utilities/mocks.mocks.dart b/app_dart/test/src/utilities/mocks.mocks.dart
index 394960c..7f6ba5d 100644
--- a/app_dart/test/src/utilities/mocks.mocks.dart
+++ b/app_dart/test/src/utilities/mocks.mocks.dart
@@ -9,9 +9,9 @@
import 'dart:typed_data' as _i27;
import 'package:cocoon_service/src/foundation/github_checks_util.dart' as _i11;
-import 'package:cocoon_service/src/foundation/utils.dart' as _i35;
+import 'package:cocoon_service/src/foundation/utils.dart' as _i36;
import 'package:cocoon_service/src/model/appengine/commit.dart' as _i34;
-import 'package:cocoon_service/src/model/appengine/task.dart' as _i36;
+import 'package:cocoon_service/src/model/appengine/task.dart' as _i35;
import 'package:cocoon_service/src/model/ci_yaml/target.dart' as _i32;
import 'package:cocoon_service/src/model/github/checks.dart' as _i33;
import 'package:cocoon_service/src/model/luci/buildbucket.dart' as _i9;
@@ -4877,20 +4877,51 @@
)),
) as _i17.Future<_i9.Build>);
@override
- _i17.Future<_i9.Build> rescheduleUsingCheckRunEvent(_i33.CheckRunEvent? checkRunEvent) => (super.noSuchMethod(
+ _i17.Future<_i9.Build> reschedulePresubmitBuildUsingCheckRunEvent(_i33.CheckRunEvent? checkRunEvent) =>
+ (super.noSuchMethod(
Invocation.method(
- #rescheduleUsingCheckRunEvent,
+ #reschedulePresubmitBuildUsingCheckRunEvent,
[checkRunEvent],
),
returnValue: _i17.Future<_i9.Build>.value(_FakeBuild_8(
this,
Invocation.method(
- #rescheduleUsingCheckRunEvent,
+ #reschedulePresubmitBuildUsingCheckRunEvent,
[checkRunEvent],
),
)),
) as _i17.Future<_i9.Build>);
@override
+ _i17.Future<_i9.Build> reschedulePostsubmitBuildUsingCheckRunEvent(
+ _i33.CheckRunEvent? checkRunEvent, {
+ required _i34.Commit? commit,
+ required _i35.Task? task,
+ required _i32.Target? target,
+ }) =>
+ (super.noSuchMethod(
+ Invocation.method(
+ #reschedulePostsubmitBuildUsingCheckRunEvent,
+ [checkRunEvent],
+ {
+ #commit: commit,
+ #task: task,
+ #target: target,
+ },
+ ),
+ returnValue: _i17.Future<_i9.Build>.value(_FakeBuild_8(
+ this,
+ Invocation.method(
+ #reschedulePostsubmitBuildUsingCheckRunEvent,
+ [checkRunEvent],
+ {
+ #commit: commit,
+ #task: task,
+ #target: target,
+ },
+ ),
+ )),
+ ) as _i17.Future<_i9.Build>);
+ @override
_i17.Future<_i9.Build> getBuildById(
String? id, {
String? fields,
@@ -4929,7 +4960,7 @@
@override
_i17.Future<void> schedulePostsubmitBuilds({
required _i34.Commit? commit,
- required List<_i35.Tuple<_i32.Target, _i36.Task, int>>? toBeScheduled,
+ required List<_i36.Tuple<_i32.Target, _i35.Task, int>>? toBeScheduled,
}) =>
(super.noSuchMethod(
Invocation.method(
@@ -4965,7 +4996,7 @@
_i17.Future<bool> checkRerunBuilder({
required _i34.Commit? commit,
required _i32.Target? target,
- required _i36.Task? task,
+ required _i35.Task? task,
required _i37.DatastoreService? datastore,
Map<String, List<String>>? tags,
bool? ignoreChecks = false,
diff --git a/licenses/check_licenses.dart b/licenses/check_licenses.dart
index 3a9ae9c..5d0494d 100644
--- a/licenses/check_licenses.dart
+++ b/licenses/check_licenses.dart
@@ -32,7 +32,7 @@
// TESTS
String _generateLicense(String prefix) {
- return '${prefix}Copyright (2014|2015|2016|2017|2018|2019|2020|2021|2022) The Flutter Authors. All rights reserved.\n'
+ return '${prefix}Copyright (2014|2015|2016|2017|2018|2019|2020|2021|2022|2023) The Flutter Authors. All rights reserved.\n'
'${prefix}Use of this source code is governed by a BSD-style license that can be\n'
'${prefix}found in the LICENSE file.';
}