Skip to content

Don't mark dashboard checks as neutral on rerun. #5154

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

Merged
auto-submit merged 3 commits into from
Sep 21, 2026
+2 −151
Merged

Don't mark dashboard checks as neutral on rerun. #5154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e4f408
Don't mark dashboard checks as neutral on rerun.
eyebrowsoffire Sep 21, 2026
5fdd1c2
Fix unit test.
eyebrowsoffire Sep 21, 2026
ba4ba74
Remove rerequest of the Dashboard Checks check run.
eyebrowsoffire Sep 21, 2026
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
37 changes: 0 additions & 37 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 @@ -445,43 +445,6 @@ class LuciBuildService {
);
}

// 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) {
if (isRerun && stage != null) {
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) {
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!,
);
}
} 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,
);
}
}
}

return targets.keys.toList();
}

Expand Down
25 changes: 1 addition & 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,13 @@ $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;
Comment on lines 1711 to 1716

@gemini-code-assist gemini-code-assist Bot Sep 21, 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

To adhere to defensive programming and Dart's null safety best practices, avoid using the null assertion operator (!) repeatedly (e.g., checkRunEvent.checkRun!.checkSuite!.id!). If any of these fields are unexpectedly null (for example, in unit tests or due to API changes), it will cause a runtime crash. Instead, use null-safe navigation (?.) and handle the null case gracefully.

Suggested change
final checkSuiteId = checkRunEvent.checkRun!.checkSuite!.id!;
log.debug(
'$logCrumb: Requested re-run of "$name" for '
'$slug / $checkSuiteId - ignoring',
);
success = true;
final checkSuiteId = checkRunEvent.checkRun?.checkSuite?.id;
if (checkSuiteId == null) {
log.warn('$logCrumb: Requested re-run of "$name" but checkSuiteId was null');
return const ProcessCheckRunResult.success();
}
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
77 changes: 0 additions & 77 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 @@ -627,83 +627,6 @@ void main() {
},
);

test(
'reRequests check run 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 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,
);

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},
engineArtifacts: EngineArtifacts.builtFromSource(
commitSha: pullRequest.head!.sha!,
),
dashboardChecks: checkRunGuard,
stage: CiStage.fusionTests,
),
completion([isTarget.hasName('Linux foo')]),
);

verify(
mockCheckRunsService.reRequestCheckRun(
RepositorySlug.full('flutter/flutter'),
checkRunId: 1234,
),
).called(1);
},
);

test(
'does not reRequest check run for re-run failed checks when failedJobs > 0',
() async {
Expand Down
14 changes: 1 addition & 13 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 resets check run is ignored', () async {
final mockGithubService = MockGithubService();
final mockGithubClient = MockGitHub();
config = FakeConfig(githubService: mockGithubService);
Expand Down Expand Up @@ -1084,18 +1084,6 @@ void main() {
await scheduler.processCheckRun(checkRunEvent),
const ProcessCheckRunResult.success(),
);
verify(
mockGithubChecksUtil.updateCheckRun(
any,
RepositorySlug.full('flutter/cocoon'),
any,
conclusion: CheckRunConclusion.neutral,
output: const CheckRunOutput(
title: Config.kDashboardCheckName,
summary: Scheduler.kDashboardChecksDescription,
),
),
).called(1);
// Verifies no checks were created
verifyNever(mockGithubChecksUtil.createCheckRun(any, any, any, any));
});
Expand Down
Loading