Delete legacy flow for closeMqGuardAfterPresubmit == false (#5101)
Deleting legacy `closeMqGuardAfterPresubmit = false` flow as it obsolete
**Context:**
We have 2 different processing flows based on whether `processJobStatusUpdate` config variable is `true` or `false`:
1. **Legacy**. if `closeMqGuardAfterPresubmit` flag is set to `false` then flow is the following:
```mermaid
sequenceDiagram
LUCI->>Cocoon: Build complete<br/>notification
Cocoon->>Github: Update Check
Github->>Cocoon: Check complete<br/>notification
Note over Cocoon: Check complete processing
Cocoon->>Github: If all success<br/>unlock merge queue
```
For **Check complete processing** `processJobStatusUpdate()` called with `PresubmitJobState` constructed from check using `PresubmitJobState.fromCheckRun()`
2. **Current**. if `closeMqGuardAfterPresubmit` flag is set to `true` or for **Unified Check Run** flow :
```mermaid
sequenceDiagram
LUCI->>Cocoon: Build complete<br/>notification
Note over Cocoon: Build complete processing
Cocoon->>Github: Update Check<br/>or Job in firestore
Cocoon->>Github: If all success<br/>unlock merge queue
Github->>Cocoon: Check complete<br/>notification
Note over Cocoon: Ignoring
```diff --git a/app_dart/lib/src/model/common/presubmit_completed_check.dart b/app_dart/lib/src/model/common/presubmit_completed_check.dart
index 6aba3c4..6de08aa 100644
--- a/app_dart/lib/src/model/common/presubmit_completed_check.dart
+++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart
@@ -64,30 +64,6 @@
this.buildId,
});
- /// Creates a [PresubmitCompletedJob] from a GitHub [CheckRun].
- factory PresubmitCompletedJob.fromCheckRun(
- cocoon_checks.CheckRun checkRun,
- RepositorySlug slug,
- ) {
- return PresubmitCompletedJob(
- name: checkRun.name!,
- sha: checkRun.headSha!,
- slug: slug,
- status: ChecksExtension.fromConclusion(checkRun.conclusion),
- isMergeGroup: _isMergeGroup(checkRun.checkSuite?.headBranch),
- checkRunId: checkRun.id!,
- checkSuiteId: checkRun.checkSuite?.id,
- headBranch: checkRun.checkSuite?.headBranch,
- isUnifiedCheckRun: false,
- // CheckRun model doesn't have time/summary fields currently
- startTime: null,
- endTime: null,
- summary: null,
- buildNumber: null,
- buildId: null,
- );
- }
-
/// Creates a [PresubmitCompletedJob] from a BuildBucket [Build].
factory PresubmitCompletedJob.fromBuild(
Build build,
diff --git a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
index 5878d32..8a96afc 100644
--- a/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
+++ b/app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
@@ -165,16 +165,12 @@
);
}
if (!rescheduled) {
- // Process to the check-run status in the merge queue document during
- // the LUCI callback.
- if (config.flags.closeMqGuardAfterPresubmit || isUnifiedCheckRun) {
- final check = PresubmitCompletedJob.fromBuild(
- build,
- userData,
- status: override == .neutral ? .neutral : null,
- );
- await _scheduler.processCheckRunCompleted(check);
- }
+ final check = PresubmitCompletedJob.fromBuild(
+ build,
+ userData,
+ status: override == .neutral ? .neutral : null,
+ );
+ await _scheduler.processCheckRunCompleted(check);
}
return Response.emptyOk;
diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart
index ced9831..6a712a3 100644
--- a/app_dart/lib/src/service/scheduler.dart
+++ b/app_dart/lib/src/service/scheduler.dart
@@ -1625,14 +1625,7 @@
) async {
switch (checkRunEvent.action) {
case 'completed':
- if (!_config.flags.closeMqGuardAfterPresubmit) {
- await processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- checkRunEvent.checkRun!,
- checkRunEvent.repository!.slug(),
- ),
- );
- }
+ // Processing of completed check runs is moved to PresubmitLuciSubscription.
break;
case 'rerequested':
return await _reRun(checkRunEvent);
diff --git a/app_dart/test/model/common/presubmit_completed_check_test.dart b/app_dart/test/model/common/presubmit_completed_check_test.dart
index 79f2ca5..d2fdc37 100644
--- a/app_dart/test/model/common/presubmit_completed_check_test.dart
+++ b/app_dart/test/model/common/presubmit_completed_check_test.dart
@@ -8,7 +8,6 @@
import 'package:cocoon_service/src/model/commit_ref.dart';
import 'package:cocoon_service/src/model/common/presubmit_completed_check.dart';
import 'package:cocoon_service/src/model/firestore/base.dart';
-import 'package:cocoon_service/src/model/github/checks.dart' as cocoon_checks;
import 'package:cocoon_service/src/service/config.dart';
import 'package:cocoon_service/src/service/luci_build_service/user_data.dart';
import 'package:fixnum/fixnum.dart';
@@ -22,28 +21,6 @@
const sha = 'abc';
final slug = RepositorySlug('flutter', 'flutter');
- test('fromCheckRun creates correct instance', () {
- final checkRun = const cocoon_checks.CheckRun(
- id: 1,
- name: 'test_check',
- headSha: sha,
- conclusion: 'success',
- );
-
- final check = PresubmitCompletedJob.fromCheckRun(checkRun, slug);
-
- expect(check.name, 'test_check');
- expect(check.sha, sha);
- expect(check.slug, slug);
- expect(check.status, TaskStatus.succeeded);
- expect(check.isMergeGroup, false);
- expect(check.checkRunId, 1);
- expect(check.checkSuiteId, null);
- expect(check.headBranch, null);
- expect(check.isUnifiedCheckRun, false);
- expect(check.checkRun.name, 'test_check');
- });
-
test('fromBuild creates correct unified check', () {
final build = Build(
id: Int64.MAX_VALUE,
diff --git a/app_dart/test/request_handlers/presubmit_luci_subscription_neutral_test.dart b/app_dart/test/request_handlers/presubmit_luci_subscription_neutral_test.dart
index 1dfae51..1745802 100644
--- a/app_dart/test/request_handlers/presubmit_luci_subscription_neutral_test.dart
+++ b/app_dart/test/request_handlers/presubmit_luci_subscription_neutral_test.dart
@@ -35,11 +35,7 @@
setUp(() async {
firestore = FakeFirestoreService();
- config = FakeConfig(
- dynamicConfig: DynamicConfig.fromJson({
- 'closeMqGuardAfterPresubmit': true,
- }),
- );
+ config = FakeConfig();
mockGithubChecksService = MockGithubChecksService();
mockScheduler = MockScheduler();
diff --git a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart
index 54f1bb0..b5a25fd 100644
--- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart
+++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart
@@ -39,11 +39,7 @@
setUp(() async {
firestore = FakeFirestoreService();
- config = FakeConfig(
- dynamicConfig: DynamicConfig.fromJson({
- 'closeMqGuardAfterPresubmit': true,
- }),
- );
+ config = FakeConfig();
mockLuciBuildService = MockLuciBuildService();
mockGithubChecksService = MockGithubChecksService();
mockScheduler = MockScheduler();
diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart
index 7f84931..8d801d3 100644
--- a/app_dart/test/service/scheduler_test.dart
+++ b/app_dart/test/service/scheduler_test.dart
@@ -1654,9 +1654,16 @@
for (final ignored in Scheduler.kCheckRunsToIgnore) {
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: ignored, sha: 'abc123'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: ignored,
+ sha: 'abc123',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -1689,9 +1696,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'abc123'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'abc123',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isFalse,
@@ -1728,9 +1742,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'abc123'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'abc123',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isFalse,
@@ -1782,9 +1803,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'abc123'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'abc123',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -1892,9 +1920,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'testSha'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2036,13 +2071,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(
- name: 'Bar bar',
- sha: 'testSha',
- checkSuiteId: 0,
- ),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 0,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2145,9 +2183,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'testSha'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2409,13 +2454,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(
- name: 'Bar bar',
- sha: 'testSha',
- conclusion: 'failure',
- ),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.failed,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2492,14 +2540,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(
- name: 'Bar bar',
- sha: 'testSha',
- conclusion: 'failure',
- headBranch: headBranch,
- ),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.failed,
+ isMergeGroup: true,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: headBranch,
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2575,13 +2625,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(
- name: 'Bar bar',
- sha: 'testSha',
- headBranch: headBranch,
- ),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: true,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: headBranch,
+ isUnifiedCheckRun: false,
),
),
isTrue,
@@ -2698,9 +2751,16 @@
expect(
await scheduler.processCheckRunCompleted(
- PresubmitCompletedJob.fromCheckRun(
- createCocoonCheckRun(name: 'Bar bar', sha: 'testSha'),
- createGithubRepository().slug(),
+ PresubmitCompletedJob(
+ name: 'Bar bar',
+ sha: 'testSha',
+ slug: createGithubRepository().slug(),
+ status: TaskStatus.succeeded,
+ isMergeGroup: false,
+ checkRunId: 1,
+ checkSuiteId: 668083231,
+ headBranch: 'master',
+ isUnifiedCheckRun: false,
),
),
isTrue,
