diff --git a/app_dart/lib/src/service/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart index bcd7b0b2a3..99aae7c675 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -208,9 +208,18 @@ final class UnifiedCheckRun { final builds = guard.builds; final currentStatus = builds[buildName]!; + // If build is failed we increment remain builds and decrement failed. + // If build succeeded re-run is not possible but if some how they manage to + // request re-run we have to only increment remaining builds. + // If build is still in progress re-run is not possible but if some how they + // manage to request re-run we should not touch any counters. if (currentStatus.isBuildCompleted) { guard.remainingBuilds += 1; + if (currentStatus.isBuildFailed && guard.failedBuilds > 0) { + guard.failedBuilds -= 1; + } } + builds[buildName] = TaskStatus.waitingForBackfill; guard.builds = builds; @@ -580,54 +589,42 @@ final class UnifiedCheckRun { } valid = true; } else { - // "remaining" should go down if build is succeeded or failed. - // "failed_count" can go up or down depending on: - // attemptNumber > 1 && buildSuccessed: down (-1) - // attemptNumber = 1 && buildFailed: up (+1) - // So if the test existed and either remaining or failed_count is changed; - // the response is valid. - status = state.status; - if (status.isBuildCompleted) { - // Guard against going negative and log enough info so we can debug. - if (remaining == 0) { - throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; - } - remaining = remaining - 1; - } - - if (status.isBuildSuccessed) { - // Only rollback the "failed" counter if this is a successful test run, - // i.e. the test failed, the user requested a rerun, and now it passes. - if (state.attemptNumber > 1) { - log.info( - '$logCrumb: conclusion flipped to positive - assuming test was re-run', - ); - if (failed == 0) { - throw '$logCrumb: field "${PresubmitGuard.fieldFailedBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; + // If build already compleated remaining and failed should not updated. + if (!status.isBuildCompleted) { + // "remaining" should go down if build is succeeded or failed. + // "failed_count" can go up or down depending on: + // attemptNumber > 1 && buildSuccessed: down (-1) + // attemptNumber = 1 && buildFailed: up (+1) + // So if the test existed and either remaining or failed_count is changed; + // the response is valid. + if (state.status.isBuildCompleted) { + // Guard against going negative and log enough info so we can debug. + if (remaining == 0) { + throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}'; } - failed = failed - 1; + remaining -= 1; + valid = true; } - valid = true; - } - // Only increment the "failed" counter if the conclusion failed for the first attempt. - if (status.isBuildFailed) { - if (state.attemptNumber == 1) { + if (state.status.isBuildFailed) { log.info('$logCrumb: test failed'); - failed = failed + 1; + failed += 1; + valid = true; } + status = state.status; + // All checks pass. "valid" is only set to true if there was a change in either the remaining or failed count. + log.info( + '$logCrumb: setting remaining to $remaining, failed to $failed', + ); + presubmitGuard.remainingBuilds = remaining; + presubmitGuard.failedBuilds = failed; + presubmitCheck.endTime = state.endTime!; + presubmitCheck.summary = state.summary; + presubmitCheck.buildNumber = state.buildNumber; + } else { + status = state.status; valid = true; } - - // All checks pass. "valid" is only set to true if there was a change in either the remaining or failed count. - log.info( - '$logCrumb: setting remaining to $remaining, failed to $failed', - ); - presubmitGuard.remainingBuilds = remaining; - presubmitGuard.failedBuilds = failed; - presubmitCheck.endTime = state.endTime!; - presubmitCheck.summary = state.summary; - presubmitCheck.buildNumber = state.buildNumber; } builds[state.buildName] = status; presubmitGuard.builds = builds; diff --git a/conductor/tracks/rerun_failed_ui_20260309/index.md b/conductor/archive/rerun_failed_ui_20260309/index.md similarity index 100% rename from conductor/tracks/rerun_failed_ui_20260309/index.md rename to conductor/archive/rerun_failed_ui_20260309/index.md diff --git a/conductor/tracks/rerun_failed_ui_20260309/metadata.json b/conductor/archive/rerun_failed_ui_20260309/metadata.json similarity index 100% rename from conductor/tracks/rerun_failed_ui_20260309/metadata.json rename to conductor/archive/rerun_failed_ui_20260309/metadata.json diff --git a/conductor/tracks/rerun_failed_ui_20260309/plan.md b/conductor/archive/rerun_failed_ui_20260309/plan.md similarity index 94% rename from conductor/tracks/rerun_failed_ui_20260309/plan.md rename to conductor/archive/rerun_failed_ui_20260309/plan.md index a7eb41b662..96e50fbab5 100644 --- a/conductor/tracks/rerun_failed_ui_20260309/plan.md +++ b/conductor/archive/rerun_failed_ui_20260309/plan.md @@ -23,13 +23,13 @@ Update the backend service interface and the state management logic to support r - [x] Task: Rename `rerunTask` to `rerunFailedJob` and `rerunFailed` to `rerunAllFailedJobs` in `PresubmitState` and its usages. - [x] Task: Conductor - User Manual Verification 'Phase 1: Service and State Updates' (Protocol in workflow.md) -## Phase 3: Data Seeding Updates +## Phase 3: Data Seeding Updates [checkpoint: 0011a64] Update the backend data seeder to provide more realistic dummy data for integration tests. -- [~] Task: Update `_seedPresubmitData` in `dashboard/lib/service/data_seeder.dart` to seed `prCheckRuns` with dummy `pull_request`. +- [x] Task: Update `_seedPresubmitData` in `dashboard/lib/service/data_seeder.dart` to seed `prCheckRuns` with dummy `pull_request`. - For every `PresubmitGuard`, create a matching `PrCheckRuns` document. - Include a dummy `PullRequest` object with: `head.sha`, `base.repo`, `base.ref`, `user.login`, `number`, `labels`. -- [ ] Task: Conductor - User Manual Verification 'Phase 3: Data Seeding Updates' (Protocol in workflow.md) +- [x] Task: Conductor - User Manual Verification 'Phase 3: Data Seeding Updates' (Protocol in workflow.md) ## Phase 2: UI Implementation Update the `PresubmitView` to connect the existing buttons to the new state methods and handle error reporting. diff --git a/conductor/tracks/rerun_failed_ui_20260309/spec.md b/conductor/archive/rerun_failed_ui_20260309/spec.md similarity index 100% rename from conductor/tracks/rerun_failed_ui_20260309/spec.md rename to conductor/archive/rerun_failed_ui_20260309/spec.md diff --git a/conductor/product.md b/conductor/product.md index a7be7308bd..ce269c3051 100644 --- a/conductor/product.md +++ b/conductor/product.md @@ -19,7 +19,7 @@ Cocoon is the CI coordination and orchestration system for the Flutter project. * **Tree Status Dashboard:** A Flutter-based web application that provides a visual overview of build health across various commits and branches. * **Presubmit Check Details:** Backend APIs to retrieve detailed attempt history and status for specific presubmit checks, aiding in debugging and visibility. * **Presubmit Guard Summaries:** Backend APIs to retrieve summaries of all presubmit checks (Presubmit Guards) of the provided pull request to the dashboard. -* **Presubmit Guard Details:** Displays detailed information and CI check statuses for a specific presubmit check (Presubmit Guard), including the commit SHA in the view header for precise version tracking. +* **Presubmit Guard Details:** Displays detailed information and CI check statuses for a specific presubmit check (Presubmit Guard), and allows authenticated users to trigger re-runs for individual jobs or all failed jobs directly from the dashboard. * **Merge Queue Visibility:** APIs for querying and inspecting recent GitHub Merge Queue webhook events to diagnose integration issues. * **Auto-submit Bot:** Handles automated pull request management, including label-based merges, reverts, and validation checks. * **GitHub Integration:** Robust handling of GitHub webhooks to sync commits, manage check runs, and report build statuses back to PRs. diff --git a/conductor/tracks.md b/conductor/tracks.md index 2f13ad003c..43048451de 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -4,8 +4,3 @@ - [~] **Track: Build a Merge Queue Dashboard** *Link: [./tracks/merge_queue_dashboard_20260205/](./tracks/merge_queue_dashboard_20260205/)* - ---- - -- [x] **Track: Re-run Event Handling in Presubmit Guard Details** -*Link: [./tracks/rerun_failed_ui_20260309/](./tracks/rerun_failed_ui_20260309/)* diff --git a/dashboard/test/service/data_seeder_test.dart b/dashboard/test/service/data_seeder_test.dart index 0769c93d88..959f73a0d1 100644 --- a/dashboard/test/service/data_seeder_test.dart +++ b/dashboard/test/service/data_seeder_test.dart @@ -13,10 +13,10 @@ void main() { seeder.seed(); final firestore = server.firestore; - final changes = firestore.documents - .where((d) => d.name!.contains('tree_status_change')) - .toList(); - expect(changes, isNotEmpty); + expect( + firestore.documents.where((d) => d.name!.contains('tree_status_change')), + isNotEmpty, + ); }); test('DataSeeder seeds suppressed tests', () async { @@ -25,9 +25,31 @@ void main() { seeder.seed(); final firestore = server.firestore; - final suppressed = firestore.documents - .where((d) => d.name!.contains('suppressed_tests')) - .toList(); - expect(suppressed, isNotEmpty); + expect( + firestore.documents.where((d) => d.name!.contains('suppressed_tests')), + isNotEmpty, + ); + }); + + test('DataSeeder seeds presubmit data', () async { + final server = IntegrationServer(); + final seeder = DataSeeder(server); + seeder.seed(); + + final firestore = server.firestore; + expect( + firestore.documents.where((d) => d.name!.contains('presubmit_guards')), + isNotEmpty, + ); + + expect( + firestore.documents.where((d) => d.name!.contains('presubmit_checks')), + isNotEmpty, + ); + + expect( + firestore.documents.where((d) => d.name!.contains('prCheckRuns')), + isNotEmpty, + ); }); } diff --git a/packages/cocoon_common/lib/guard_status.dart b/packages/cocoon_common/lib/guard_status.dart index af3998c5e5..bef6aa1b18 100644 --- a/packages/cocoon_common/lib/guard_status.dart +++ b/packages/cocoon_common/lib/guard_status.dart @@ -33,14 +33,14 @@ enum GuardStatus { required int remainingBuilds, required int totalBuilds, }) { - if (failedBuilds > 0) { - return GuardStatus.failed; - } else if (failedBuilds == 0 && remainingBuilds == 0) { + if (failedBuilds == 0 && remainingBuilds == 0) { return GuardStatus.succeeded; } else if (remainingBuilds == totalBuilds) { return GuardStatus.waitingForBackfill; - } else { + } else if (remainingBuilds > 0) { return GuardStatus.inProgress; + } else { + return GuardStatus.failed; } } }