Skip to content

Commit 49b5564

Browse files
authored
re-run job change status to InProgress (#4983)
Decrease failed count for re-run. Made status inProgress while at least one build is running. Also archived conductor track of a previous task. Fixes: flutter/flutter#183601
1 parent 9c52ac2 commit 49b5564

9 files changed

Lines changed: 76 additions & 62 deletions

File tree

app_dart/lib/src/service/firestore/unified_check_run.dart

Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,18 @@ final class UnifiedCheckRun {
208208
final builds = guard.builds;
209209
final currentStatus = builds[buildName]!;
210210

211+
// If build is failed we increment remain builds and decrement failed.
212+
// If build succeeded re-run is not possible but if some how they manage to
213+
// request re-run we have to only increment remaining builds.
214+
// If build is still in progress re-run is not possible but if some how they
215+
// manage to request re-run we should not touch any counters.
211216
if (currentStatus.isBuildCompleted) {
212217
guard.remainingBuilds += 1;
218+
if (currentStatus.isBuildFailed && guard.failedBuilds > 0) {
219+
guard.failedBuilds -= 1;
220+
}
213221
}
222+
214223
builds[buildName] = TaskStatus.waitingForBackfill;
215224
guard.builds = builds;
216225

@@ -580,54 +589,42 @@ final class UnifiedCheckRun {
580589
}
581590
valid = true;
582591
} else {
583-
// "remaining" should go down if build is succeeded or failed.
584-
// "failed_count" can go up or down depending on:
585-
// attemptNumber > 1 && buildSuccessed: down (-1)
586-
// attemptNumber = 1 && buildFailed: up (+1)
587-
// So if the test existed and either remaining or failed_count is changed;
588-
// the response is valid.
589-
status = state.status;
590-
if (status.isBuildCompleted) {
591-
// Guard against going negative and log enough info so we can debug.
592-
if (remaining == 0) {
593-
throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}';
594-
}
595-
remaining = remaining - 1;
596-
}
597-
598-
if (status.isBuildSuccessed) {
599-
// Only rollback the "failed" counter if this is a successful test run,
600-
// i.e. the test failed, the user requested a rerun, and now it passes.
601-
if (state.attemptNumber > 1) {
602-
log.info(
603-
'$logCrumb: conclusion flipped to positive - assuming test was re-run',
604-
);
605-
if (failed == 0) {
606-
throw '$logCrumb: field "${PresubmitGuard.fieldFailedBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}';
592+
// If build already compleated remaining and failed should not updated.
593+
if (!status.isBuildCompleted) {
594+
// "remaining" should go down if build is succeeded or failed.
595+
// "failed_count" can go up or down depending on:
596+
// attemptNumber > 1 && buildSuccessed: down (-1)
597+
// attemptNumber = 1 && buildFailed: up (+1)
598+
// So if the test existed and either remaining or failed_count is changed;
599+
// the response is valid.
600+
if (state.status.isBuildCompleted) {
601+
// Guard against going negative and log enough info so we can debug.
602+
if (remaining == 0) {
603+
throw '$logCrumb: field "${PresubmitGuard.fieldRemainingBuilds}" is already zero for $transaction / ${presubmitGuardDocument.fields}';
607604
}
608-
failed = failed - 1;
605+
remaining -= 1;
606+
valid = true;
609607
}
610-
valid = true;
611-
}
612608

613-
// Only increment the "failed" counter if the conclusion failed for the first attempt.
614-
if (status.isBuildFailed) {
615-
if (state.attemptNumber == 1) {
609+
if (state.status.isBuildFailed) {
616610
log.info('$logCrumb: test failed');
617-
failed = failed + 1;
611+
failed += 1;
612+
valid = true;
618613
}
614+
status = state.status;
615+
// All checks pass. "valid" is only set to true if there was a change in either the remaining or failed count.
616+
log.info(
617+
'$logCrumb: setting remaining to $remaining, failed to $failed',
618+
);
619+
presubmitGuard.remainingBuilds = remaining;
620+
presubmitGuard.failedBuilds = failed;
621+
presubmitCheck.endTime = state.endTime!;
622+
presubmitCheck.summary = state.summary;
623+
presubmitCheck.buildNumber = state.buildNumber;
624+
} else {
625+
status = state.status;
619626
valid = true;
620627
}
621-
622-
// All checks pass. "valid" is only set to true if there was a change in either the remaining or failed count.
623-
log.info(
624-
'$logCrumb: setting remaining to $remaining, failed to $failed',
625-
);
626-
presubmitGuard.remainingBuilds = remaining;
627-
presubmitGuard.failedBuilds = failed;
628-
presubmitCheck.endTime = state.endTime!;
629-
presubmitCheck.summary = state.summary;
630-
presubmitCheck.buildNumber = state.buildNumber;
631628
}
632629
builds[state.buildName] = status;
633630
presubmitGuard.builds = builds;
File renamed without changes.

conductor/tracks/rerun_failed_ui_20260309/metadata.json renamed to conductor/archive/rerun_failed_ui_20260309/metadata.json

File renamed without changes.

conductor/tracks/rerun_failed_ui_20260309/plan.md renamed to conductor/archive/rerun_failed_ui_20260309/plan.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ Update the backend service interface and the state management logic to support r
2323
- [x] Task: Rename `rerunTask` to `rerunFailedJob` and `rerunFailed` to `rerunAllFailedJobs` in `PresubmitState` and its usages.
2424
- [x] Task: Conductor - User Manual Verification 'Phase 1: Service and State Updates' (Protocol in workflow.md)
2525

26-
## Phase 3: Data Seeding Updates
26+
## Phase 3: Data Seeding Updates [checkpoint: 0011a64]
2727
Update the backend data seeder to provide more realistic dummy data for integration tests.
2828

29-
- [~] Task: Update `_seedPresubmitData` in `dashboard/lib/service/data_seeder.dart` to seed `prCheckRuns` with dummy `pull_request`.
29+
- [x] Task: Update `_seedPresubmitData` in `dashboard/lib/service/data_seeder.dart` to seed `prCheckRuns` with dummy `pull_request`.
3030
- For every `PresubmitGuard`, create a matching `PrCheckRuns` document.
3131
- Include a dummy `PullRequest` object with: `head.sha`, `base.repo`, `base.ref`, `user.login`, `number`, `labels`.
32-
- [ ] Task: Conductor - User Manual Verification 'Phase 3: Data Seeding Updates' (Protocol in workflow.md)
32+
- [x] Task: Conductor - User Manual Verification 'Phase 3: Data Seeding Updates' (Protocol in workflow.md)
3333

3434
## Phase 2: UI Implementation
3535
Update the `PresubmitView` to connect the existing buttons to the new state methods and handle error reporting.
File renamed without changes.

conductor/product.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ Cocoon is the CI coordination and orchestration system for the Flutter project.
1919
* **Tree Status Dashboard:** A Flutter-based web application that provides a visual overview of build health across various commits and branches.
2020
* **Presubmit Check Details:** Backend APIs to retrieve detailed attempt history and status for specific presubmit checks, aiding in debugging and visibility.
2121
* **Presubmit Guard Summaries:** Backend APIs to retrieve summaries of all presubmit checks (Presubmit Guards) of the provided pull request to the dashboard.
22-
* **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.
22+
* **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.
2323
* **Merge Queue Visibility:** APIs for querying and inspecting recent GitHub Merge Queue webhook events to diagnose integration issues.
2424
* **Auto-submit Bot:** Handles automated pull request management, including label-based merges, reverts, and validation checks.
2525
* **GitHub Integration:** Robust handling of GitHub webhooks to sync commits, manage check runs, and report build statuses back to PRs.

conductor/tracks.md

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,3 @@
44

55
- [~] **Track: Build a Merge Queue Dashboard**
66
*Link: [./tracks/merge_queue_dashboard_20260205/](./tracks/merge_queue_dashboard_20260205/)*
7-
8-
---
9-
10-
- [x] **Track: Re-run Event Handling in Presubmit Guard Details**
11-
*Link: [./tracks/rerun_failed_ui_20260309/](./tracks/rerun_failed_ui_20260309/)*

dashboard/test/service/data_seeder_test.dart

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ void main() {
1313
seeder.seed();
1414

1515
final firestore = server.firestore;
16-
final changes = firestore.documents
17-
.where((d) => d.name!.contains('tree_status_change'))
18-
.toList();
19-
expect(changes, isNotEmpty);
16+
expect(
17+
firestore.documents.where((d) => d.name!.contains('tree_status_change')),
18+
isNotEmpty,
19+
);
2020
});
2121

2222
test('DataSeeder seeds suppressed tests', () async {
@@ -25,9 +25,31 @@ void main() {
2525
seeder.seed();
2626

2727
final firestore = server.firestore;
28-
final suppressed = firestore.documents
29-
.where((d) => d.name!.contains('suppressed_tests'))
30-
.toList();
31-
expect(suppressed, isNotEmpty);
28+
expect(
29+
firestore.documents.where((d) => d.name!.contains('suppressed_tests')),
30+
isNotEmpty,
31+
);
32+
});
33+
34+
test('DataSeeder seeds presubmit data', () async {
35+
final server = IntegrationServer();
36+
final seeder = DataSeeder(server);
37+
seeder.seed();
38+
39+
final firestore = server.firestore;
40+
expect(
41+
firestore.documents.where((d) => d.name!.contains('presubmit_guards')),
42+
isNotEmpty,
43+
);
44+
45+
expect(
46+
firestore.documents.where((d) => d.name!.contains('presubmit_checks')),
47+
isNotEmpty,
48+
);
49+
50+
expect(
51+
firestore.documents.where((d) => d.name!.contains('prCheckRuns')),
52+
isNotEmpty,
53+
);
3254
});
3355
}

packages/cocoon_common/lib/guard_status.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,14 @@ enum GuardStatus {
3333
required int remainingBuilds,
3434
required int totalBuilds,
3535
}) {
36-
if (failedBuilds > 0) {
37-
return GuardStatus.failed;
38-
} else if (failedBuilds == 0 && remainingBuilds == 0) {
36+
if (failedBuilds == 0 && remainingBuilds == 0) {
3937
return GuardStatus.succeeded;
4038
} else if (remainingBuilds == totalBuilds) {
4139
return GuardStatus.waitingForBackfill;
42-
} else {
40+
} else if (remainingBuilds > 0) {
4341
return GuardStatus.inProgress;
42+
} else {
43+
return GuardStatus.failed;
4444
}
4545
}
4646
}

0 commit comments

Comments
 (0)