Skip to content

do not reRequest CheckRun #5142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ievdokdm wants to merge 1 commit into
base: main
Choose a base branch
from
Open

do not reRequest CheckRun #5142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions app_dart/lib/src/service/luci_build_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
);
Comment on lines +469 to +478

@gemini-code-assist gemini-code-assist Bot Aug 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When updating a check run with a conclusion (such as CheckRunConclusion.neutral), GitHub API requires the status to be set to completed. Explicitly passing status: CheckRunStatus.completed avoids potential API validation errors or unexpected behavior.

Suggested change
await _githubChecksUtil.updateCheckRun(
_config,
slug,
dashboardChecks,
conclusion: CheckRunConclusion.neutral,
output: const CheckRunOutput(
title: Config.kDashboardCheckName,
summary: Scheduler.kDashboardChecksDescription,
),
);
await _githubChecksUtil.updateCheckRun(
_config,
slug,
dashboardChecks,
status: CheckRunStatus.completed,
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.
Expand Down
27 changes: 3 additions & 24 deletions app_dart/lib/src/service/scheduler.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
41 changes: 13 additions & 28 deletions app_dart/test/service/luci_build_service/schedule_try_builds_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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),
),
Expand Down Expand Up @@ -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);
Comment on lines 691 to 702

@gemini-code-assist gemini-code-assist Bot Aug 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Update the test verification to match the explicit status: CheckRunStatus.completed parameter added to the updateCheckRun call.

        verify(
          mockGithubChecksUtil.updateCheckRun(
            any,
            RepositorySlug.full('flutter/flutter'),
            checkRunGuard,
            status: CheckRunStatus.completed,
            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,
Expand All @@ -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),
),
Expand Down Expand Up @@ -772,20 +763,14 @@ void main() {
completion([isTarget.hasName('Linux foo')]),
);

verifyNever(
mockCheckRunsService.reRequestCheckRun(
any,
checkRunId: anyNamed('checkRunId'),
),
);

verifyNever(
mockGithubChecksUtil.updateCheckRun(
any,
any,
any,
status: anyNamed('status'),
conclusion: anyNamed('conclusion'),
output: anyNamed('output'),
),
);
},
Expand Down
16 changes: 7 additions & 9 deletions app_dart/test/service/scheduler_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
});
Expand Down
Loading