diff --git a/app_dart/lib/src/model/firestore/task.dart b/app_dart/lib/src/model/firestore/task.dart index e92fcb13c..773e8d70a 100644 --- a/app_dart/lib/src/model/firestore/task.dart +++ b/app_dart/lib/src/model/firestore/task.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:cocoon_service/cocoon_service.dart'; import 'package:googleapis/firestore/v1.dart' hide Status; import '../../request_handling/exceptions.dart'; @@ -77,24 +78,27 @@ class Task extends Document { /// /// This is _not_ when the task first started running, as tasks start out in /// the 'New' state until they've been picked up by an [Agent]. - int? get createTimestamp => int.parse(fields!['createTimestamp']!.integerValue!); + int? get createTimestamp => int.parse(fields![kTaskCreateTimestampField]!.integerValue!); /// The timestamp (in milliseconds since the Epoch) that this task started /// running. /// /// Tasks may be run more than once. If this task has been run more than /// once, this timestamp represents when the task was most recently started. - int? get startTimestamp => int.parse(fields!['startTimestamp']!.integerValue!); + int? get startTimestamp => int.parse(fields![kTaskStartTimestampField]!.integerValue!); /// The timestamp (in milliseconds since the Epoch) that this task last /// finished running. - int? get endTimestamp => int.parse(fields!['endTimestamp']!.integerValue!); + int? get endTimestamp => int.parse(fields![kTaskEndTimestampField]!.integerValue!); /// The name of the task. /// /// This is a human-readable name, typically a test name (e.g. /// "hello_world__memory"). - String? get taskName => fields!['endTimestamp']!.stringValue!; + String? get taskName => fields![kTaskNameField]!.stringValue!; + + /// The sha of the task commit. + String? get commitSha => fields![kTaskCommitShaField]!.stringValue!; /// The number of attempts that have been made to run this task successfully. /// @@ -109,7 +113,7 @@ class Task extends Document { /// * /// /// A flaky (`bringup: true`) task will not block the tree. - bool? get bringup => fields!['bringup']!.booleanValue!; + bool? get bringup => fields![kTaskBringupField]!.booleanValue!; /// Whether the test execution of this task shows flake. /// @@ -117,16 +121,17 @@ class Task extends Document { /// /// See also: /// * - bool? get testFlaky => fields!['testFlaky']!.booleanValue!; + bool? get testFlaky => fields![kTaskTestFlakyField]!.booleanValue!; /// The build number of luci build: https://chromium.googlesource.com/infra/luci/luci-go/+/master/buildbucket/proto/build.proto#146 - int? get buildNumber => fields!.containsKey('buildNumber') ? int.parse(fields!['buildNumber']!.integerValue!) : null; + int? get buildNumber => + fields!.containsKey(kTaskBuildNumberField) ? int.parse(fields![kTaskBuildNumberField]!.integerValue!) : null; /// The status of the task. /// /// Legal values and their meanings are defined in [legalStatusValues]. String get status { - final String taskStatus = fields!['status']!.stringValue!; + final String taskStatus = fields![kTaskStatusField]!.stringValue!; if (!legalStatusValues.contains(taskStatus)) { throw ArgumentError('Invalid state: "$taskStatus"'); } @@ -137,10 +142,18 @@ class Task extends Document { if (!legalStatusValues.contains(value)) { throw ArgumentError('Invalid state: "$value"'); } - fields!['status'] = Value(stringValue: value); + fields![kTaskStatusField] = Value(stringValue: value); return value; } + void setEndTimestamp(int endTimestamp) { + fields![kTaskEndTimestampField] = Value(integerValue: endTimestamp.toString()); + } + + void setTestFlaky(bool testFlaky) { + fields![kTaskTestFlakyField] = Value(booleanValue: testFlaky); + } + /// Update [Task] fields based on a LUCI [Build]. void updateFromBuild(Build build) { final List? tags = build.tags; @@ -150,14 +163,34 @@ class Task extends Document { log.warning('Tags: $tags'); throw const BadRequestException('build_address does not contain build number'); } - fields!['buildNumber'] = Value(integerValue: buildAddress.split('/').last); - fields!['createTimestamp'] = Value(integerValue: (build.createdTimestamp?.millisecondsSinceEpoch ?? 0).toString()); - fields!['startTimestamp'] = Value(integerValue: (build.startedTimestamp?.millisecondsSinceEpoch ?? 0).toString()); - fields!['endTimestamp'] = Value(integerValue: (build.completedTimestamp?.millisecondsSinceEpoch ?? 0).toString()); + fields![kTaskBuildNumberField] = Value(integerValue: buildAddress.split('/').last); + fields![kTaskCreateTimestampField] = Value( + integerValue: (build.createdTimestamp?.millisecondsSinceEpoch ?? kTaskDefaultTimestampValue).toString(), + ); + fields![kTaskStartTimestampField] = Value( + integerValue: (build.startedTimestamp?.millisecondsSinceEpoch ?? kTaskDefaultTimestampValue).toString(), + ); + fields![kTaskEndTimestampField] = Value( + integerValue: (build.completedTimestamp?.millisecondsSinceEpoch ?? kTaskDefaultTimestampValue).toString(), + ); _setStatusFromLuciStatus(build); } + void resetAsRetry({int attempt = 1}) { + name = '$kDatabase/documents/$kTaskCollectionId/${commitSha}_${taskName}_$attempt'; + fields = { + kTaskCreateTimestampField: Value(integerValue: DateTime.now().millisecondsSinceEpoch.toString()), + kTaskEndTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()), + kTaskBringupField: Value(booleanValue: bringup), + kTaskNameField: Value(stringValue: taskName), + kTaskStartTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()), + kTaskStatusField: Value(stringValue: Task.statusNew), + kTaskTestFlakyField: Value(booleanValue: false), + kTaskCommitShaField: Value(stringValue: commitSha), + }; + } + /// Get a [Task] status from a LUCI [Build] status/result. String _setStatusFromLuciStatus(Build build) { // Updates can come out of order. Ensure completed statuses are kept. @@ -201,13 +234,13 @@ class Task extends Document { String toString() { final StringBuffer buf = StringBuffer() ..write('$runtimeType(') - ..write(', createTimestamp: $createTimestamp') - ..write(', startTimestamp: $startTimestamp') - ..write(', endTimestamp: $endTimestamp') - ..write(', name: $name') - ..write(', bringup: $bringup') - ..write(', testRunFlaky: $testFlaky') - ..write(', status: $status') + ..write(', $kTaskCreateTimestampField: $createTimestamp') + ..write(', $kTaskStartTimestampField: $startTimestamp') + ..write(', $kTaskEndTimestampField: $endTimestamp') + ..write(', $kTaskNameField: $name') + ..write(', $kTaskBringupField: $bringup') + ..write(', $kTaskTestFlakyField: $testFlaky') + ..write(', $kTaskStatusField: $status') ..write(')'); return buf.toString(); } diff --git a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart index 90aa1da6b..2a5bb783e 100644 --- a/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart +++ b/app_dart/lib/src/request_handlers/postsubmit_luci_subscription.dart @@ -9,7 +9,7 @@ import 'package:meta/meta.dart'; import '../model/appengine/commit.dart'; import '../model/appengine/task.dart'; -import '../model/firestore/task.dart' as f; +import '../model/firestore/task.dart' as firestore; import '../model/luci/push_message.dart'; import '../request_handling/body.dart'; import '../request_handling/exceptions.dart'; @@ -45,6 +45,7 @@ class PostsubmitLuciSubscription extends SubscriptionHandler { @override Future post() async { final DatastoreService datastore = datastoreProvider(config.db); + final FirestoreService firestoreService = await config.createFirestoreService(); final BuildPushMessage buildPushMessage = BuildPushMessage.fromPushMessage(message); log.fine('userData=${buildPushMessage.userData}'); @@ -83,12 +84,14 @@ class PostsubmitLuciSubscription extends SubscriptionHandler { } log.fine('Found $task'); + firestore.Task taskDocument = firestore.Task(); + if (_shouldUpdateTask(build, task)) { final String oldTaskStatus = task.status; task.updateFromBuild(build); await datastore.insert([task]); try { - await updateFirestore(build, rawCommitKey, task.name!); + taskDocument = await updateFirestore(build, rawCommitKey, task.name!, firestoreService); } catch (error) { log.warning('Failed to update task in Firestore: $error'); } @@ -114,6 +117,8 @@ class PostsubmitLuciSubscription extends SubscriptionHandler { target: target, task: task, datastore: datastore, + taskDocument: taskDocument, + firestoreService: firestoreService, ); log.info('Retried: $retried'); } @@ -144,17 +149,23 @@ class PostsubmitLuciSubscription extends SubscriptionHandler { } /// Queries the task document and updates based on the latest build data. - Future updateFirestore(Build build, String commitKeyId, String taskName) async { - final FirestoreService firestoreService = await config.createFirestoreService(); + Future updateFirestore( + Build build, + String commitKeyId, + String taskName, + FirestoreService firestoreService, + ) async { + final int currentAttempt = githubChecksService.currentAttempt(build); final String sha = commitKeyId.split('/').last; - final String documentName = '$kDatabase/documents/tasks/${sha}_${taskName}_1'; + final String documentName = '$kDatabase/documents/tasks/${sha}_${taskName}_$currentAttempt'; log.info('getting firestore document: $documentName'); - final f.Task firestoreTask = - await f.Task.fromFirestore(firestoreService: firestoreService, documentName: documentName); + final firestore.Task firestoreTask = + await firestore.Task.fromFirestore(firestoreService: firestoreService, documentName: documentName); log.info('updating firestoreTask based on build'); firestoreTask.updateFromBuild(build); log.info('finished updating firestoreTask based on builds'); final List writes = documentsToWrites([firestoreTask], exists: true); await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase); + return firestoreTask; } } diff --git a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart index 63d484106..8049eb282 100644 --- a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart @@ -72,7 +72,7 @@ class PresubmitLuciSubscription extends SubscriptionHandler { ); bool rescheduled = false; if (githubChecksService.taskFailed(buildPushMessage)) { - final int currentAttempt = githubChecksService.currentAttempt(buildPushMessage); + final int currentAttempt = githubChecksService.currentAttempt(build); final int maxAttempt = await _getMaxAttempt(buildPushMessage, slug, builderName); if (currentAttempt < maxAttempt) { rescheduled = true; diff --git a/app_dart/lib/src/request_handlers/push_gold_status_to_github.dart b/app_dart/lib/src/request_handlers/push_gold_status_to_github.dart index d3fdcd9a6..52d6a54db 100644 --- a/app_dart/lib/src/request_handlers/push_gold_status_to_github.dart +++ b/app_dart/lib/src/request_handlers/push_gold_status_to_github.dart @@ -119,11 +119,22 @@ class PushGoldStatusToGithub extends ApiRequestHandler { for (Map checkRun in checkRuns) { log.fine('Check run: $checkRun'); final String name = checkRun['name'].toLowerCase() as String; - // Framework shards run framework goldens - // Web shards run web version of framework goldens - // Misc shard runs API docs goldens - if (name.contains('framework') || name.contains('web engine') || name.contains('misc')) { - runsGoldenFileTests = true; + if (slug == Config.engineSlug) { + if (const [ + 'linux_android_emulator', + 'linux_host_engine', + 'mac_host_engine', + 'linux_web_engine', + ].any((String shardSubString) => name.contains(shardSubString))) { + runsGoldenFileTests = true; + } + } else if (slug == Config.flutterSlug) { + if (const [ + 'framework', + 'misc', + ].any((String shardSubString) => name.contains(shardSubString))) { + runsGoldenFileTests = true; + } } if (checkRun['conclusion'] == null || checkRun['conclusion'].toUpperCase() != 'SUCCESS') { incompleteChecks.add(name); diff --git a/app_dart/lib/src/request_handlers/update_task_status.dart b/app_dart/lib/src/request_handlers/update_task_status.dart index e4e6ed4fd..34ebe4d0c 100644 --- a/app_dart/lib/src/request_handlers/update_task_status.dart +++ b/app_dart/lib/src/request_handlers/update_task_status.dart @@ -5,14 +5,17 @@ import 'dart:async'; import 'package:gcloud/db.dart'; +import 'package:googleapis/firestore/v1.dart'; import 'package:meta/meta.dart'; import '../model/appengine/commit.dart'; import '../model/appengine/task.dart'; +import '../model/firestore/task.dart' as firestore; import '../request_handling/api_request_handler.dart'; import '../request_handling/body.dart'; import '../request_handling/exceptions.dart'; import '../service/datastore.dart'; +import '../service/firestore.dart'; import '../service/logging.dart'; /// Endpoint for task runners to update Cocoon with test run information. @@ -60,9 +63,33 @@ class UpdateTaskStatus extends ApiRequestHandler { task.isTestFlaky = isTestFlaky; await datastore.insert([task]); + + try { + await updateTaskDocument(task.status, task.endTimestamp!, task.isTestFlaky!); + } catch (error) { + log.warning('Failed to update task in Firestore: $error'); + } return UpdateTaskStatusResponse(task); } + Future updateTaskDocument(String status, int endTimestamp, bool isTestFlaky) async { + final FirestoreService firestoreService = await config.createFirestoreService(); + final String sha = (requestData![gitShaParam] as String).trim(); + final String? taskName = requestData![builderNameParam] as String?; + final String documentName = '$kDatabase/documents/tasks/${sha}_${taskName}_1'; + log.info('getting firestore document: $documentName'); + final List initialTasks = await firestoreService.queryCommitTasks(sha); + // Targets the latest task. This assumes only one task is running at any time. + final firestore.Task firestoreTask = initialTasks.where((firestore.Task task) => task.taskName == taskName).reduce( + (firestore.Task current, firestore.Task next) => current.name!.compareTo(next.name!) > 0 ? current : next, + ); + firestoreTask.setStatus(status); + firestoreTask.setEndTimestamp(endTimestamp); + firestoreTask.setTestFlaky(isTestFlaky); + final List writes = documentsToWrites([firestoreTask], exists: true); + await firestoreService.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase); + } + /// Retrieve [Task] from [DatastoreService] when given [gitShaParam], [gitBranchParam], and [builderNameParam]. /// /// This is used when the DeviceLab test runner is uploading results to Cocoon for runs on LUCI. diff --git a/app_dart/lib/src/service/firestore.dart b/app_dart/lib/src/service/firestore.dart index 8494f62d3..50418fb96 100644 --- a/app_dart/lib/src/service/firestore.dart +++ b/app_dart/lib/src/service/firestore.dart @@ -4,16 +4,41 @@ import 'dart:async'; +import 'package:cocoon_service/cocoon_service.dart'; import 'package:googleapis/firestore/v1.dart'; import 'package:http/http.dart'; import '../model/appengine/commit.dart'; import '../model/appengine/task.dart'; +import '../model/firestore/task.dart' as firestore; import '../model/ci_yaml/target.dart'; import 'access_client_provider.dart'; import 'config.dart'; const String kDatabase = 'projects/${Config.flutterGcpProjectId}/databases/${Config.flutterGcpFirestoreDatabase}'; +const String kDocumentParent = '$kDatabase/documents'; +const String kFieldFilterOpEqual = 'EQUAL'; + +const String kTaskCollectionId = 'tasks'; +const int kTaskDefaultTimestampValue = 0; +const String kTaskBringupField = 'bringup'; +const String kTaskBuildNumberField = 'buildNumber'; +const String kTaskCommitShaField = 'commitSha'; +const String kTaskCreateTimestampField = 'createTimestamp'; +const String kTaskEndTimestampField = 'endTimestamp'; +const String kTaskNameField = 'name'; +const String kTaskStartTimestampField = 'startTimestamp'; +const String kTaskStatusField = 'status'; +const String kTaskTestFlakyField = 'testFlaky'; + +const String kCommitCollectionId = 'commits'; +const String kCommitAvatarField = 'avatar'; +const String kCommitBranchField = 'branch'; +const String kCommitCreateTimestampField = 'createTimestamp'; +const String kCommitAuthorField = 'author'; +const String kCommitMessageField = 'message'; +const String kCommitRepositoryPathField = 'repositoryPath'; +const String kCommitShaField = 'sha'; class FirestoreService { const FirestoreService(this.accessClientProvider); @@ -65,22 +90,40 @@ class FirestoreService { CommitRequest(transaction: beginTransactionResponse.transaction, writes: writes); return databasesDocumentsResource.commit(commitRequest, kDatabase); } + + Future> queryCommitTasks(String commitSha) async { + final ProjectsDatabasesDocumentsResource databasesDocumentsResource = await documentResource(); + final List from = [CollectionSelector(collectionId: kTaskCollectionId)]; + final Filter filter = Filter( + fieldFilter: FieldFilter( + field: FieldReference(fieldPath: kTaskCommitShaField), + op: kFieldFilterOpEqual, + value: Value(stringValue: commitSha), + ), + ); + final RunQueryRequest runQueryRequest = + RunQueryRequest(structuredQuery: StructuredQuery(from: from, where: filter)); + final List runQueryResponseElements = + await databasesDocumentsResource.runQuery(runQueryRequest, kDocumentParent); + final List documents = runQueryResponseElements.map((e) => e.document!).toList(); + return documents.map((Document document) => firestore.Task.fromDocument(taskDocument: document)).toList(); + } } /// Generates task documents based on targets. List targetsToTaskDocuments(Commit commit, List targets) { final Iterable iterableDocuments = targets.map( (Target target) => Document( - name: '$kDatabase/documents/tasks/${commit.sha}_${target.value.name}_1', + name: '$kDatabase/documents/$kTaskCollectionId/${commit.sha}_${target.value.name}_1', fields: { - 'createTimestamp': Value(integerValue: commit.timestamp!.toString()), - 'endTimestamp': Value(integerValue: '0'), - 'bringup': Value(booleanValue: target.value.bringup), - 'name': Value(stringValue: target.value.name.toString()), - 'startTimestamp': Value(integerValue: '0'), - 'status': Value(stringValue: Task.statusNew), - 'testFlaky': Value(booleanValue: false), - 'commitSha': Value(stringValue: commit.sha), + kTaskCreateTimestampField: Value(integerValue: commit.timestamp!.toString()), + kTaskEndTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()), + kTaskBringupField: Value(booleanValue: target.value.bringup), + kTaskNameField: Value(stringValue: target.value.name.toString()), + kTaskStartTimestampField: Value(integerValue: kTaskDefaultTimestampValue.toString()), + kTaskStatusField: Value(stringValue: Task.statusNew), + kTaskTestFlakyField: Value(booleanValue: false), + kTaskCommitShaField: Value(stringValue: commit.sha), }, ), ); @@ -90,15 +133,15 @@ List targetsToTaskDocuments(Commit commit, List targets) { /// Generates commit document based on datastore commit data model. Document commitToCommitDocument(Commit commit) { return Document( - name: '$kDatabase/documents/commits/${commit.sha}', + name: '$kDatabase/documents/$kCommitCollectionId/${commit.sha}', fields: { - 'avatar': Value(stringValue: commit.authorAvatarUrl), - 'branch': Value(stringValue: commit.branch), - 'createTimestamp': Value(integerValue: commit.timestamp.toString()), - 'author': Value(stringValue: commit.author), - 'message': Value(stringValue: commit.message), - 'repositoryPath': Value(stringValue: commit.repository), - 'sha': Value(stringValue: commit.sha), + kCommitAvatarField: Value(stringValue: commit.authorAvatarUrl), + kCommitBranchField: Value(stringValue: commit.branch), + kCommitCreateTimestampField: Value(integerValue: commit.timestamp.toString()), + kCommitAuthorField: Value(stringValue: commit.author), + kCommitMessageField: Value(stringValue: commit.message), + kCommitRepositoryPathField: Value(stringValue: commit.repository), + kCommitShaField: Value(stringValue: commit.sha), }, ); } diff --git a/app_dart/lib/src/service/github_checks_service.dart b/app_dart/lib/src/service/github_checks_service.dart index 9a962e24f..48e208505 100644 --- a/app_dart/lib/src/service/github_checks_service.dart +++ b/app_dart/lib/src/service/github_checks_service.dart @@ -144,8 +144,7 @@ class GithubChecksService { /// Returns current reschedule attempt. /// /// It returns 1 if this is the first run, and +1 with each reschedule. - int currentAttempt(push_message.BuildPushMessage buildPushMessage) { - final push_message.Build build = buildPushMessage.build!; + int currentAttempt(push_message.Build build) { if (build.tagsByName('current_attempt').isEmpty) { return 1; } else { diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 7537d5967..4327ac519 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -7,26 +7,23 @@ import 'dart:convert'; import 'dart:math'; import 'dart:typed_data'; +import 'package:cocoon_service/cocoon_service.dart'; import 'package:github/github.dart' as github; import 'package:github/hooks.dart'; +import 'package:googleapis/firestore/v1.dart' hide Status; import 'package:googleapis/pubsub/v1.dart'; import '../foundation/github_checks_util.dart'; -import '../foundation/utils.dart'; import '../model/appengine/commit.dart'; import '../model/appengine/task.dart'; +import '../model/firestore/task.dart' as f; import '../model/ci_yaml/target.dart'; import '../model/github/checks.dart' as cocoon_checks; import '../model/luci/buildbucket.dart'; import '../model/luci/push_message.dart' as push_message; -import '../request_handling/pubsub.dart'; import '../service/datastore.dart'; import '../service/logging.dart'; -import 'buildbucket.dart'; -import 'cache_service.dart'; -import 'config.dart'; import 'exceptions.dart'; -import 'gerrit_service.dart'; import 'github_service.dart'; const Set taskFailStatusSet = { @@ -619,6 +616,8 @@ class LuciBuildService { tags['user_agent'] = ['flutter-cocoon']; // Tag `scheduler_job_id` is needed when calling buildbucket search build API. tags['scheduler_job_id'] = ['flutter/${target.value.name}']; + // Default attempt is the initial attempt, which is 1. + tags['current_attempt'] = tags['current_attempt'] ?? ['1']; final Map processedProperties = target.getProperties(); processedProperties.addAll(properties ?? {}); processedProperties['git_branch'] = commit.branch!; @@ -682,8 +681,10 @@ class LuciBuildService { required Target target, required Task task, required DatastoreService datastore, + FirestoreService? firestoreService, Map>? tags, bool ignoreChecks = false, + f.Task? taskDocument, }) async { if (ignoreChecks == false && await _shouldRerunBuilder(task, commit, datastore) == false) { return false; @@ -692,6 +693,19 @@ class LuciBuildService { tags ??= >{}; tags['trigger_type'] = ['retry']; + // TODO(keyonghan): remove check when [ResetProdTask] supports firestore update. + if (taskDocument != null) { + try { + final int newAttempt = int.parse(taskDocument.name!.split('_').last) + 1; + tags['current_attempt'] = [newAttempt.toString()]; + taskDocument.resetAsRetry(attempt: newAttempt); + final List writes = documentsToWrites([taskDocument]); + await firestoreService!.batchWriteDocuments(BatchWriteRequest(writes: writes), kDatabase); + } catch (error) { + log.warning('Failed to insert retried task in Firestore: $error'); + } + } + final BatchRequest request = BatchRequest( requests: [ Request( diff --git a/app_dart/test/model/firestore/task_test.dart b/app_dart/test/model/firestore/task_test.dart index 836ad2ae8..997630880 100644 --- a/app_dart/test/model/firestore/task_test.dart +++ b/app_dart/test/model/firestore/task_test.dart @@ -146,4 +146,19 @@ void main() { expect(resultedTask.fields, firestoreTask.fields); }); }); + + group('resert as retry', () { + test('success', () { + final Task task = generateFirestoreTask( + 1, + status: Task.statusFailed, + testFlaky: true, + ); + task.resetAsRetry(attempt: 2); + + expect(int.parse(task.name!.split('_').last), 2); + expect(task.status, Task.statusNew); + expect(task.testFlaky, false); + }); + }); } diff --git a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart index f87eb5a77..119f28149 100644 --- a/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/postsubmit_luci_subscription_test.dart @@ -5,7 +5,7 @@ import 'package:cocoon_service/cocoon_service.dart'; import 'package:cocoon_service/src/model/appengine/commit.dart'; import 'package:cocoon_service/src/model/appengine/task.dart'; -import 'package:cocoon_service/src/model/firestore/task.dart' as f; +import 'package:cocoon_service/src/model/firestore/task.dart' as firestore; import 'package:cocoon_service/src/request_handling/exceptions.dart'; import 'package:cocoon_service/src/service/datastore.dart'; import 'package:gcloud/db.dart'; @@ -92,7 +92,7 @@ void main() { }); test('updates task based on message', () async { - final f.Task firestoreTask = generateFirestoreTask(1); + final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 2); when( mockFirestoreService.getDocument( captureAny, @@ -110,6 +110,7 @@ void main() { ).thenAnswer((Invocation invocation) { return Future.value(BatchWriteResponse()); }); + when(mockGithubChecksService.currentAttempt(any)).thenAnswer((_) => 2); final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822'); final Task task = generateTask( 4507531199512576, @@ -301,6 +302,25 @@ void main() { }); test('fallback to build parameters if task_key is not present', () async { + when(mockGithubChecksService.currentAttempt(any)).thenAnswer((_) => 2); + final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 2); + when( + mockFirestoreService.getDocument( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value( + firestoreTask, + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822'); final Task task = generateTask( 4507531199512576, @@ -325,6 +345,25 @@ void main() { test('non-bringup target updates check run', () async { scheduler.ciYaml = nonBringupPackagesConfig; when(mockGithubChecksService.updateCheckStatus(any, any, any)).thenAnswer((_) async => true); + when(mockGithubChecksService.currentAttempt(any)).thenAnswer((_) => 2); + final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 2); + when( + mockFirestoreService.getDocument( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value( + firestoreTask, + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822', repo: 'packages'); final Task task = generateTask( 4507531199512576, @@ -349,6 +388,25 @@ void main() { test('bringup target does not update check run', () async { scheduler.ciYaml = bringupPackagesConfig; when(mockGithubChecksService.updateCheckStatus(any, any, any)).thenAnswer((_) async => true); + when(mockGithubChecksService.currentAttempt(any)).thenAnswer((_) => 2); + final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 2); + when( + mockFirestoreService.getDocument( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value( + firestoreTask, + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822'); final Task task = generateTask( 4507531199512576, @@ -373,6 +431,25 @@ void main() { test('unsupported repo target does not update check run', () async { scheduler.ciYaml = unsupportedPostsubmitCheckrunConfig; when(mockGithubChecksService.updateCheckStatus(any, any, any)).thenAnswer((_) async => true); + when(mockGithubChecksService.currentAttempt(any)).thenAnswer((_) => 2); + final firestore.Task firestoreTask = generateFirestoreTask(1, attempts: 2); + when( + mockFirestoreService.getDocument( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value( + firestoreTask, + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Commit commit = generateCommit(1, sha: '87f88734747805589f2131753620d61b22922822'); final Task task = generateTask( 4507531199512576, diff --git a/app_dart/test/request_handlers/push_gold_status_to_github_test.dart b/app_dart/test/request_handlers/push_gold_status_to_github_test.dart index 80eb75882..261cf28da 100644 --- a/app_dart/test/request_handlers/push_gold_status_to_github_test.dart +++ b/app_dart/test/request_handlers/push_gold_status_to_github_test.dart @@ -767,7 +767,7 @@ void main() { {'name': 'framework', 'status': 'in_progress', 'conclusion': 'neutral'}, ]; engineCheckRuns = [ - {'name': 'web engine', 'status': 'in_progress', 'conclusion': 'neutral'}, + {'name': 'Linux linux_web_engine', 'status': 'in_progress', 'conclusion': 'neutral'}, ]; final Body body = await tester.get(handler); @@ -1482,7 +1482,7 @@ void main() { {'name': 'framework', 'status': 'completed', 'conclusion': 'success'}, ]; engineCheckRuns = [ - {'name': 'web engine', 'status': 'completed', 'conclusion': 'success'}, + {'name': 'Linux linux_web_engine', 'status': 'completed', 'conclusion': 'success'}, ]; // Requests sent to Gold. diff --git a/app_dart/test/request_handlers/update_task_status_test.dart b/app_dart/test/request_handlers/update_task_status_test.dart index 9325d1f93..b495ef36f 100644 --- a/app_dart/test/request_handlers/update_task_status_test.dart +++ b/app_dart/test/request_handlers/update_task_status_test.dart @@ -4,10 +4,13 @@ import 'package:cocoon_service/src/model/appengine/commit.dart'; import 'package:cocoon_service/src/model/appengine/task.dart'; +import 'package:cocoon_service/src/model/firestore/task.dart' as firestore; import 'package:cocoon_service/src/request_handlers/update_task_status.dart'; import 'package:cocoon_service/src/service/config.dart'; import 'package:cocoon_service/src/service/datastore.dart'; import 'package:gcloud/db.dart'; +import 'package:googleapis/firestore/v1.dart'; +import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; import '../src/bigquery/fake_tabledata_resource.dart'; @@ -15,11 +18,14 @@ import '../src/datastore/fake_config.dart'; import '../src/datastore/fake_datastore.dart'; import '../src/request_handling/api_request_handler_tester.dart'; import '../src/request_handling/fake_authentication.dart'; +import '../src/utilities/entity_generators.dart'; +import '../src/utilities/mocks.dart'; void main() { group('UpdateTaskStatus', () { late FakeConfig config; late ApiRequestHandlerTester tester; + late MockFirestoreService mockFirestoreService; late UpdateTaskStatus handler; final FakeTabledataResource tabledataResourceApi = FakeTabledataResource(); late Commit commit; @@ -27,11 +33,13 @@ void main() { const int taskId = 4506830800027648; setUp(() { + mockFirestoreService = MockFirestoreService(); final FakeDatastoreDB datastoreDB = FakeDatastoreDB(); config = FakeConfig( dbValue: datastoreDB, tabledataResource: tabledataResourceApi, maxTaskRetriesValue: 2, + firestoreService: mockFirestoreService, ); tester = ApiRequestHandlerTester(); handler = UpdateTaskStatus( @@ -48,6 +56,25 @@ void main() { }); test('TestFlaky is false when not injected', () async { + final firestore.Task firestoreTask1 = generateFirestoreTask(1, name: 'linux_integration_ui_ios', attempts: 1); + final firestore.Task firestoreTask2 = generateFirestoreTask(2, name: 'linux_integration_ui_ios', attempts: 2); + when( + mockFirestoreService.queryCommitTasks( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future>.value( + [firestoreTask1, firestoreTask2], + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Task task = Task( key: commit.key.append(Task, id: taskId), name: 'integration_ui_ios', @@ -69,9 +96,29 @@ void main() { await tester.post(handler); expect(task.isTestFlaky, false); + expect(firestoreTask2.testFlaky, false); }); test('TestFlaky is true when injected', () async { + final firestore.Task firestoreTask1 = generateFirestoreTask(1, name: 'linux_integration_ui_ios', attempts: 1); + final firestore.Task firestoreTask2 = generateFirestoreTask(2, name: 'linux_integration_ui_ios', attempts: 2); + when( + mockFirestoreService.queryCommitTasks( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future>.value( + [firestoreTask1, firestoreTask2], + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Task task = Task( key: commit.key.append(Task, id: taskId), name: 'integration_ui_ios', @@ -97,6 +144,25 @@ void main() { }); test('task name requests can update tasks', () async { + final firestore.Task firestoreTask1 = generateFirestoreTask(1, name: 'linux_integration_ui_ios', attempts: 1); + final firestore.Task firestoreTask2 = generateFirestoreTask(2, name: 'linux_integration_ui_ios', attempts: 2); + when( + mockFirestoreService.queryCommitTasks( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future>.value( + [firestoreTask1, firestoreTask2], + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); final Task task = Task( key: commit.key.append(Task, id: taskId), name: 'integration_ui_ios', @@ -119,6 +185,8 @@ void main() { expect(task.status, 'Failed'); expect(task.attempts, 1); + expect(firestoreTask2.status, 'Failed'); + expect(firestoreTask2.attempts, 2); }); test('task name requests when task does not exists returns exception', () async { @@ -132,6 +200,25 @@ void main() { }); test('task name request updates when input has whitespace', () async { + final firestore.Task firestoreTask1 = generateFirestoreTask(1, name: 'linux_integration_ui_ios', attempts: 1); + final firestore.Task firestoreTask2 = generateFirestoreTask(2, name: 'linux_integration_ui_ios', attempts: 2); + when( + mockFirestoreService.queryCommitTasks( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future>.value( + [firestoreTask1, firestoreTask2], + ); + }); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); config.db.values[commit.key] = commit; final Task cocoonTask = Task( key: commit.key.append(Task, id: taskId), diff --git a/app_dart/test/service/firestore_test.dart b/app_dart/test/service/firestore_test.dart index 4d642671a..96bc50198 100644 --- a/app_dart/test/service/firestore_test.dart +++ b/app_dart/test/service/firestore_test.dart @@ -21,28 +21,28 @@ void main() { ]; final List taskDocuments = targetsToTaskDocuments(commit, targets); expect(taskDocuments.length, 2); - expect(taskDocuments[0].name, '$kDatabase/documents/tasks/${commit.sha}_${targets[0].value.name}_1'); - expect(taskDocuments[0].fields!['createTimestamp']!.integerValue, commit.timestamp.toString()); - expect(taskDocuments[0].fields!['endTimestamp']!.integerValue, '0'); - expect(taskDocuments[0].fields!['bringup']!.booleanValue, false); - expect(taskDocuments[0].fields!['name']!.stringValue, targets[0].value.name); - expect(taskDocuments[0].fields!['startTimestamp']!.integerValue, '0'); - expect(taskDocuments[0].fields!['status']!.stringValue, Task.statusNew); - expect(taskDocuments[0].fields!['testFlaky']!.booleanValue, false); - expect(taskDocuments[0].fields!['commitSha']!.stringValue, commit.sha); + expect(taskDocuments[0].name, '$kDatabase/documents/$kTaskCollectionId/${commit.sha}_${targets[0].value.name}_1'); + expect(taskDocuments[0].fields![kTaskCreateTimestampField]!.integerValue, commit.timestamp.toString()); + expect(taskDocuments[0].fields![kTaskEndTimestampField]!.integerValue, '0'); + expect(taskDocuments[0].fields![kTaskBringupField]!.booleanValue, false); + expect(taskDocuments[0].fields![kTaskNameField]!.stringValue, targets[0].value.name); + expect(taskDocuments[0].fields![kTaskStartTimestampField]!.integerValue, '0'); + expect(taskDocuments[0].fields![kTaskStatusField]!.stringValue, Task.statusNew); + expect(taskDocuments[0].fields![kTaskTestFlakyField]!.booleanValue, false); + expect(taskDocuments[0].fields![kTaskCommitShaField]!.stringValue, commit.sha); }); test('creates commit document correctly from commit data model', () async { final Commit commit = generateCommit(1); final Document commitDocument = commitToCommitDocument(commit); - expect(commitDocument.name, '$kDatabase/documents/commits/${commit.sha}'); - expect(commitDocument.fields!['avatar']!.stringValue, commit.authorAvatarUrl); - expect(commitDocument.fields!['branch']!.stringValue, commit.branch); - expect(commitDocument.fields!['createTimestamp']!.integerValue, commit.timestamp.toString()); - expect(commitDocument.fields!['author']!.stringValue, commit.author); - expect(commitDocument.fields!['message']!.stringValue, commit.message); - expect(commitDocument.fields!['repositoryPath']!.stringValue, commit.repository); - expect(commitDocument.fields!['sha']!.stringValue, commit.sha); + expect(commitDocument.name, '$kDatabase/documents/$kCommitCollectionId/${commit.sha}'); + expect(commitDocument.fields![kCommitAvatarField]!.stringValue, commit.authorAvatarUrl); + expect(commitDocument.fields![kCommitBranchField]!.stringValue, commit.branch); + expect(commitDocument.fields![kCommitCreateTimestampField]!.integerValue, commit.timestamp.toString()); + expect(commitDocument.fields![kCommitAuthorField]!.stringValue, commit.author); + expect(commitDocument.fields![kCommitMessageField]!.stringValue, commit.message); + expect(commitDocument.fields![kCommitRepositoryPathField]!.stringValue, commit.repository); + expect(commitDocument.fields![kCommitShaField]!.stringValue, commit.sha); }); test('creates writes correctly from documents', () async { diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart index bb687bc28..283ef94ee 100644 --- a/app_dart/test/service/luci_build_service_test.dart +++ b/app_dart/test/service/luci_build_service_test.dart @@ -8,13 +8,15 @@ import 'dart:core'; import 'package:cocoon_service/cocoon_service.dart'; import 'package:cocoon_service/src/model/appengine/commit.dart'; import 'package:cocoon_service/src/model/appengine/task.dart'; +import 'package:cocoon_service/src/model/firestore/task.dart' as f; import 'package:cocoon_service/src/model/ci_yaml/target.dart'; +import 'package:cocoon_service/src/model/github/checks.dart' as cocoon_checks; import 'package:cocoon_service/src/model/luci/buildbucket.dart'; import 'package:cocoon_service/src/model/luci/push_message.dart' as push_message; import 'package:cocoon_service/src/service/exceptions.dart'; import 'package:cocoon_service/src/service/datastore.dart'; import 'package:github/github.dart'; -import 'package:cocoon_service/src/model/github/checks.dart' as cocoon_checks; +import 'package:googleapis/firestore/v1.dart' hide Status; import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; @@ -911,11 +913,13 @@ void main() { late Commit totCommit; late DatastoreService datastore; late MockGithubChecksUtil mockGithubChecksUtil; + late MockFirestoreService mockFirestoreService; setUp(() { cache = CacheService(inMemory: true); config = FakeConfig(); mockBuildBucketClient = MockBuildBucketClient(); mockGithubChecksUtil = MockGithubChecksUtil(); + mockFirestoreService = MockFirestoreService(); when(mockGithubChecksUtil.createCheckRun(any, any, any, any, output: anyNamed('output'))) .thenAnswer((realInvocation) async => generateCheckRun(1)); pubsub = FakePubSub(); @@ -1064,5 +1068,45 @@ void main() { ); expect(rerunFlag, isFalse); }); + + test('insert retried task document to firestore', () async { + final f.Task firestoreTask = generateFirestoreTask(1, attempts: 1); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); + totCommit = generateCommit(1); + config.db.values[totCommit.key] = totCommit; + config.maxLuciTaskRetriesValue = 1; + final Task task = generateTask( + 1, + status: Task.statusInfraFailure, + parent: totCommit, + buildNumber: 1, + ); + final Target target = generateTarget(1); + expect(firestoreTask.attempts, 1); + final bool rerunFlag = await service.checkRerunBuilder( + commit: totCommit, + task: task, + target: target, + datastore: datastore, + firestoreService: mockFirestoreService, + taskDocument: firestoreTask, + ); + expect(rerunFlag, isTrue); + + expect(firestoreTask.attempts, 2); + final List captured = verify(mockFirestoreService.batchWriteDocuments(captureAny, captureAny)).captured; + expect(captured.length, 2); + final BatchWriteRequest batchWriteRequest = captured[0] as BatchWriteRequest; + expect(batchWriteRequest.writes!.length, 1); + final Document insertedTaskDocument = batchWriteRequest.writes![0].update!; + expect(insertedTaskDocument, firestoreTask); + }); }); } diff --git a/app_dart/test/src/utilities/entity_generators.dart b/app_dart/test/src/utilities/entity_generators.dart index a28229840..957d83fe1 100644 --- a/app_dart/test/src/utilities/entity_generators.dart +++ b/app_dart/test/src/utilities/entity_generators.dart @@ -112,7 +112,7 @@ f.Task generateFirestoreTask( final String taskName = name ?? 'task$i'; final String sha = commitSha ?? 'testSha'; final f.Task task = f.Task() - ..name = '${sha}_${taskName}_1' + ..name = '${sha}_${taskName}_$attempts' ..fields = { 'createTimestamp': Value(integerValue: (created?.millisecondsSinceEpoch ?? 0).toString()), 'startTimestamp': Value(integerValue: (started?.millisecondsSinceEpoch ?? 0).toString()), @@ -121,6 +121,7 @@ f.Task generateFirestoreTask( 'testFlaky': Value(booleanValue: testFlaky), 'status': Value(stringValue: status), 'name': Value(stringValue: taskName), + 'commitSha': Value(stringValue: sha), }; if (buildNumber != null) { task.fields!['buildNumber'] = Value(integerValue: buildNumber.toString()); diff --git a/app_dart/test/src/utilities/mocks.mocks.dart b/app_dart/test/src/utilities/mocks.mocks.dart index 2258efeae..a2e8283d1 100644 --- a/app_dart/test/src/utilities/mocks.mocks.dart +++ b/app_dart/test/src/utilities/mocks.mocks.dart @@ -18,10 +18,11 @@ import 'package:cocoon_service/src/model/appengine/github_gold_status_update.dar import 'package:cocoon_service/src/model/appengine/key_helper.dart' as _i12; import 'package:cocoon_service/src/model/appengine/stage.dart' as _i34; import 'package:cocoon_service/src/model/appengine/task.dart' as _i33; -import 'package:cocoon_service/src/model/ci_yaml/target.dart' as _i38; -import 'package:cocoon_service/src/model/github/checks.dart' as _i39; +import 'package:cocoon_service/src/model/ci_yaml/target.dart' as _i39; +import 'package:cocoon_service/src/model/firestore/task.dart' as _i37; +import 'package:cocoon_service/src/model/github/checks.dart' as _i40; import 'package:cocoon_service/src/model/luci/buildbucket.dart' as _i8; -import 'package:cocoon_service/src/model/luci/push_message.dart' as _i37; +import 'package:cocoon_service/src/model/luci/push_message.dart' as _i38; import 'package:cocoon_service/src/service/access_client_provider.dart' as _i5; import 'package:cocoon_service/src/service/access_token_provider.dart' as _i26; import 'package:cocoon_service/src/service/bigquery.dart' as _i16; @@ -41,11 +42,11 @@ import 'package:http/http.dart' as _i2; import 'package:mockito/mockito.dart' as _i1; import 'package:mockito/src/dummies.dart' as _i28; import 'package:neat_cache/neat_cache.dart' as _i25; -import 'package:process/src/interface/process_manager.dart' as _i40; +import 'package:process/src/interface/process_manager.dart' as _i41; import 'package:retry/retry.dart' as _i27; import '../../service/cache_service_test.dart' as _i35; -import 'mocks.dart' as _i41; +import 'mocks.dart' as _i42; // ignore_for_file: type=lint // ignore_for_file: avoid_redundant_argument_values @@ -2590,6 +2591,15 @@ class MockFirestoreService extends _i1.Mock implements _i15.FirestoreService { ), )), ) as _i20.Future<_i21.CommitResponse>); + + @override + _i20.Future> queryCommitTasks(String? commitSha) => (super.noSuchMethod( + Invocation.method( + #queryCommitTasks, + [commitSha], + ), + returnValue: _i20.Future>.value(<_i37.Task>[]), + ) as _i20.Future>); } /// A class which mocks [IssuesService]. @@ -3367,7 +3377,7 @@ class MockGithubChecksService extends _i1.Mock implements _i15.GithubChecksServi @override _i20.Future updateCheckStatus( - _i37.BuildPushMessage? buildPushMessage, + _i38.BuildPushMessage? buildPushMessage, _i15.LuciBuildService? luciBuildService, _i13.RepositorySlug? slug, { bool? rescheduled = false, @@ -3386,7 +3396,7 @@ class MockGithubChecksService extends _i1.Mock implements _i15.GithubChecksServi ) as _i20.Future); @override - bool taskFailed(_i37.BuildPushMessage? buildPushMessage) => (super.noSuchMethod( + bool taskFailed(_i38.BuildPushMessage? buildPushMessage) => (super.noSuchMethod( Invocation.method( #taskFailed, [buildPushMessage], @@ -3395,10 +3405,10 @@ class MockGithubChecksService extends _i1.Mock implements _i15.GithubChecksServi ) as bool); @override - int currentAttempt(_i37.BuildPushMessage? buildPushMessage) => (super.noSuchMethod( + int currentAttempt(_i38.Build? build) => (super.noSuchMethod( Invocation.method( #currentAttempt, - [buildPushMessage], + [build], ), returnValue: 0, ) as int); @@ -3419,7 +3429,7 @@ class MockGithubChecksService extends _i1.Mock implements _i15.GithubChecksServi ) as String); @override - _i13.CheckRunConclusion conclusionForResult(_i37.Result? result) => (super.noSuchMethod( + _i13.CheckRunConclusion conclusionForResult(_i38.Result? result) => (super.noSuchMethod( Invocation.method( #conclusionForResult, [result], @@ -3434,7 +3444,7 @@ class MockGithubChecksService extends _i1.Mock implements _i15.GithubChecksServi ) as _i13.CheckRunConclusion); @override - _i13.CheckRunStatus statusForResult(_i37.Status? status) => (super.noSuchMethod( + _i13.CheckRunStatus statusForResult(_i38.Status? status) => (super.noSuchMethod( Invocation.method( #statusForResult, [status], @@ -6378,8 +6388,8 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { ) as _i20.Future>); @override - _i20.Future> scheduleTryBuilds({ - required List<_i38.Target>? targets, + _i20.Future> scheduleTryBuilds({ + required List<_i39.Target>? targets, required _i13.PullRequest? pullRequest, _i30.CheckSuiteEvent? checkSuiteEvent, }) => @@ -6393,8 +6403,8 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { #checkSuiteEvent: checkSuiteEvent, }, ), - returnValue: _i20.Future>.value(<_i38.Target>[]), - ) as _i20.Future>); + returnValue: _i20.Future>.value(<_i39.Target>[]), + ) as _i20.Future>); @override _i20.Future cancelBuilds( @@ -6416,7 +6426,7 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { @override _i20.Future> failedBuilds( _i13.PullRequest? pullRequest, - List<_i38.Target>? targets, + List<_i39.Target>? targets, ) => (super.noSuchMethod( Invocation.method( @@ -6432,7 +6442,7 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { @override _i20.Future<_i8.Build> rescheduleBuild({ required String? builderName, - required _i37.BuildPushMessage? buildPushMessage, + required _i38.BuildPushMessage? buildPushMessage, required int? rescheduleAttempt, }) => (super.noSuchMethod( @@ -6460,7 +6470,7 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { ) as _i20.Future<_i8.Build>); @override - _i20.Future<_i8.Build> reschedulePresubmitBuildUsingCheckRunEvent(_i39.CheckRunEvent? checkRunEvent) => + _i20.Future<_i8.Build> reschedulePresubmitBuildUsingCheckRunEvent(_i40.CheckRunEvent? checkRunEvent) => (super.noSuchMethod( Invocation.method( #reschedulePresubmitBuildUsingCheckRunEvent, @@ -6490,10 +6500,10 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { @override _i20.Future<_i8.Build> reschedulePostsubmitBuildUsingCheckRunEvent( - _i39.CheckRunEvent? checkRunEvent, { + _i40.CheckRunEvent? checkRunEvent, { required _i32.Commit? commit, required _i33.Task? task, - required _i38.Target? target, + required _i39.Target? target, }) => (super.noSuchMethod( Invocation.method( @@ -6558,9 +6568,9 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { ) as _i20.Future>); @override - _i20.Future>> schedulePostsubmitBuilds({ + _i20.Future>> schedulePostsubmitBuilds({ required _i32.Commit? commit, - required List<_i15.Tuple<_i38.Target, _i33.Task, int>>? toBeScheduled, + required List<_i15.Tuple<_i39.Target, _i33.Task, int>>? toBeScheduled, }) => (super.noSuchMethod( Invocation.method( @@ -6571,14 +6581,14 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { #toBeScheduled: toBeScheduled, }, ), - returnValue: _i20.Future>>.value( - <_i15.Tuple<_i38.Target, _i33.Task, int>>[]), - ) as _i20.Future>>); + returnValue: _i20.Future>>.value( + <_i15.Tuple<_i39.Target, _i33.Task, int>>[]), + ) as _i20.Future>>); @override _i20.Future createPostsubmitCheckRun( _i32.Commit? commit, - _i38.Target? target, + _i39.Target? target, Map? rawUserData, ) => (super.noSuchMethod( @@ -6597,11 +6607,13 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { @override _i20.Future checkRerunBuilder({ required _i32.Commit? commit, - required _i38.Target? target, + required _i39.Target? target, required _i33.Task? task, required _i9.DatastoreService? datastore, + _i15.FirestoreService? firestoreService, Map>? tags, bool? ignoreChecks = false, + _i37.Task? taskDocument, }) => (super.noSuchMethod( Invocation.method( @@ -6612,8 +6624,10 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { #target: target, #task: task, #datastore: datastore, + #firestoreService: firestoreService, #tags: tags, #ignoreChecks: ignoreChecks, + #taskDocument: taskDocument, }, ), returnValue: _i20.Future.value(false), @@ -6623,7 +6637,7 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { /// A class which mocks [ProcessManager]. /// /// See the documentation for Mockito's code generation for more information. -class MockProcessManager extends _i1.Mock implements _i40.ProcessManager { +class MockProcessManager extends _i1.Mock implements _i41.ProcessManager { MockProcessManager() { _i1.throwOnMissingStub(this); } @@ -9354,7 +9368,7 @@ class MockGitHub extends _i1.Mock implements _i13.GitHub { #preview: preview, }, ), - returnValue: _i41.postJsonShim( + returnValue: _i42.postJsonShim( path, statusCode: statusCode, fail: fail, diff --git a/auto_submit/lib/validations/ci_successful.dart b/auto_submit/lib/validations/ci_successful.dart index 27f8c8774..8cccdd597 100644 --- a/auto_submit/lib/validations/ci_successful.dart +++ b/auto_submit/lib/validations/ci_successful.dart @@ -145,12 +145,6 @@ class CiSuccessful extends Validation { // How can name be null but presumed to not be null below when added to failure? final String? name = status.context; - // If the account author is a roller account do not block merge on flutter-gold check. - if (isToEngineRoller(author, slug) && name == 'flutter-gold') { - log.info('Skipping status check for flutter-gold for ${slug.fullName}/$prNumber, pr author: $author.'); - continue; - } - if (status.state != STATUS_SUCCESS) { if (notInAuthorsControl.contains(name) && labelNames.contains(overrideTreeStatusLabel)) { continue; @@ -159,7 +153,10 @@ class CiSuccessful extends Validation { if (status.state == STATUS_FAILURE && !notInAuthorsControl.contains(name)) { failures.add(FailureDetail(name!, status.targetUrl!)); } - if (status.state == STATUS_PENDING && isStale(status.createdAt!) && supportStale(author, slug)) { + if (status.state == STATUS_PENDING && + status.createdAt != null && + isStale(status.createdAt!) && + supportStale(author, slug)) { staleStatuses.add(status); } } diff --git a/auto_submit/test/validations/ci_successful_test.dart b/auto_submit/test/validations/ci_successful_test.dart index e325118b2..1935677b3 100644 --- a/auto_submit/test/validations/ci_successful_test.dart +++ b/auto_submit/test/validations/ci_successful_test.dart @@ -318,7 +318,7 @@ void main() { expect(failures.length, 2); }); - test('Validate flutter-gold is not checked for engine auto roller pull requests.', () { + test('Validate flutter-gold is checked for engine auto roller pull requests.', () { final List contextNodeList = getContextNodeListFromJson(repositoryStatusesWithGoldMock); const bool allSuccess = true; final Author author = Author(login: 'skia-flutter-autoroll'); @@ -331,13 +331,13 @@ void main() { convertContextNodeStatuses(contextNodeList); expect( ciSuccessful.validateStatuses(slug, prNumber, author, labelNames, contextNodeList, failures, allSuccess), - isTrue, + isFalse, ); expect(failures, isEmpty); expect(failures.length, 0); }); - test('Validate flutter-gold is not checked even if failing for engine auto roller pull requests.', () { + test('Validate flutter-gold is checked even if failing for engine auto roller pull requests.', () { final List contextNodeList = getContextNodeListFromJson(repositoryStatusesWithFailedGoldMock); const bool allSuccess = true; final Author author = Author(login: 'skia-flutter-autoroll'); @@ -350,10 +350,10 @@ void main() { convertContextNodeStatuses(contextNodeList); expect( ciSuccessful.validateStatuses(slug, prNumber, author, labelNames, contextNodeList, failures, allSuccess), - isTrue, + isFalse, ); - expect(failures, isEmpty); - expect(failures.length, 0); + expect(failures, isNotEmpty); + expect(failures.length, 1); }); test('Validate flutter-gold is checked for non engine auto roller pull requests.', () {