Skip to content

188110 failure Dashboard Checks as soon as 1 test fails#5095

Open
ievdokdm wants to merge 1 commit into
flutter:mainfrom
ievdokdm:188110-fail
Open

188110 failure Dashboard Checks as soon as 1 test fails#5095
ievdokdm wants to merge 1 commit into
flutter:mainfrom
ievdokdm:188110-fail

Conversation

@ievdokdm

Copy link
Copy Markdown
Contributor

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces PresubmitGuardState to track the previous and current states (remaining and failed jobs) of a CI stage, updating CiStaging, UnifiedCheckRun, and Scheduler accordingly. This allows unified check runs to fail immediately on the first failed job. Additionally, GuardStatus.calculate is updated to return a failed status immediately if there are any failed builds. The review feedback points out a critical correctness issue in scheduler.dart where the merge queue guard could be prematurely unlocked on successful jobs in unified check runs because the pending check is skipped even when no failures have occurred yet. A code suggestion is provided to ensure we only skip waiting if there is an active failure.

Comment thread app_dart/lib/src/service/scheduler.dart Outdated
@ievdokdm

ievdokdm commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the presubmit job tracking by replacing PresubmitCompletedJob with PresubmitJobState and introducing PresubmitGuardState to track previous and current job states. It also updates the scheduler to process job status updates through cleaner, flow-specific helper methods and modifies GuardStatus.calculate to fail immediately when any build fails. The review feedback highlights two critical issues: a potential null-pointer exception in scheduler.dart when accessing conclusion.checkRunGuard during an internal error, and an incorrect method invocation name in the generated mock file mocks.mocks.dart.

Comment thread app_dart/lib/src/service/scheduler.dart
Comment thread packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart
@ievdokdm ievdokdm added CICD Run CI/CD and removed CICD Run CI/CD labels Jul 1, 2026
Comment thread app_dart/lib/src/model/common/presubmit_job_state.dart Outdated
Comment thread app_dart/lib/src/model/common/presubmit_job_state.dart Outdated
@ievdokdm ievdokdm added autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD and removed CICD Run CI/CD labels Jul 7, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Jul 7, 2026
@auto-submit

auto-submit Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/cocoon/5095, because This PR has not met approval requirements for merging. Changes were requested by {eyebrowsoffire}, please make the needed changes and resubmit this PR.
The PR author is a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a member of flutter-hackers before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@ievdokdm ievdokdm requested a review from eyebrowsoffire July 7, 2026 19:48
@ievdokdm ievdokdm force-pushed the 188110-fail branch 2 times, most recently from 9f53437 to 4ceb509 Compare July 9, 2026 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show some indication of failure as soon as 1 test fails for Dashboard Checks

2 participants