diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index fe49b651b..53be4e693 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -462,14 +462,28 @@ class LuciBuildService { ); final guard = PresubmitGuard.fromDocument(presubmitGuardDoc); if (guard.failedJobs == 0) { - log.info( - 'Re-requesting dashboard checks id ${dashboardChecks.id} for Guard $guard', - ); - final githubClient = await _config.createGitHubClient(slug: slug); - await githubClient.checks.checkRuns.reRequestCheckRun( - slug, - checkRunId: dashboardChecks.id!, - ); + try { + log.info( + 'Resetting dashboard checks ${dashboardChecks.id} to neutral', + ); + await _githubChecksUtil.updateCheckRun( + _config, + slug, + dashboardChecks, + conclusion: CheckRunConclusion.neutral, + output: const CheckRunOutput( + title: Config.kDashboardCheckName, + summary: Scheduler.kDashboardChecksDescription, + ), + ); + } catch (e, s) { + // We are not going to block on this error. + log.warn( + 'Failed to reset dashboard checks ${dashboardChecks.id} to neutral', + e, + s, + ); + } } } catch (e, s) { // We are not going to block on this error. diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index ab984096d..16577da43 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -1707,36 +1707,15 @@ $stacktrace switch (name) { case Config.kMergeQueueLockName: + case Config.kDashboardCheckName: final checkSuiteId = checkRunEvent.checkRun!.checkSuite!.id!; log.debug( '$logCrumb: Requested re-run of "$name" for ' '$slug / $checkSuiteId - ignoring', ); success = true; - case Config.kDashboardCheckName: - try { - log.info( - 'Resetting dashboard checks ${checkRunEvent.checkRun!.id} to neutral', - ); - await _githubChecksService.githubChecksUtil.updateCheckRun( - _config, - slug, - checkRunEvent.checkRun!.toGithubCheckRun(), - conclusion: CheckRunConclusion.neutral, - output: const CheckRunOutput( - title: Config.kDashboardCheckName, - summary: Scheduler.kDashboardChecksDescription, - ), - ); - success = true; - } catch (e, s) { - // We are not going to block on this error. - log.warn( - 'Failed to reset dashboard checks ${checkRunEvent.checkRun!.id} to neutral', - e, - s, - ); - } + + case Config.kCiYamlCheckName: // The CheckRunEvent.checkRun.pullRequests array is empty for this // event, so we need to find the matching pull request. 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 e8caad7bc..6d4267596 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,13 +6,13 @@ 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/config.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'; @@ -21,6 +21,7 @@ import 'package:cocoon_service/src/service/luci_build_service.dart'; import 'package:cocoon_service/src/service/luci_build_service/build_tags.dart'; import 'package:cocoon_service/src/service/luci_build_service/engine_artifacts.dart'; import 'package:cocoon_service/src/service/luci_build_service/user_data.dart'; +import 'package:cocoon_service/src/service/scheduler.dart'; import 'package:fixnum/fixnum.dart'; import 'package:github/github.dart'; import 'package:mockito/mockito.dart'; @@ -628,7 +629,7 @@ void main() { ); test( - 'reRequests check run for re-run failed checks when failedJobs is 0', + 'resets dashboard checks check run to neutral for re-run failed checks when failedJobs is 0', () async { final pullRequest = generatePullRequest( id: 1, @@ -643,16 +644,8 @@ void main() { name: 'Linux foo', ); - 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), ), @@ -696,16 +689,22 @@ void main() { ); verify( - mockCheckRunsService.reRequestCheckRun( + mockGithubChecksUtil.updateCheckRun( + any, RepositorySlug.full('flutter/flutter'), - checkRunId: 1234, + checkRunGuard, + conclusion: CheckRunConclusion.neutral, + output: const CheckRunOutput( + title: Config.kDashboardCheckName, + summary: Scheduler.kDashboardChecksDescription, + ), ), ).called(1); }, ); test( - 'does not reRequest check run for re-run failed checks when failedJobs > 0', + 'does not reset dashboard checks check run for re-run failed checks when failedJobs > 0', () async { final pullRequest = generatePullRequest( id: 1, @@ -720,16 +719,8 @@ void main() { name: 'Linux foo', ); - 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), ), @@ -772,13 +763,6 @@ void main() { completion([isTarget.hasName('Linux foo')]), ); - verifyNever( - mockCheckRunsService.reRequestCheckRun( - any, - checkRunId: anyNamed('checkRunId'), - ), - ); - verifyNever( mockGithubChecksUtil.updateCheckRun( any, @@ -786,6 +770,7 @@ void main() { any, status: anyNamed('status'), conclusion: anyNamed('conclusion'), + output: anyNamed('output'), ), ); }, diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index 8cfd86a87..5d8d424de 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -1050,7 +1050,7 @@ void main() { verifyNever(mockGithubChecksUtil.createCheckRun(any, any, any, any)); }); - test('rerequested dashboard check resets check run to neutral', () async { + test('rerequested dashboard check is ignored', () async { final mockGithubService = MockGithubService(); final mockGithubClient = MockGitHub(); config = FakeConfig(githubService: mockGithubService); @@ -1084,18 +1084,16 @@ void main() { await scheduler.processCheckRun(checkRunEvent), const ProcessCheckRunResult.success(), ); - verify( + verifyNever( mockGithubChecksUtil.updateCheckRun( any, - RepositorySlug.full('flutter/cocoon'), any, - conclusion: CheckRunConclusion.neutral, - output: const CheckRunOutput( - title: Config.kDashboardCheckName, - summary: Scheduler.kDashboardChecksDescription, - ), + any, + status: anyNamed('status'), + conclusion: anyNamed('conclusion'), + output: anyNamed('output'), ), - ).called(1); + ); // Verifies no checks were created verifyNever(mockGithubChecksUtil.createCheckRun(any, any, any, any)); });