From 256e11e5cbdb909f491e1544f5c04e75733304ad Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 29 Jul 2026 13:57:02 -0700 Subject: [PATCH 1/3] reset failed checkrun back to in progress if all failed jobs re-run --- .../lib/src/service/luci_build_service.dart | 70 +++- app_dart/pubspec.yaml | 2 +- .../schedule_try_builds_test.dart | 143 +++++-- auto_submit/pubspec.yaml | 2 +- packages/cocoon_integration_test/pubspec.yaml | 2 +- packages/cocoon_server_test/lib/mocks.dart | 2 + .../cocoon_server_test/lib/mocks.mocks.dart | 385 +++++++++++++++++- 7 files changed, 533 insertions(+), 73 deletions(-) diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 859615350c..74891dbbe5 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -445,25 +445,57 @@ class LuciBuildService { ); } - // Only set the dashboard check status to `CheckRunStatus.inProgress` for - // the initial run. For Re-run Failed Checks, we need to call GitHub's - // "rerequest check run" API, once it is supported by the `github` package. - final isInitialRun = targets.values.first == 1; - if (isUnifiedCheckRunFlow && dashboardChecks != null && isInitialRun) { - try { - await _githubChecksUtil.updateCheckRun( - _config, - slug, - dashboardChecks, - status: CheckRunStatus.inProgress, - ); - } catch (e, s) { - // We are not going to block on this error. - log.warn( - 'Failed to update dashboard checks for PR# ${pullRequest.number} to in progress', - e, - s, - ); + // Set the dashboard check status to `CheckRunStatus.inProgress` for the + // initial run. For Re-run Failed Checks, if all failed jobs were reset, we + // need to re-request the check run before updating it to in progress. + final isRerun = targets.values.first > 1; + if (isUnifiedCheckRunFlow && dashboardChecks != null && stage != null) { + var shouldUpdateCheckRun = !isRerun; + if (isRerun) { + try { + final presubmitGuardDoc = await _firestore.getDocument( + PresubmitGuard.documentNameFor( + slug: slug, + prNum: pullRequest.number!, + checkRunId: dashboardChecks.id!, + stage: stage, + ), + ); + final guard = PresubmitGuard.fromDocument(presubmitGuardDoc); + if (guard.failedJobs == 0) { + final githubClient = await _config.createGitHubClient(slug: slug); + await githubClient.checks.checkRuns.reRequestCheckRun( + slug, + checkRunId: dashboardChecks.id!, + ); + shouldUpdateCheckRun = true; + } + } catch (e, s) { + // We are not going to block on this error. + log.warn( + 'Failed to re-request dashboard checks for PR# ${pullRequest.number}', + e, + s, + ); + } + } + + if (shouldUpdateCheckRun) { + try { + await _githubChecksUtil.updateCheckRun( + _config, + slug, + dashboardChecks, + status: CheckRunStatus.inProgress, + ); + } catch (e, s) { + // We are not going to block on this error. + log.warn( + 'Failed to update dashboard checks for PR# ${pullRequest.number} to in progress', + e, + s, + ); + } } } diff --git a/app_dart/pubspec.yaml b/app_dart/pubspec.yaml index 36f8c4e727..deea1aea8a 100644 --- a/app_dart/pubspec.yaml +++ b/app_dart/pubspec.yaml @@ -31,7 +31,7 @@ dependencies: gcloud: 0.8.19 genkit: ^0.13.0 genkit_google_genai: ^0.2.5 - github: 9.25.0 + github: 9.26.0 googleapis: 14.0.0 googleapis_auth: 2.0.0 gql: ^1.0.1-alpha+1730759315362 diff --git a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart index b7d2b14032..8bf09fcf2d 100644 --- a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart +++ b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart @@ -6,11 +6,14 @@ import 'package:buildbucket/buildbucket_pb.dart' as bbv2; import 'package:cocoon_common_test/cocoon_common_test.dart'; import 'package:cocoon_integration_test/testing.dart'; import 'package:cocoon_server/logging.dart'; +import 'package:cocoon_server_test/mocks.dart'; import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/src/model/commit_ref.dart'; import 'package:cocoon_service/src/model/firestore/base.dart'; import 'package:cocoon_service/src/model/firestore/pr_check_runs.dart'; +import 'package:cocoon_service/src/model/firestore/presubmit_guard.dart'; import 'package:cocoon_service/src/service/cache_service.dart'; +import 'package:cocoon_service/src/service/firestore.dart'; import 'package:cocoon_service/src/service/flags/dynamic_config.dart'; import 'package:cocoon_service/src/service/flags/ordered_presubmit_flags.dart'; import 'package:cocoon_service/src/service/flags/unified_check_run_flow_flags.dart'; @@ -631,7 +634,7 @@ void main() { }, ); - test('does not update dashboard checks for re-run failed checks', () async { + test('reRequests check run and updates dashboard checks for re-run failed checks when failedJobs is 0', () async { final pullRequest = generatePullRequest( id: 1, repo: 'flutter', @@ -645,9 +648,16 @@ void main() { name: 'Linux foo', ); - // Enable Unified Check Run Flow + final mockGithubClient = MockGitHub(); + final mockChecksService = MockChecksService(); + final mockCheckRunsService = MockCheckRunsService(); + + when(mockGithubClient.checks).thenReturn(mockChecksService); + when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); + luci = LuciBuildService( config: FakeConfig( + githubClient: mockGithubClient, dynamicConfig: DynamicConfig( unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), ), @@ -662,10 +672,25 @@ void main() { final checkRunGuard = generateCheckRun(1234, name: 'Guard'); + final guard = PresubmitGuard( + checkRun: checkRunGuard, + headSha: 'headsha123', + slug: RepositorySlug.full('flutter/flutter'), + prNum: pullRequest.number!, + stage: CiStage.fusionTests, + creationTime: 123456789, + author: 'dash', + remainingJobs: 1, + failedJobs: 0, + ); + await firestore.writeViaTransaction( + documentsToWrites([guard], exists: false), + ); + await expectLater( luci.reScheduleTryBuilds( pullRequest: pullRequest, - targets: {buildTarget: 2}, // Re-run failed (attempt 2) + targets: {buildTarget: 2}, engineArtifacts: EngineArtifacts.builtFromSource( commitSha: pullRequest.head!.sha!, ), @@ -675,43 +700,103 @@ void main() { completion([isTarget.hasName('Linux foo')]), ); - // Should NOT create individual check runs - verifyNever(mockGithubChecksUtil.createCheckRun(any, any, any, any)); + verify( + mockCheckRunsService.reRequestCheckRun( + RepositorySlug.full('flutter/flutter'), + checkRunId: 1234, + ), + ).called(1); - // Should NOT update dashboard checks status - verifyNever( + verify( mockGithubChecksUtil.updateCheckRun( any, any, - any, - status: anyNamed('status'), + checkRunGuard, + status: CheckRunStatus.inProgress, ), + ).called(1); + }); + + test('does not reRequest or update dashboard checks for re-run failed checks when failedJobs > 0', () async { + final pullRequest = generatePullRequest( + id: 1, + repo: 'flutter', + headSha: 'headsha123', ); - final bbv2.ScheduleBuildRequest scheduleBuild; - { - final batchRequest = bbv2.BatchRequest().createEmptyInstance(); - batchRequest.mergeFromProto3Json(pubSub.messages.single); - scheduleBuild = batchRequest.requests.single.scheduleBuild; - } + final buildTarget = generateTarget( + 1, + properties: {'os': 'abc'}, + slug: RepositorySlug.full('flutter/flutter'), + name: 'Linux foo', + ); - final userData = PresubmitUserData.fromBytes( - scheduleBuild.notify.userData, + final mockGithubClient = MockGitHub(); + final mockChecksService = MockChecksService(); + final mockCheckRunsService = MockCheckRunsService(); + + when(mockGithubClient.checks).thenReturn(mockChecksService); + when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); + + luci = LuciBuildService( + config: FakeConfig( + githubClient: mockGithubClient, + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), + ), + cache: CacheService.inMemory(), + buildBucketClient: mockBuildBucketClient, + githubChecksUtil: mockGithubChecksUtil, + pubsub: pubSub, + gerritService: gerritService, + firestore: firestore, ); - expect(userData.guardCheckRunId, 1234); - expect(userData.stage, CiStage.fusionTests); - expect(userData.checkRunId, isNull); - final tags = BuildTags.fromStringPairs(scheduleBuild.tags); - expect( - tags.buildTags.contains(GuardCheckRunIdBuildTag(guardCheckRunId: 1234)), - isTrue, - reason: 'Should have GuardCheckRunIdBuildTag', + final checkRunGuard = generateCheckRun(1234, name: 'Guard'); + + final guard = PresubmitGuard( + checkRun: checkRunGuard, + headSha: 'headsha123', + slug: RepositorySlug.full('flutter/flutter'), + prNum: pullRequest.number!, + stage: CiStage.fusionTests, + creationTime: 123456789, + author: 'dash', + remainingJobs: 1, + failedJobs: 1, ); - expect( - tags.buildTags.contains(CurrentAttemptBuildTag(attemptNumber: 2)), - isTrue, - reason: 'Should have CurrentAttemptBuildTag', + await firestore.writeViaTransaction( + documentsToWrites([guard], exists: false), + ); + + await expectLater( + luci.reScheduleTryBuilds( + pullRequest: pullRequest, + targets: {buildTarget: 2}, + engineArtifacts: EngineArtifacts.builtFromSource( + commitSha: pullRequest.head!.sha!, + ), + dashboardChecks: checkRunGuard, + stage: CiStage.fusionTests, + ), + completion([isTarget.hasName('Linux foo')]), + ); + + verifyNever( + mockCheckRunsService.reRequestCheckRun( + any, + checkRunId: anyNamed('checkRunId'), + ), + ); + + verifyNever( + mockGithubChecksUtil.updateCheckRun( + any, + any, + any, + status: anyNamed('status'), + ), ); }); }); diff --git a/auto_submit/pubspec.yaml b/auto_submit/pubspec.yaml index af6f6dd2d2..08f3a98c49 100644 --- a/auto_submit/pubspec.yaml +++ b/auto_submit/pubspec.yaml @@ -20,7 +20,7 @@ dependencies: path: ../packages/cocoon_server corsac_jwt: 2.0.2 crypto: 3.0.7 - github: 9.25.0 + github: 9.26.0 googleapis: 14.0.0 googleapis_auth: 2.0.0 gql: 1.0.1 diff --git a/packages/cocoon_integration_test/pubspec.yaml b/packages/cocoon_integration_test/pubspec.yaml index 534e7d13b9..dfad293ef9 100644 --- a/packages/cocoon_integration_test/pubspec.yaml +++ b/packages/cocoon_integration_test/pubspec.yaml @@ -22,7 +22,7 @@ dependencies: path: ../../app_dart collection: ^1.19.1 fixnum: 1.1.1 - github: 9.25.0 + github: 9.26.0 googleapis: 14.0.0 googleapis_auth: ^2.0.0 gql: ^1.0.1 diff --git a/packages/cocoon_server_test/lib/mocks.dart b/packages/cocoon_server_test/lib/mocks.dart index 76196c06e0..3b5ca267c7 100644 --- a/packages/cocoon_server_test/lib/mocks.dart +++ b/packages/cocoon_server_test/lib/mocks.dart @@ -7,6 +7,8 @@ MockSpec(), MockSpec(), MockSpec(), + MockSpec(), + MockSpec(), ]) import 'package:github/github.dart'; diff --git a/packages/cocoon_server_test/lib/mocks.mocks.dart b/packages/cocoon_server_test/lib/mocks.mocks.dart index 502ce3bc4f..af927ec52b 100644 --- a/packages/cocoon_server_test/lib/mocks.mocks.dart +++ b/packages/cocoon_server_test/lib/mocks.mocks.dart @@ -258,30 +258,47 @@ class _FakeReleaseNotes_41 extends _i1.SmartFake implements _i2.ReleaseNotes { : super(parent, parentInvocation); } -class _FakeJobCancelResponse_42 extends _i1.SmartFake +class _FakeCheckRunsService_42 extends _i1.SmartFake + implements _i2.CheckRunsService { + _FakeCheckRunsService_42(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCheckSuitesService_43 extends _i1.SmartFake + implements _i2.CheckSuitesService { + _FakeCheckSuitesService_43(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeCheckRun_44 extends _i1.SmartFake implements _i2.CheckRun { + _FakeCheckRun_44(Object parent, Invocation parentInvocation) + : super(parent, parentInvocation); +} + +class _FakeJobCancelResponse_45 extends _i1.SmartFake implements _i5.JobCancelResponse { - _FakeJobCancelResponse_42(Object parent, Invocation parentInvocation) + _FakeJobCancelResponse_45(Object parent, Invocation parentInvocation) : super(parent, parentInvocation); } -class _FakeJob_43 extends _i1.SmartFake implements _i5.Job { - _FakeJob_43(Object parent, Invocation parentInvocation) +class _FakeJob_46 extends _i1.SmartFake implements _i5.Job { + _FakeJob_46(Object parent, Invocation parentInvocation) : super(parent, parentInvocation); } -class _FakeGetQueryResultsResponse_44 extends _i1.SmartFake +class _FakeGetQueryResultsResponse_47 extends _i1.SmartFake implements _i5.GetQueryResultsResponse { - _FakeGetQueryResultsResponse_44(Object parent, Invocation parentInvocation) + _FakeGetQueryResultsResponse_47(Object parent, Invocation parentInvocation) : super(parent, parentInvocation); } -class _FakeJobList_45 extends _i1.SmartFake implements _i5.JobList { - _FakeJobList_45(Object parent, Invocation parentInvocation) +class _FakeJobList_48 extends _i1.SmartFake implements _i5.JobList { + _FakeJobList_48(Object parent, Invocation parentInvocation) : super(parent, parentInvocation); } -class _FakeQueryResponse_46 extends _i1.SmartFake implements _i5.QueryResponse { - _FakeQueryResponse_46(Object parent, Invocation parentInvocation) +class _FakeQueryResponse_49 extends _i1.SmartFake implements _i5.QueryResponse { + _FakeQueryResponse_49(Object parent, Invocation parentInvocation) : super(parent, parentInvocation); } @@ -2908,6 +2925,330 @@ class MockRepositoriesService extends _i1.Mock as _i4.Future<_i2.ReleaseNotes>); } +/// A class which mocks [ChecksService]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockChecksService extends _i1.Mock implements _i2.ChecksService { + @override + _i2.CheckRunsService get checkRuns => + (super.noSuchMethod( + Invocation.getter(#checkRuns), + returnValue: _FakeCheckRunsService_42( + this, + Invocation.getter(#checkRuns), + ), + returnValueForMissingStub: _FakeCheckRunsService_42( + this, + Invocation.getter(#checkRuns), + ), + ) + as _i2.CheckRunsService); + + @override + _i2.CheckSuitesService get checkSuites => + (super.noSuchMethod( + Invocation.getter(#checkSuites), + returnValue: _FakeCheckSuitesService_43( + this, + Invocation.getter(#checkSuites), + ), + returnValueForMissingStub: _FakeCheckSuitesService_43( + this, + Invocation.getter(#checkSuites), + ), + ) + as _i2.CheckSuitesService); + + @override + _i2.GitHub get github => + (super.noSuchMethod( + Invocation.getter(#github), + returnValue: _FakeGitHub_17(this, Invocation.getter(#github)), + returnValueForMissingStub: _FakeGitHub_17( + this, + Invocation.getter(#github), + ), + ) + as _i2.GitHub); +} + +/// A class which mocks [CheckRunsService]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockCheckRunsService extends _i1.Mock implements _i2.CheckRunsService { + @override + _i2.GitHub get github => + (super.noSuchMethod( + Invocation.getter(#github), + returnValue: _FakeGitHub_17(this, Invocation.getter(#github)), + returnValueForMissingStub: _FakeGitHub_17( + this, + Invocation.getter(#github), + ), + ) + as _i2.GitHub); + + @override + _i4.Future<_i2.CheckRun> createCheckRun( + _i2.RepositorySlug? slug, { + required String? name, + required String? headSha, + String? detailsUrl, + String? externalId, + _i2.CheckRunStatus? status = _i2.CheckRunStatus.queued, + DateTime? startedAt, + _i2.CheckRunConclusion? conclusion, + DateTime? completedAt, + _i2.CheckRunOutput? output, + List<_i2.CheckRunAction>? actions, + }) => + (super.noSuchMethod( + Invocation.method( + #createCheckRun, + [slug], + { + #name: name, + #headSha: headSha, + #detailsUrl: detailsUrl, + #externalId: externalId, + #status: status, + #startedAt: startedAt, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + returnValue: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #createCheckRun, + [slug], + { + #name: name, + #headSha: headSha, + #detailsUrl: detailsUrl, + #externalId: externalId, + #status: status, + #startedAt: startedAt, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + ), + ), + returnValueForMissingStub: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #createCheckRun, + [slug], + { + #name: name, + #headSha: headSha, + #detailsUrl: detailsUrl, + #externalId: externalId, + #status: status, + #startedAt: startedAt, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + ), + ), + ) + as _i4.Future<_i2.CheckRun>); + + @override + _i4.Future<_i2.CheckRun> updateCheckRun( + _i2.RepositorySlug? slug, + _i2.CheckRun? checkRunToUpdate, { + String? name, + String? detailsUrl, + String? externalId, + DateTime? startedAt, + _i2.CheckRunStatus? status = _i2.CheckRunStatus.queued, + _i2.CheckRunConclusion? conclusion, + DateTime? completedAt, + _i2.CheckRunOutput? output, + List<_i2.CheckRunAction>? actions, + }) => + (super.noSuchMethod( + Invocation.method( + #updateCheckRun, + [slug, checkRunToUpdate], + { + #name: name, + #detailsUrl: detailsUrl, + #externalId: externalId, + #startedAt: startedAt, + #status: status, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + returnValue: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #updateCheckRun, + [slug, checkRunToUpdate], + { + #name: name, + #detailsUrl: detailsUrl, + #externalId: externalId, + #startedAt: startedAt, + #status: status, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + ), + ), + returnValueForMissingStub: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #updateCheckRun, + [slug, checkRunToUpdate], + { + #name: name, + #detailsUrl: detailsUrl, + #externalId: externalId, + #startedAt: startedAt, + #status: status, + #conclusion: conclusion, + #completedAt: completedAt, + #output: output, + #actions: actions, + }, + ), + ), + ), + ) + as _i4.Future<_i2.CheckRun>); + + @override + _i4.Stream<_i2.CheckRun> listCheckRunsForRef( + _i2.RepositorySlug? slug, { + required String? ref, + String? checkName, + _i2.CheckRunStatus? status, + _i2.CheckRunFilter? filter, + }) => + (super.noSuchMethod( + Invocation.method( + #listCheckRunsForRef, + [slug], + { + #ref: ref, + #checkName: checkName, + #status: status, + #filter: filter, + }, + ), + returnValue: _i4.Stream<_i2.CheckRun>.empty(), + returnValueForMissingStub: _i4.Stream<_i2.CheckRun>.empty(), + ) + as _i4.Stream<_i2.CheckRun>); + + @override + _i4.Stream<_i2.CheckRun> listCheckRunsInSuite( + _i2.RepositorySlug? slug, { + required int? checkSuiteId, + String? checkName, + _i2.CheckRunStatus? status, + _i2.CheckRunFilter? filter, + }) => + (super.noSuchMethod( + Invocation.method( + #listCheckRunsInSuite, + [slug], + { + #checkSuiteId: checkSuiteId, + #checkName: checkName, + #status: status, + #filter: filter, + }, + ), + returnValue: _i4.Stream<_i2.CheckRun>.empty(), + returnValueForMissingStub: _i4.Stream<_i2.CheckRun>.empty(), + ) + as _i4.Stream<_i2.CheckRun>); + + @override + _i4.Future<_i2.CheckRun> getCheckRun( + _i2.RepositorySlug? slug, { + required int? checkRunId, + }) => + (super.noSuchMethod( + Invocation.method(#getCheckRun, [slug], {#checkRunId: checkRunId}), + returnValue: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #getCheckRun, + [slug], + {#checkRunId: checkRunId}, + ), + ), + ), + returnValueForMissingStub: _i4.Future<_i2.CheckRun>.value( + _FakeCheckRun_44( + this, + Invocation.method( + #getCheckRun, + [slug], + {#checkRunId: checkRunId}, + ), + ), + ), + ) + as _i4.Future<_i2.CheckRun>); + + @override + _i4.Stream<_i2.CheckRunAnnotation> listAnnotationsInCheckRun( + _i2.RepositorySlug? slug, { + required _i2.CheckRun? checkRun, + }) => + (super.noSuchMethod( + Invocation.method( + #listAnnotationsInCheckRun, + [slug], + {#checkRun: checkRun}, + ), + returnValue: _i4.Stream<_i2.CheckRunAnnotation>.empty(), + returnValueForMissingStub: + _i4.Stream<_i2.CheckRunAnnotation>.empty(), + ) + as _i4.Stream<_i2.CheckRunAnnotation>); + + @override + _i4.Future reRequestCheckRun( + _i2.RepositorySlug? slug, { + required int? checkRunId, + }) => + (super.noSuchMethod( + Invocation.method( + #reRequestCheckRun, + [slug], + {#checkRunId: checkRunId}, + ), + returnValue: _i4.Future.value(), + returnValueForMissingStub: _i4.Future.value(), + ) + as _i4.Future); +} + /// A class which mocks [JobsResource]. /// /// See the documentation for Mockito's code generation for more information. @@ -2926,7 +3267,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { {#location: location, #$fields: $fields}, ), returnValue: _i4.Future<_i5.JobCancelResponse>.value( - _FakeJobCancelResponse_42( + _FakeJobCancelResponse_45( this, Invocation.method( #cancel, @@ -2936,7 +3277,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), ), returnValueForMissingStub: _i4.Future<_i5.JobCancelResponse>.value( - _FakeJobCancelResponse_42( + _FakeJobCancelResponse_45( this, Invocation.method( #cancel, @@ -2980,7 +3321,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { {#location: location, #$fields: $fields}, ), returnValue: _i4.Future<_i5.Job>.value( - _FakeJob_43( + _FakeJob_46( this, Invocation.method( #get, @@ -2990,7 +3331,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), ), returnValueForMissingStub: _i4.Future<_i5.Job>.value( - _FakeJob_43( + _FakeJob_46( this, Invocation.method( #get, @@ -3030,7 +3371,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { }, ), returnValue: _i4.Future<_i5.GetQueryResultsResponse>.value( - _FakeGetQueryResultsResponse_44( + _FakeGetQueryResultsResponse_47( this, Invocation.method( #getQueryResults, @@ -3050,7 +3391,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), returnValueForMissingStub: _i4.Future<_i5.GetQueryResultsResponse>.value( - _FakeGetQueryResultsResponse_44( + _FakeGetQueryResultsResponse_47( this, Invocation.method( #getQueryResults, @@ -3090,7 +3431,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { }, ), returnValue: _i4.Future<_i5.Job>.value( - _FakeJob_43( + _FakeJob_46( this, Invocation.method( #insert, @@ -3104,7 +3445,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), ), returnValueForMissingStub: _i4.Future<_i5.Job>.value( - _FakeJob_43( + _FakeJob_46( this, Invocation.method( #insert, @@ -3150,7 +3491,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { }, ), returnValue: _i4.Future<_i5.JobList>.value( - _FakeJobList_45( + _FakeJobList_48( this, Invocation.method( #list, @@ -3170,7 +3511,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), ), returnValueForMissingStub: _i4.Future<_i5.JobList>.value( - _FakeJobList_45( + _FakeJobList_48( this, Invocation.method( #list, @@ -3205,7 +3546,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { {#$fields: $fields}, ), returnValue: _i4.Future<_i5.QueryResponse>.value( - _FakeQueryResponse_46( + _FakeQueryResponse_49( this, Invocation.method( #query, @@ -3215,7 +3556,7 @@ class MockJobsResource extends _i1.Mock implements _i5.JobsResource { ), ), returnValueForMissingStub: _i4.Future<_i5.QueryResponse>.value( - _FakeQueryResponse_46( + _FakeQueryResponse_49( this, Invocation.method( #query, From 7834dba21527bbc968bef584fed26e7a02cab63c Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 29 Jul 2026 13:57:21 -0700 Subject: [PATCH 2/3] format --- .../schedule_try_builds_test.dart | 290 +++++++++--------- 1 file changed, 148 insertions(+), 142 deletions(-) diff --git a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart index 8bf09fcf2d..df94287b3a 100644 --- a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart +++ b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart @@ -634,171 +634,177 @@ void main() { }, ); - test('reRequests check run and updates dashboard checks for re-run failed checks when failedJobs is 0', () async { - final pullRequest = generatePullRequest( - id: 1, - repo: 'flutter', - headSha: 'headsha123', - ); + test( + 'reRequests check run and updates dashboard checks for re-run failed checks when failedJobs is 0', + () async { + final pullRequest = generatePullRequest( + id: 1, + repo: 'flutter', + headSha: 'headsha123', + ); - final buildTarget = generateTarget( - 1, - properties: {'os': 'abc'}, - slug: RepositorySlug.full('flutter/flutter'), - name: 'Linux foo', - ); + final buildTarget = generateTarget( + 1, + properties: {'os': 'abc'}, + slug: RepositorySlug.full('flutter/flutter'), + name: 'Linux foo', + ); - final mockGithubClient = MockGitHub(); - final mockChecksService = MockChecksService(); - final mockCheckRunsService = MockCheckRunsService(); + final mockGithubClient = MockGitHub(); + final mockChecksService = MockChecksService(); + final mockCheckRunsService = MockCheckRunsService(); - when(mockGithubClient.checks).thenReturn(mockChecksService); - when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); + when(mockGithubClient.checks).thenReturn(mockChecksService); + when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); - luci = LuciBuildService( - config: FakeConfig( - githubClient: mockGithubClient, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + luci = LuciBuildService( + config: FakeConfig( + githubClient: mockGithubClient, + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), ), - ), - cache: CacheService.inMemory(), - buildBucketClient: mockBuildBucketClient, - githubChecksUtil: mockGithubChecksUtil, - pubsub: pubSub, - gerritService: gerritService, - firestore: firestore, - ); + cache: CacheService.inMemory(), + buildBucketClient: mockBuildBucketClient, + githubChecksUtil: mockGithubChecksUtil, + pubsub: pubSub, + gerritService: gerritService, + firestore: firestore, + ); - final checkRunGuard = generateCheckRun(1234, name: 'Guard'); + final checkRunGuard = generateCheckRun(1234, name: 'Guard'); - final guard = PresubmitGuard( - checkRun: checkRunGuard, - headSha: 'headsha123', - slug: RepositorySlug.full('flutter/flutter'), - prNum: pullRequest.number!, - stage: CiStage.fusionTests, - creationTime: 123456789, - author: 'dash', - remainingJobs: 1, - failedJobs: 0, - ); - await firestore.writeViaTransaction( - documentsToWrites([guard], exists: false), - ); + final guard = PresubmitGuard( + checkRun: checkRunGuard, + headSha: 'headsha123', + slug: RepositorySlug.full('flutter/flutter'), + prNum: pullRequest.number!, + stage: CiStage.fusionTests, + creationTime: 123456789, + author: 'dash', + remainingJobs: 1, + failedJobs: 0, + ); + await firestore.writeViaTransaction( + documentsToWrites([guard], exists: false), + ); - await expectLater( - luci.reScheduleTryBuilds( - pullRequest: pullRequest, - targets: {buildTarget: 2}, - engineArtifacts: EngineArtifacts.builtFromSource( - commitSha: pullRequest.head!.sha!, + await expectLater( + luci.reScheduleTryBuilds( + pullRequest: pullRequest, + targets: {buildTarget: 2}, + engineArtifacts: EngineArtifacts.builtFromSource( + commitSha: pullRequest.head!.sha!, + ), + dashboardChecks: checkRunGuard, + stage: CiStage.fusionTests, ), - dashboardChecks: checkRunGuard, - stage: CiStage.fusionTests, - ), - completion([isTarget.hasName('Linux foo')]), - ); + completion([isTarget.hasName('Linux foo')]), + ); - verify( - mockCheckRunsService.reRequestCheckRun( - RepositorySlug.full('flutter/flutter'), - checkRunId: 1234, - ), - ).called(1); + verify( + mockCheckRunsService.reRequestCheckRun( + RepositorySlug.full('flutter/flutter'), + checkRunId: 1234, + ), + ).called(1); - verify( - mockGithubChecksUtil.updateCheckRun( - any, - any, - checkRunGuard, - status: CheckRunStatus.inProgress, - ), - ).called(1); - }); + verify( + mockGithubChecksUtil.updateCheckRun( + any, + any, + checkRunGuard, + status: CheckRunStatus.inProgress, + ), + ).called(1); + }, + ); - test('does not reRequest or update dashboard checks for re-run failed checks when failedJobs > 0', () async { - final pullRequest = generatePullRequest( - id: 1, - repo: 'flutter', - headSha: 'headsha123', - ); + test( + 'does not reRequest or update dashboard checks for re-run failed checks when failedJobs > 0', + () async { + final pullRequest = generatePullRequest( + id: 1, + repo: 'flutter', + headSha: 'headsha123', + ); - final buildTarget = generateTarget( - 1, - properties: {'os': 'abc'}, - slug: RepositorySlug.full('flutter/flutter'), - name: 'Linux foo', - ); + final buildTarget = generateTarget( + 1, + properties: {'os': 'abc'}, + slug: RepositorySlug.full('flutter/flutter'), + name: 'Linux foo', + ); - final mockGithubClient = MockGitHub(); - final mockChecksService = MockChecksService(); - final mockCheckRunsService = MockCheckRunsService(); + final mockGithubClient = MockGitHub(); + final mockChecksService = MockChecksService(); + final mockCheckRunsService = MockCheckRunsService(); - when(mockGithubClient.checks).thenReturn(mockChecksService); - when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); + when(mockGithubClient.checks).thenReturn(mockChecksService); + when(mockChecksService.checkRuns).thenReturn(mockCheckRunsService); - luci = LuciBuildService( - config: FakeConfig( - githubClient: mockGithubClient, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + luci = LuciBuildService( + config: FakeConfig( + githubClient: mockGithubClient, + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), ), - ), - cache: CacheService.inMemory(), - buildBucketClient: mockBuildBucketClient, - githubChecksUtil: mockGithubChecksUtil, - pubsub: pubSub, - gerritService: gerritService, - firestore: firestore, - ); + cache: CacheService.inMemory(), + buildBucketClient: mockBuildBucketClient, + githubChecksUtil: mockGithubChecksUtil, + pubsub: pubSub, + gerritService: gerritService, + firestore: firestore, + ); - final checkRunGuard = generateCheckRun(1234, name: 'Guard'); + final checkRunGuard = generateCheckRun(1234, name: 'Guard'); - final guard = PresubmitGuard( - checkRun: checkRunGuard, - headSha: 'headsha123', - slug: RepositorySlug.full('flutter/flutter'), - prNum: pullRequest.number!, - stage: CiStage.fusionTests, - creationTime: 123456789, - author: 'dash', - remainingJobs: 1, - failedJobs: 1, - ); - await firestore.writeViaTransaction( - documentsToWrites([guard], exists: false), - ); + final guard = PresubmitGuard( + checkRun: checkRunGuard, + headSha: 'headsha123', + slug: RepositorySlug.full('flutter/flutter'), + prNum: pullRequest.number!, + stage: CiStage.fusionTests, + creationTime: 123456789, + author: 'dash', + remainingJobs: 1, + failedJobs: 1, + ); + await firestore.writeViaTransaction( + documentsToWrites([guard], exists: false), + ); - await expectLater( - luci.reScheduleTryBuilds( - pullRequest: pullRequest, - targets: {buildTarget: 2}, - engineArtifacts: EngineArtifacts.builtFromSource( - commitSha: pullRequest.head!.sha!, + await expectLater( + luci.reScheduleTryBuilds( + pullRequest: pullRequest, + targets: {buildTarget: 2}, + engineArtifacts: EngineArtifacts.builtFromSource( + commitSha: pullRequest.head!.sha!, + ), + dashboardChecks: checkRunGuard, + stage: CiStage.fusionTests, ), - dashboardChecks: checkRunGuard, - stage: CiStage.fusionTests, - ), - completion([isTarget.hasName('Linux foo')]), - ); + completion([isTarget.hasName('Linux foo')]), + ); - verifyNever( - mockCheckRunsService.reRequestCheckRun( - any, - checkRunId: anyNamed('checkRunId'), - ), - ); + verifyNever( + mockCheckRunsService.reRequestCheckRun( + any, + checkRunId: anyNamed('checkRunId'), + ), + ); - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - ), - ); - }); + verifyNever( + mockGithubChecksUtil.updateCheckRun( + any, + any, + any, + status: anyNamed('status'), + ), + ); + }, + ); }); group('Ordered Presubmit', () { From 36e3ebaa819a067e62814f946481a2e8f566823a Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Wed, 29 Jul 2026 14:05:47 -0700 Subject: [PATCH 3/3] fix ai review --- app_dart/lib/src/service/luci_build_service.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 74891dbbe5..65dd367e52 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -449,9 +449,9 @@ class LuciBuildService { // initial run. For Re-run Failed Checks, if all failed jobs were reset, we // need to re-request the check run before updating it to in progress. final isRerun = targets.values.first > 1; - if (isUnifiedCheckRunFlow && dashboardChecks != null && stage != null) { + if (isUnifiedCheckRunFlow && dashboardChecks != null) { var shouldUpdateCheckRun = !isRerun; - if (isRerun) { + if (isRerun && stage != null) { try { final presubmitGuardDoc = await _firestore.getDocument( PresubmitGuard.documentNameFor(