diff --git a/app_dart/lib/src/service/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart index 81bafcfc6..dcb0d89d7 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -615,11 +615,15 @@ final class UnifiedCheckRun { // So if the test existed and either remaining or failed_count is changed; // the response is valid. if (state.status.isComplete) { - // Guard against going negative and log enough info so we can debug. - if (remaining == 0) { - throw '$logCrumb: field "${PresubmitGuard.fieldRemainingJobs}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; + // If remaining is 0 we should not decrement it and we should log + // this fact. + if (remaining > 0) { + remaining -= 1; + } else { + log.error( + '$logCrumb: field "${PresubmitGuard.fieldRemainingJobs}" is already zero for $transaction / ${presubmitGuardDocument.fields}', + ); } - remaining -= 1; valid = true; } diff --git a/app_dart/test/service/firestore/unified_check_run_test.dart b/app_dart/test/service/firestore/unified_check_run_test.dart index c9353b5f5..dfb3d7985 100644 --- a/app_dart/test/service/firestore/unified_check_run_test.dart +++ b/app_dart/test/service/firestore/unified_check_run_test.dart @@ -326,6 +326,51 @@ void main() { expect(checkDoc.buildNumber, 456); expect(checkDoc.buildId, Int64.MAX_VALUE); }); + test('keeps remaining jobs at 0 if it is already 0', () async { + final guardDoc = await firestoreService.getDocument( + 'projects/flutter-dashboard/databases/cocoon/documents/presubmit_guards/${guardId.documentId}', + ); + final guard = PresubmitGuard.fromDocument(guardDoc); + guard.remainingJobs = 0; + await firestoreService.writeViaTransaction( + documentsToWrites([guard], exists: true), + ); + + final state = const PresubmitJobState( + jobName: 'linux', + status: TaskStatus.succeeded, + attemptNumber: 1, + startTime: 2000, + endTime: 3000, + ); + + final result = await UnifiedCheckRun.markConclusion( + firestoreService: firestoreService, + guardId: guardId, + state: state, + ); + + expect(result.result, PresubmitGuardConclusionResult.ok); + expect(result.remaining, 0); + + final updatedGuardDoc = await firestoreService.getDocument( + 'projects/flutter-dashboard/databases/cocoon/documents/presubmit_guards/${guardId.documentId}', + ); + final updatedGuard = PresubmitGuard.fromDocument(updatedGuardDoc); + expect(updatedGuard.remainingJobs, 0); + expect(updatedGuard.jobs['linux'], TaskStatus.succeeded); + + final checkDoc = await PresubmitJob.fromFirestore( + firestoreService, + PresubmitJobId( + slug: slug, + checkRunId: 123, + jobName: 'linux', + attemptNumber: 1, + ), + ); + expect(checkDoc.status, TaskStatus.succeeded); + }); }); group('reInitializeFailedChecks', () { late PresubmitGuardId fusionGuardId;