Skip to content

Conversation

@ievdokdm
Copy link
Contributor

@ievdokdm ievdokdm commented Jan 7, 2026

Refactor GitHub Checks Service and Luci Build Service for Unified Check Run Flow

  • Updated GithubChecksService to use the new isTaskFailed method for checking task failure.
  • Removed the deprecated taskFailed method from GithubChecksService.
  • Modified LuciBuildService to support unified check run flow, including changes to method signatures and logic for scheduling builds.
  • Enhanced PresubmitUserData to include optional fields for guard check run ID and stage.
  • Updated JSON serialization for PresubmitUserData to handle new fields.
  • Refactored Scheduler to integrate unified check run logic, including changes to document initialization and task scheduling.
  • Adjusted tests to reflect changes in method signatures and logic, ensuring proper mocking and verification.

Fixes: flutter/flutter#176981
Fixes: flutter/flutter#176982

…ck Run Flow

- Updated `GithubChecksService` to use the new `isTaskFailed` method for checking task failure.
- Removed the deprecated `taskFailed` method from `GithubChecksService`.
- Modified `LuciBuildService` to support unified check run flow, including changes to method signatures and logic for scheduling builds.
- Enhanced `PresubmitUserData` to include optional fields for guard check run ID and stage.
- Updated JSON serialization for `PresubmitUserData` to handle new fields.
- Refactored `Scheduler` to integrate unified check run logic, including changes to document initialization and task scheduling.
- Adjusted tests to reflect changes in method signatures and logic, ensuring proper mocking and verification.
@ievdokdm ievdokdm requested a review from jtmcdole January 7, 2026 23:41
build.summaryMarkdown = (await _luciBuildService.getBuildById(
build.id,
buildMask: bbv2.BuildMask(
// Need to use allFields as there is a bug with fieldMask and summaryMarkdown.
Copy link
Member

Choose a reason for hiding this comment

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

bug with our implementation or somewhere is? issue tracking id?

Comment on lines +140 to +141
if (!rescheduled && config.flags.closeMqGuardAfterPresubmit ||
!rescheduled && userData.guardCheckRunId != null) {
Copy link
Member

Choose a reason for hiding this comment

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

Thoughts on:

final hasGuardCheckRunId = userData.guardCheckRunId != null;

if (!rescheduled && (config.flags.closeMqGuardAfterPresubmit || hasGuardCheckRunId))

and then later:

id: hasGuardCheckRunId ? userData.guardCheckRunId! | userData.checkRunId,
name: hasGuardCheckRunId ? 'Merge Queue Guard' | builderName,

Comment on lines +23 to 30
import '../model/common/presubmit_check_state.dart';
import '../model/common/presubmit_guard_conclusion.dart';
import '../model/firestore/base.dart';
import '../model/firestore/ci_staging.dart';
import '../model/firestore/commit.dart' as fs;
import '../model/firestore/pr_check_runs.dart';
import '../model/firestore/presubmit_guard.dart';
import '../model/firestore/task.dart' as fs;
Copy link
Member

Choose a reason for hiding this comment

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

Unused imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Consolidated Check-Run flow for specific users only Store LUCI Tests execution details updates in Firestore

2 participants