From 3eb5d9695494a0313315cef38551c97f711cd8af Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Thu, 3 Sep 2026 15:49:48 -0700 Subject: [PATCH 01/10] removed unifiedCheckRunFlow flag --- app_dart/config.yaml | 14 --- app_dart/lib/cocoon_service.dart | 1 - app_dart/lib/src/generated_config.dart | 14 --- .../service/firestore/unified_check_run.dart | 8 +- .../lib/src/service/flags/dynamic_config.dart | 17 --- .../src/service/flags/dynamic_config.g.dart | 6 - .../flags/unified_check_run_flow_flags.dart | 53 --------- .../flags/unified_check_run_flow_flags.g.dart | 24 ---- .../lib/src/service/luci_build_service.dart | 10 +- app_dart/lib/src/service/scheduler.dart | 74 +++++------- .../github/webhook_subscription_test.dart | 19 ++- .../firestore/unified_check_run_test.dart | 36 +----- .../schedule_try_builds_test.dart | 90 +------------- app_dart/test/service/scheduler_test.dart | 111 +++++++++--------- .../lib/src/fakes/fake_firestore_service.dart | 5 +- .../test/fake_firestore_service_test.dart | 4 +- 16 files changed, 111 insertions(+), 375 deletions(-) delete mode 100644 app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart delete mode 100644 app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart diff --git a/app_dart/config.yaml b/app_dart/config.yaml index 275a1fff4a..58d35222f9 100644 --- a/app_dart/config.yaml +++ b/app_dart/config.yaml @@ -28,20 +28,6 @@ dynamicTestSuppression: true # The Gemini model to use for log analysis. geminiModel: gemini-3-flash-preview -# Whether to allow unified check run flow to specific users or to everyone. -unifiedCheckRunFlow: - useForAll: true - useForUsers: - - ievdokdm - - eyebrowsoffire - - andywolff - - camsim99 - - walley892 - - loic-sharma - - vashworth - - mboetger - - justinmc - # Whether to process LUCI notifications of builds progress ordered within check run. orderedPresubmit: useForAll: true diff --git a/app_dart/lib/cocoon_service.dart b/app_dart/lib/cocoon_service.dart index 2fd0263f54..e54286cac0 100644 --- a/app_dart/lib/cocoon_service.dart +++ b/app_dart/lib/cocoon_service.dart @@ -60,7 +60,6 @@ export 'src/service/config.dart'; export 'src/service/firestore.dart'; export 'src/service/flags/dynamic_config.dart'; export 'src/service/flags/ordered_presubmit_flags.dart'; -export 'src/service/flags/unified_check_run_flow_flags.dart'; export 'src/service/gerrit_service.dart'; export 'src/service/github_checks_service.dart'; export 'src/service/issue_service.dart'; diff --git a/app_dart/lib/src/generated_config.dart b/app_dart/lib/src/generated_config.dart index 8337901621..6b9561636d 100644 --- a/app_dart/lib/src/generated_config.dart +++ b/app_dart/lib/src/generated_config.dart @@ -32,20 +32,6 @@ dynamicTestSuppression: true # The Gemini model to use for log analysis. geminiModel: gemini-3-flash-preview -# Whether to allow unified check run flow to specific users or to everyone. -unifiedCheckRunFlow: - useForAll: true - useForUsers: - - ievdokdm - - eyebrowsoffire - - andywolff - - camsim99 - - walley892 - - loic-sharma - - vashworth - - mboetger - - justinmc - # Whether to process LUCI notifications of builds progress ordered within check run. orderedPresubmit: useForAll: true 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 b763671efc..b9b272f88a 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -36,14 +36,10 @@ final class UnifiedCheckRun { CheckRun? mergeQueueGuard, @visibleForTesting DateTime Function() utcNow = DateTime.timestamp, }) async { - if (dashboardChecks != null && - pullRequest != null && - config.flags.isUnifiedCheckRunFlowEnabledForUser( - pullRequest.user!.login!, - )) { + if (dashboardChecks != null && pullRequest != null) { // Create the presubmit_guard and associated presubmit_job documents. log.info( - 'Storing UnifiedCheckRun data for ${slug.fullName}#${pullRequest.number} as it enabled for user ${pullRequest.user!.login}.', + 'Storing UnifiedCheckRun data for ${slug.fullName}#${pullRequest.number}.', ); // We store the creation time of the guard since there might be several // guards for the same PR created and each new one created after previous diff --git a/app_dart/lib/src/service/flags/dynamic_config.dart b/app_dart/lib/src/service/flags/dynamic_config.dart index 0e735074a0..2cf5f5d4ac 100644 --- a/app_dart/lib/src/service/flags/dynamic_config.dart +++ b/app_dart/lib/src/service/flags/dynamic_config.dart @@ -15,7 +15,6 @@ import 'ci_yaml_flags.dart'; import 'content_aware_hashing_flags.dart'; import 'dynamic_config_updater.dart'; import 'ordered_presubmit_flags.dart'; -import 'unified_check_run_flow_flags.dart'; part 'dynamic_config.g.dart'; @@ -39,7 +38,6 @@ final class DynamicConfig { contentAwareHashing: ContentAwareHashing.defaultInstance, closeMqGuardAfterPresubmit: false, enableGeminiLogAnalysis: false, - unifiedCheckRunFlow: UnifiedCheckRunFlow.defaultInstance, orderedPresubmit: OrderedPresubmit.defaultInstance, dynamicTestSuppression: false, geminiModel: 'gemini-3-flash-preview', @@ -69,10 +67,6 @@ final class DynamicConfig { @JsonKey() final bool enableGeminiLogAnalysis; - /// Flags related tp unified check-run flow configuration. - @JsonKey() - final UnifiedCheckRunFlow unifiedCheckRunFlow; - /// Flags related to ordered presubmit configuration. @JsonKey() final OrderedPresubmit orderedPresubmit; @@ -91,7 +85,6 @@ final class DynamicConfig { required this.contentAwareHashing, required this.closeMqGuardAfterPresubmit, required this.enableGeminiLogAnalysis, - required this.unifiedCheckRunFlow, required this.orderedPresubmit, required this.dynamicTestSuppression, required this.geminiModel, @@ -106,7 +99,6 @@ final class DynamicConfig { ContentAwareHashing? contentAwareHashing, bool? closeMqGuardAfterPresubmit, bool? enableGeminiLogAnalysis, - UnifiedCheckRunFlow? unifiedCheckRunFlow, OrderedPresubmit? orderedPresubmit, bool? dynamicTestSuppression, String? geminiModel, @@ -122,8 +114,6 @@ final class DynamicConfig { defaultInstance.closeMqGuardAfterPresubmit, enableGeminiLogAnalysis: enableGeminiLogAnalysis ?? defaultInstance.enableGeminiLogAnalysis, - unifiedCheckRunFlow: - unifiedCheckRunFlow ?? defaultInstance.unifiedCheckRunFlow, orderedPresubmit: orderedPresubmit ?? defaultInstance.orderedPresubmit, dynamicTestSuppression: dynamicTestSuppression ?? defaultInstance.dynamicTestSuppression, @@ -159,13 +149,6 @@ final class DynamicConfig { /// The inverse operation of [DynamicConfig.fromJson]. Map toJson() => _$DynamicConfigToJson(this); - bool isUnifiedCheckRunFlowEnabledForUser(String githubUsername) { - if (unifiedCheckRunFlow.useForAll) { - return true; - } - return unifiedCheckRunFlow.useForUsers.contains(githubUsername); - } - bool isOrderedPresubmitEnabledForUser(String githubUsername) { if (orderedPresubmit.useForAll) { return true; diff --git a/app_dart/lib/src/service/flags/dynamic_config.g.dart b/app_dart/lib/src/service/flags/dynamic_config.g.dart index d958c97c10..a760949baf 100644 --- a/app_dart/lib/src/service/flags/dynamic_config.g.dart +++ b/app_dart/lib/src/service/flags/dynamic_config.g.dart @@ -21,11 +21,6 @@ DynamicConfig _$DynamicConfigFromJson(Map json) => ), closeMqGuardAfterPresubmit: json['closeMqGuardAfterPresubmit'] as bool?, enableGeminiLogAnalysis: json['enableGeminiLogAnalysis'] as bool?, - unifiedCheckRunFlow: json['unifiedCheckRunFlow'] == null - ? null - : UnifiedCheckRunFlow.fromJson( - json['unifiedCheckRunFlow'] as Map?, - ), orderedPresubmit: json['orderedPresubmit'] == null ? null : OrderedPresubmit.fromJson( @@ -42,7 +37,6 @@ Map _$DynamicConfigToJson(DynamicConfig instance) => 'ciYaml': instance.ciYaml.toJson(), 'closeMqGuardAfterPresubmit': instance.closeMqGuardAfterPresubmit, 'enableGeminiLogAnalysis': instance.enableGeminiLogAnalysis, - 'unifiedCheckRunFlow': instance.unifiedCheckRunFlow.toJson(), 'orderedPresubmit': instance.orderedPresubmit.toJson(), 'dynamicTestSuppression': instance.dynamicTestSuppression, 'geminiModel': instance.geminiModel, diff --git a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart deleted file mode 100644 index 85cfc47524..0000000000 --- a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2025 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -import 'package:json_annotation/json_annotation.dart'; -import 'package:meta/meta.dart'; - -part 'unified_check_run_flow_flags.g.dart'; - -/// Flags related to content-aware hashing. -@JsonSerializable() -@immutable -final class UnifiedCheckRunFlow { - /// Default configuration for [UnifiedCheckRunFlow] flags. - static const defaultInstance = UnifiedCheckRunFlow._( - useForAll: false, - useForUsers: [], - ); - - /// Whether to use unified check-run flow with only one check-run created - /// for all LUCI tests or github check-run flow. - @JsonKey() - final bool useForAll; - - /// List of users to use unified check-run flow. - @JsonKey() - final List useForUsers; - - const UnifiedCheckRunFlow._({ - required this.useForAll, // - required this.useForUsers, // - }); - - /// Creates [UnifiedCheckRunFlow] flags from the provided fields. - /// - /// Any omitted fields default to the values in [defaultInstance]. - factory UnifiedCheckRunFlow({bool? useForAll, List? useForUsers}) { - return UnifiedCheckRunFlow._( - useForAll: useForAll ?? defaultInstance.useForAll, - useForUsers: useForUsers ?? defaultInstance.useForUsers, - ); - } - - /// Creates [UnifiedCheckRunFlow] flags from a [json] object. - /// - /// Any omitted fields default to the values in [defaultInstance]. - factory UnifiedCheckRunFlow.fromJson(Map? json) { - return _$UnifiedCheckRunFlowFromJson(json ?? {}); - } - - /// The inverse operation of [UnifiedCheckRunFlow.fromJson]. - Map toJson() => _$UnifiedCheckRunFlowToJson(this); -} diff --git a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart deleted file mode 100644 index 7c8879fe3e..0000000000 --- a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart +++ /dev/null @@ -1,24 +0,0 @@ -// GENERATED CODE - DO NOT MODIFY BY HAND - -// ignore_for_file: always_specify_types, implicit_dynamic_parameter - -part of 'unified_check_run_flow_flags.dart'; - -// ************************************************************************** -// JsonSerializableGenerator -// ************************************************************************** - -UnifiedCheckRunFlow _$UnifiedCheckRunFlowFromJson(Map json) => - UnifiedCheckRunFlow( - useForAll: json['useForAll'] as bool?, - useForUsers: (json['useForUsers'] as List?) - ?.map((e) => e as String) - .toList(), - ); - -Map _$UnifiedCheckRunFlowToJson( - UnifiedCheckRunFlow instance, -) => { - 'useForAll': instance.useForAll, - 'useForUsers': instance.useForUsers, -}; diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index fe49b651b7..33a0893539 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -293,8 +293,6 @@ class LuciBuildService { final slug = pullRequest.base!.repo!.slug(); final commitBranch = pullRequest.base!.ref!.replaceAll('refs/heads/', ''); final isFusion = slug == Config.flutterSlug; - final isUnifiedCheckRunFlow = _config.flags - .isUnifiedCheckRunFlowEnabledForUser(pullRequest.user!.login!); final isOrderedPresubmit = _config.flags.isOrderedPresubmitEnabledForUser( pullRequest.user!.login!, ); @@ -307,7 +305,7 @@ class LuciBuildService { late PresubmitUserData userData; // If the unified check run flow is enabled, do not create individual // check runs for each target but use the guard check run instead. - if (isUnifiedCheckRunFlow && dashboardChecks != null) { + if (dashboardChecks != null) { userData = PresubmitUserData( commit: CommitRef(slug: slug, sha: commitSha, branch: commitBranch), guardCheckRunId: dashboardChecks.id!, @@ -326,7 +324,7 @@ class LuciBuildService { for (final MapEntry(key: target, value: attemptNumber) in targets.entries) { // If the unified check run flow is disabled create individual check runs // for each target. - if (!isUnifiedCheckRunFlow || dashboardChecks == null) { + if (dashboardChecks == null) { final checkRun = await _githubChecksUtil.createCheckRun( _config, target.slug, @@ -396,7 +394,7 @@ class LuciBuildService { userData: userData, properties: properties, // if unified check run flow is enabled, use guard check run othervise check run id. - tags: isUnifiedCheckRunFlow && dashboardChecks != null + tags: dashboardChecks != null ? BuildTags([ GuardCheckRunIdBuildTag( guardCheckRunId: dashboardChecks.id!, @@ -449,7 +447,7 @@ class LuciBuildService { // initial run. For Re-run Failed Checks, if all failed jobs were reset, we // need to re-request the check run before updating it to in progress. final isRerun = targets.values.first > 1; - if (isUnifiedCheckRunFlow && dashboardChecks != null) { + if (dashboardChecks != null) { if (isRerun && stage != null) { try { final presubmitGuardDoc = await _firestore.getDocument( diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index ab984096d9..4d6ecf9fa7 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -349,10 +349,6 @@ class Scheduler { return; } - final isUnifiedCheckRun = _config.flags.isUnifiedCheckRunFlowEnabledForUser( - pullRequest.user!.login!, - ); - // Always cancel running builds so we don't ever schedule duplicates. log.info( 'Attempting to cancel existing presubmit targets for ${pullRequest.number}', @@ -367,12 +363,9 @@ class Scheduler { final lockResult = await lockMergeGroupChecks( slug, sha, - // Override details url of merge queue guard check for users with unified - // check run flow enabled - detailsUrl: isUnifiedCheckRun - ? 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${slug.name}&sha=$sha' - : null, - isUnifiedCheckRun: isUnifiedCheckRun, + detailsUrl: + 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${slug.name}&sha=$sha', + isUnifiedCheckRun: true, ); final dashboardChecks = lockResult.dashboardChecks; final mergeQueueGuard = lockResult.mergeQueueGuard; @@ -385,11 +378,12 @@ class Scheduler { log.info('Creating presubmit targets for ${pullRequest.number}'); Object? exception; - final isFusion = slug == Config.flutterSlug; - final isPackages = slug == Config.packagesSlug; + final isFlutterRepo = slug == Config.flutterSlug; + final isPackagesRepo = slug == Config.packagesSlug; do { try { - if (!isFusion && !(isPackages && isUnifiedCheckRun)) { + //if its not flutter or packages, unlock the merge group lock. + if (!(isFlutterRepo || isPackagesRepo)) { unlockMergeGroup = true; } @@ -433,7 +427,7 @@ class Scheduler { ); break; } - final presubmitTargets = isFusion + final presubmitTargets = isFlutterRepo ? await _getTestsForStage(pullRequest, CiStage.fusionEngineBuild) : await getPresubmitTargets(pullRequest); final presubmitTriggerTargets = filterTargets( @@ -443,20 +437,21 @@ class Scheduler { // When running presubmits for a fusion PR; create a new staging document to track tasks needed // to complete before we can schedule more tests (i.e. build engine artifacts before testing against them). + await UnifiedCheckRun.initializeCiStagingDocument( + firestoreService: _firestore, + slug: slug, + sha: sha, + stage: isFlutterRepo + ? CiStage.fusionEngineBuild + : CiStage.genericTests, + tasks: [...presubmitTriggerTargets.map((t) => t.name)], + pullRequest: pullRequest, + config: _config, + dashboardChecks: dashboardChecks, + mergeQueueGuard: mergeQueueGuard, + ); final EngineArtifacts engineArtifacts; - if (isFusion) { - await UnifiedCheckRun.initializeCiStagingDocument( - firestoreService: _firestore, - slug: slug, - sha: sha, - stage: CiStage.fusionEngineBuild, - tasks: [...presubmitTriggerTargets.map((t) => t.name)], - pullRequest: pullRequest, - config: _config, - dashboardChecks: dashboardChecks, - mergeQueueGuard: mergeQueueGuard, - ); - + if (isFlutterRepo) { // Even though this appears to be an engine build, it could be a // release candidate build, where the engine artifacts are built // via the dart-internal builder. @@ -468,21 +463,8 @@ class Scheduler { // See https://github.com/flutter/flutter/issues/165810. engineArtifacts = EngineArtifacts.usingExistingEngine(commitSha: sha); } else { - // For non-flutter repos, if unified check run flow is enabled, create - // a presubmit_guard document to track presubmit tests. - if (isUnifiedCheckRun) { - await UnifiedCheckRun.initializeCiStagingDocument( - firestoreService: _firestore, - slug: slug, - sha: sha, - stage: CiStage.genericTests, - tasks: [...presubmitTriggerTargets.map((t) => t.name)], - pullRequest: pullRequest, - config: _config, - dashboardChecks: dashboardChecks, - mergeQueueGuard: mergeQueueGuard, - ); - } + // For non-flutter repos create a presubmit_guard document + // to track presubmit tests. engineArtifacts = const EngineArtifacts.noFrameworkTests( reason: 'This is not the flutter/flutter repository', ); @@ -493,7 +475,9 @@ class Scheduler { engineArtifacts: engineArtifacts, dashboardChecks: dashboardChecks, mergeQueueGuard: mergeQueueGuard, - stage: isFusion ? CiStage.fusionEngineBuild : CiStage.genericTests, + stage: isFlutterRepo + ? CiStage.fusionEngineBuild + : CiStage.genericTests, ); } on FormatException catch (e, s) { log.warn( @@ -529,13 +513,9 @@ class Scheduler { // there are situations (see code above) when it needs to be unlocked // immediately. if (unlockMergeGroup) { - if (isUnifiedCheckRun) { await unlockMergeQueueGuard(slug, sha, dashboardChecks); if (mergeQueueGuard != null) { await unlockMergeQueueGuard(slug, sha, mergeQueueGuard); - } - } else if (mergeQueueGuard != null) { - await unlockMergeQueueGuard(slug, sha, mergeQueueGuard); } } log.info( diff --git a/app_dart/test/request_handlers/github/webhook_subscription_test.dart b/app_dart/test/request_handlers/github/webhook_subscription_test.dart index 85f01d46f0..04b521abc8 100644 --- a/app_dart/test/request_handlers/github/webhook_subscription_test.dart +++ b/app_dart/test/request_handlers/github/webhook_subscription_test.dart @@ -12,11 +12,11 @@ import 'package:cocoon_server/logging.dart'; import 'package:cocoon_server_test/mocks.dart'; import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/cocoon_service.dart'; -import 'package:cocoon_service/src/model/firestore/ci_staging.dart'; import 'package:cocoon_service/src/model/firestore/commit.dart' as fs; import 'package:cocoon_service/src/model/github/checks.dart' hide CheckRun; import 'package:cocoon_service/src/request_handling/exceptions.dart'; import 'package:cocoon_service/src/service/big_query.dart'; +import 'package:cocoon_service/src/service/firestore/unified_check_run.dart'; import 'package:cocoon_service/src/service/github_service.dart'; import 'package:fixnum/fixnum.dart'; import 'package:github/github.dart' hide Branch; @@ -96,9 +96,7 @@ void main() { wrongBaseBranchPullRequestMessageValue: '{{target_branch}} -> {{default_branch}}', ); - config.dynamicConfig = DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ); + config.dynamicConfig = DynamicConfig(); issuesService = MockIssuesService(); when( // ignore: discarded_futures @@ -149,6 +147,8 @@ void main() { any, any, output: anyNamed('output'), + conclusion: anyNamed('conclusion'), + detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((_) async { return CheckRun.fromJson(const { @@ -2792,13 +2792,20 @@ void foo() { }); test('Tries to schedule tests for a duplicate SHA warns', () async { - await CiStaging.initializeDocument( + final pr = generatePullRequest( + number: 1, + headSha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c', + ); + final checkRunGuard = generateCheckRun(1, name: Config.kDashboardCheckName); + await UnifiedCheckRun.initializeCiStagingDocument( firestoreService: firestore, slug: Config.flutterSlug, sha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c', stage: CiStage.fusionEngineBuild, tasks: [], - checkRunGuard: '', + config: config, + pullRequest: pr, + dashboardChecks: checkRunGuard, ); config.maxFilesChangedForSkippingEnginePhaseValue = 1; await testActions( diff --git a/app_dart/test/service/firestore/unified_check_run_test.dart b/app_dart/test/service/firestore/unified_check_run_test.dart index f6b486ace7..02e056e523 100644 --- a/app_dart/test/service/firestore/unified_check_run_test.dart +++ b/app_dart/test/service/firestore/unified_check_run_test.dart @@ -48,11 +48,7 @@ void main() { group('initializeCiStagingDocument', () { test('creates PresubmitGuard and Checks when enabled for user', () async { - config.dynamicConfig = DynamicConfig.fromJson({ - 'unifiedCheckRunFlow': { - 'useForUsers': ['dash'], - }, - }); + config.dynamicConfig = DynamicConfig(); await UnifiedCheckRun.initializeCiStagingDocument( firestoreService: firestoreService, @@ -88,36 +84,6 @@ void main() { expect(checkDoc.name, endsWith(checkId.documentId)); }); - test('initializes CiStagingDocument when NOT enabled for user', () async { - config.dynamicConfig = DynamicConfig.fromJson({ - 'unifiedCheckRunFlow': {'useForUsers': []}, - }); - - await UnifiedCheckRun.initializeCiStagingDocument( - firestoreService: firestoreService, - slug: slug, - sha: sha, - stage: CiStage.fusionEngineBuild, - tasks: ['linux', 'mac'], - config: config, - pullRequest: pullRequest, - mergeQueueGuard: checkRun, - ); - - // Verify PresubmitGuard is NOT created - final guardId = PresubmitGuard.documentIdFor( - slug: slug, - prNum: 1, - checkRunId: 123, - stage: CiStage.fusionEngineBuild, - ); - expect( - () => firestoreService.getDocument( - 'projects/flutter-dashboard/databases/cocoon/documents/presubmit_guards/${guardId.documentId}', - ), - throwsA(isA()), - ); - }); }); group('markConclusion', () { diff --git a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart index e8caad7bcd..ffa7135696 100644 --- a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart +++ b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart @@ -16,7 +16,6 @@ import 'package:cocoon_service/src/service/cache_service.dart'; import 'package:cocoon_service/src/service/firestore.dart'; import 'package:cocoon_service/src/service/flags/dynamic_config.dart'; import 'package:cocoon_service/src/service/flags/ordered_presubmit_flags.dart'; -import 'package:cocoon_service/src/service/flags/unified_check_run_flow_flags.dart'; import 'package:cocoon_service/src/service/luci_build_service.dart'; import 'package:cocoon_service/src/service/luci_build_service/build_tags.dart'; import 'package:cocoon_service/src/service/luci_build_service/engine_artifacts.dart'; @@ -421,9 +420,7 @@ void main() { // Enable Unified Check Run Flow luci = LuciBuildService( config: FakeConfig( - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ), + dynamicConfig: DynamicConfig(), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, @@ -493,9 +490,7 @@ void main() { // Enable Unified Check Run Flow but provide NO guard luci = LuciBuildService( config: FakeConfig( - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ), + dynamicConfig: DynamicConfig(), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, @@ -554,79 +549,6 @@ void main() { expect(userData.checkRunId, 456); expect(userData.guardCheckRunId, isNull); }); - - test( - 'does not update dashboard checks when unified flow is disabled', - () async { - final pullRequest = generatePullRequest( - id: 1, - repo: 'flutter', - headSha: 'headsha123', - ); - - final buildTarget = generateTarget( - 1, - properties: {'os': 'abc'}, - slug: RepositorySlug.full('flutter/flutter'), - name: 'Linux foo', - ); - - // Disable Unified Check Run Flow but provide a guard (unexpected but should be handled) - luci = LuciBuildService( - config: FakeConfig( - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ), - ), - cache: CacheService.inMemory(), - buildBucketClient: mockBuildBucketClient, - githubChecksUtil: mockGithubChecksUtil, - pubsub: pubSub, - gerritService: gerritService, - firestore: firestore, - ); - - final checkRunGuard = generateCheckRun(1234, name: 'Guard'); - - when( - mockGithubChecksUtil.createCheckRun(any, any, any, any), - ).thenAnswer((_) async => generateCheckRun(456, name: 'Linux foo')); - - await expectLater( - luci.scheduleTryBuilds( - pullRequest: pullRequest, - targets: [buildTarget], - engineArtifacts: EngineArtifacts.builtFromSource( - commitSha: pullRequest.head!.sha!, - ), - dashboardChecks: checkRunGuard, // Pass guard even though disabled - ), - completion([isTarget.hasName('Linux foo')]), - ); - - // Should NOT update dashboard checks - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - ), - ); - - // Should create individual check run because unified flow is disabled - verify( - mockGithubChecksUtil.createCheckRun( - any, - RepositorySlug.full('flutter/flutter'), - 'headsha123', - 'Linux foo', - ), - ).called(1); - }, - ); - test( 'reRequests check run for re-run failed checks when failedJobs is 0', () async { @@ -653,9 +575,7 @@ void main() { luci = LuciBuildService( config: FakeConfig( githubClient: mockGithubClient, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ), + dynamicConfig: DynamicConfig(), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, @@ -730,9 +650,7 @@ void main() { luci = LuciBuildService( config: FakeConfig( githubClient: mockGithubClient, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ), + dynamicConfig: DynamicConfig(), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index 8cfd86a878..4949f83436 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -76,10 +76,9 @@ void main() { Config.flutterSlug, Config.packagesSlug, }, + maxFilesChangedForSkippingEnginePhaseValue: 0, ); - config.dynamicConfig = DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ); + config.dynamicConfig = DynamicConfig(); fakeContentAwareHash = FakeContentAwareHashService(config: config); @@ -93,6 +92,8 @@ void main() { any, any, output: anyNamed('output'), + conclusion: anyNamed('conclusion'), + detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((Invocation invocation) async { return generateCheckRun( @@ -673,9 +674,7 @@ void main() { final mockGithubClient = MockGitHub(); config = FakeConfig( githubService: mockGithubService, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ), + dynamicConfig: DynamicConfig(), ); scheduler = Scheduler( githubService: config.githubService ?? FakeGithubService(), @@ -728,6 +727,7 @@ void main() { any, any, output: anyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((_) async { return CheckRun.fromJson(const { @@ -757,9 +757,16 @@ void main() { output: anyNamed('output'), ), ); - // Verfies Linux A was created + // Verifies Dashboard Checks was created verify( - mockGithubChecksUtil.createCheckRun(any, any, any, any), + mockGithubChecksUtil.createCheckRun( + any, + any, + any, + Config.kDashboardCheckName, + output: anyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), + ), ).called(1); }); @@ -2896,6 +2903,7 @@ targets: any, captureAny, output: captureAnyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -2915,9 +2923,6 @@ targets: summary: 'If this check is stuck pending, push an empty commit to retrigger the checks', ), - 'Linux A', - null, - // Linux runIf is not run as this is for tip of tree and the files weren't affected ], ); }); @@ -2926,9 +2931,7 @@ targets: 'creates presubmit_guard document for flutter/packages when unified check run flow is enabled', () async { getFilesChanged.cannedFiles = ['README.md']; - config.dynamicConfig = DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ); + config.dynamicConfig = DynamicConfig(); when( mockGithubChecksUtil.createCheckRun( @@ -2987,9 +2990,7 @@ targets: 'unlocks merge group for cocoon when unified check run flow is enabled', () async { getFilesChanged.cannedFiles = ['README.md']; - config.dynamicConfig = DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ); + config.dynamicConfig = DynamicConfig(); when( mockGithubChecksUtil.createCheckRun( @@ -3150,9 +3151,7 @@ targets: final fakeConfig = FakeConfig( githubService: mockGithubService, githubClient: MockGitHub(), - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ), + dynamicConfig: DynamicConfig(), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -3187,6 +3186,7 @@ targets: any, captureAny, output: captureAnyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -3206,13 +3206,12 @@ targets: summary: 'If this check is stuck pending, push an empty commit to retrigger the checks', ), - 'Linux A', - null, - // runIf requires a diff in dev, so an error will cause it to be triggered - 'Linux runIf', - null, ], ); + final guards = await firestore.query(PresubmitGuard.collectionId, {}); + expect(guards, isNotEmpty); + final guard = PresubmitGuard.fromDocument(guards.first); + expect(guard.jobs.keys, containsAll(['Linux A', 'Linux runIf'])); }, ); @@ -3233,6 +3232,7 @@ targets: any, captureAny, output: captureAnyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -3279,10 +3279,6 @@ targets: [ CheckRunStatus.completed, CheckRunConclusion.success, - CheckRunStatus.completed, - CheckRunConclusion.success, - CheckRunStatus.completed, - CheckRunConclusion.success, ], ); }); @@ -3314,11 +3310,6 @@ targets: await scheduler.triggerPresubmitTargets(pullRequest: pullRequest); expect(capturedUpdates, <(String, CheckRunStatus, CheckRunConclusion)>[ - ( - Config.kDashboardCheckName, - CheckRunStatus.completed, - CheckRunConclusion.success, - ), ( 'ci.yaml validation', CheckRunStatus.completed, @@ -3342,8 +3333,6 @@ targets: ), ).captured, [ - CheckRunStatus.completed, - CheckRunConclusion.success, CheckRunStatus.completed, CheckRunConclusion.failure, ], @@ -3444,6 +3433,7 @@ targets: any, any, output: anyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((inv) async { final slug = inv.positionalArguments[1] as RepositorySlug; @@ -3467,9 +3457,7 @@ targets: githubService: mockGithubService, githubClient: MockGitHub(), maxFilesChangedForSkippingEnginePhaseValue: 0, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ), + dynamicConfig: DynamicConfig(), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -3494,6 +3482,7 @@ targets: any, captureAny, output: captureAnyNamed('output'), + detailsUrl: anyNamed('detailsUrl'), ), ).captured; stdout.writeAll(results); @@ -3519,7 +3508,7 @@ targets: mockGithubChecksUtil.updateCheckRun( any, Config.flutterSlug, - checkRuns[1], + checkRuns[2], status: argThat(equals(CheckRunStatus.completed), named: 'status'), conclusion: argThat( equals(CheckRunConclusion.success), @@ -3539,6 +3528,16 @@ targets: output: anyNamed('output'), ), ); + verifyNever( + mockGithubChecksUtil.updateCheckRun( + any, + Config.flutterSlug, + checkRuns[1], + status: anyNamed('status'), + conclusion: anyNamed('conclusion'), + output: anyNamed('output'), + ), + ); }); }); @@ -4126,9 +4125,7 @@ targets: githubService: mockGithubService, githubClient: MockGitHub(), maxFilesChangedForSkippingEnginePhaseValue: 29, - dynamicConfig: DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), - ), + dynamicConfig: DynamicConfig(), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -4210,18 +4207,18 @@ targets: 'Linux analyze', ], reason: 'Should skip Linux engine_build'); - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging - .hasStage(CiStage.fusionEngineBuild) - .hasCheckRuns(isEmpty), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Linux A': TaskConclusion.scheduled, - 'Linux analyze': TaskConclusion.scheduled, - }), - ]), - ); + final guards = await firestore.query(PresubmitGuard.collectionId, {}); + final engineGuard = guards + .map(PresubmitGuard.fromDocument) + .firstWhere((g) => g.stage == CiStage.fusionEngineBuild); + expect(engineGuard.jobs, isEmpty); + final testsGuard = guards + .map(PresubmitGuard.fromDocument) + .firstWhere((g) => g.stage == CiStage.fusionTests); + expect(testsGuard.jobs, { + 'Linux A': TaskStatus.waitingForBackfill, + 'Linux analyze': TaskStatus.waitingForBackfill, + }); }); // Regression test for https://github.com/flutter/flutter/issues/167124. @@ -4306,9 +4303,7 @@ targets: // Enable fusion ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml); - config.dynamicConfig = DynamicConfig( - unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), - ); + config.dynamicConfig = DynamicConfig(); final userData = PresubmitUserData( commit: CommitRef( diff --git a/packages/cocoon_integration_test/lib/src/fakes/fake_firestore_service.dart b/packages/cocoon_integration_test/lib/src/fakes/fake_firestore_service.dart index 9b6601b284..fc81f5acee 100644 --- a/packages/cocoon_integration_test/lib/src/fakes/fake_firestore_service.dart +++ b/packages/cocoon_integration_test/lib/src/fakes/fake_firestore_service.dart @@ -469,8 +469,11 @@ abstract base class _FakeInMemoryFirestoreService 'simulate a backend failure.', ); } + final statusCode = result.any((r) => r.code == 9) + ? HttpStatus.conflict + : 500; throw DetailedApiRequestError( - 500, + statusCode, 'The transaction was aborted:\n' '${result.where((r) => r.code != 0).map((r) => r.message).join('\n')}', ); diff --git a/packages/cocoon_integration_test/test/fake_firestore_service_test.dart b/packages/cocoon_integration_test/test/fake_firestore_service_test.dart index e0d149ee67..043f3d22b1 100644 --- a/packages/cocoon_integration_test/test/fake_firestore_service_test.dart +++ b/packages/cocoon_integration_test/test/fake_firestore_service_test.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:io'; + import 'package:cocoon_integration_test/testing.dart'; import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/src/service/firestore.dart'; @@ -524,7 +526,7 @@ void main() { isA().having( (e) => e.status, 'status', - 500, + HttpStatus.conflict, ), ), ); From 314b6740906690b6b4f7cd54ba8af73d9e5a0d9a Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Thu, 3 Sep 2026 17:00:25 -0700 Subject: [PATCH 02/10] removed isUnifiedCheckRun --- .../common/presubmit_completed_check.dart | 5 +- .../presubmit_subscription.dart | 62 +- app_dart/lib/src/service/scheduler.dart | 165 +-- .../presubmit_completed_check_test.dart | 37 - .../github/webhook_subscription_test.dart | 5 +- .../presubmit_luci_subscription_test.dart | 241 +--- .../presubmit_ordered_subscription_test.dart | 8 - .../firestore/unified_check_run_test.dart | 2 - .../schedule_try_builds_test.dart | 8 +- app_dart/test/service/scheduler_test.dart | 1006 +---------------- .../lib/src/utilities/mocks.mocks.dart | 8 +- 11 files changed, 100 insertions(+), 1447 deletions(-) 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 d7a3139bc6..65b518542a 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -34,7 +34,6 @@ class PresubmitCompletedJob { final int checkRunId; final int? checkSuiteId; final String? headBranch; - final bool isUnifiedCheckRun; final CiStage? stage; final int? prNum; final int attempt; @@ -53,7 +52,6 @@ class PresubmitCompletedJob { required this.checkRunId, required this.checkSuiteId, required this.headBranch, - required this.isUnifiedCheckRun, this.stage, this.prNum, this.attempt = 1, @@ -80,7 +78,6 @@ class PresubmitCompletedJob { checkRunId: userData.guardCheckRunId ?? userData.checkRunId!, checkSuiteId: userData.checkSuiteId, headBranch: userData.commit.branch, - isUnifiedCheckRun: userData.guardCheckRunId != null, stage: userData.stage, prNum: userData.pullRequestNumber, attempt: _getAttempt(build), @@ -103,7 +100,7 @@ class PresubmitCompletedJob { cocoon_checks.CheckRun get checkRun { return cocoon_checks.CheckRun( id: checkRunId, - name: isUnifiedCheckRun ? Config.kDashboardCheckName : name, + name: Config.kDashboardCheckName, headSha: sha, conclusion: status.toConclusion(), checkSuite: CheckSuite( diff --git a/app_dart/lib/src/request_handlers/presubmit_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_subscription.dart index be629c233a..4b23ab63b7 100644 --- a/app_dart/lib/src/request_handlers/presubmit_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_subscription.dart @@ -47,13 +47,11 @@ base class PresubmitSubscription extends SubscriptionHandler { required super.subscriptionName, super.authProvider, }) : _ciYamlFetcher = ciYamlFetcher, - _githubChecksService = githubChecksService, _luciBuildService = luciBuildService, _scheduler = scheduler, _firestore = firestore; final LuciBuildService _luciBuildService; - final GithubChecksService _githubChecksService; final CiYamlFetcher _ciYamlFetcher; final Scheduler _scheduler; final FirestoreService _firestore; @@ -143,19 +141,21 @@ base class PresubmitSubscription extends SubscriptionHandler { tagSet ??= BuildTags.fromStringPairs(build.tags); final builderName = build.builder.builder; var rescheduled = false; - final isUnifiedCheckRun = userData.guardCheckRunId != null; - log.info('Unified Check Run ${isUnifiedCheckRun ? 'Enabled' : 'Disabled'}'); if (build.status.isTaskFailed()) { - if (isUnifiedCheckRun) { - // If failed we need summaryMarkdown. For github check run flow this - // called in [GithubChecksService.updateCheckStatus(...)] - build = await _luciBuildService.getBuildById( - build.id, - buildMask: bbv2.BuildMask( - // Need to use allFields as there is a bug with fieldMask and summaryMarkdown. - allFields: true, - ), - ); + // If failed we need summaryMarkdown. For github check run flow this + // called in [GithubChecksService.updateCheckStatus(...)] + final fullBuild = await _luciBuildService.getBuildById( + build.id, + buildMask: bbv2.BuildMask( + // Need to use allFields as there is a bug with fieldMask and summaryMarkdown. + allFields: true, + ), + ); + if (fullBuild.hasStatus() && + fullBuild.status != bbv2.Status.STATUS_UNSPECIFIED) { + build = fullBuild; + } else if (fullBuild.summaryMarkdown.isNotEmpty) { + build.summaryMarkdown = fullBuild.summaryMarkdown; } final maxAttempt = await _getMaxAttempt( userData.commit, @@ -165,17 +165,14 @@ base class PresubmitSubscription extends SubscriptionHandler { if (tagSet.currentAttempt < maxAttempt) { rescheduled = true; log.info('Rerunning failed task: $builderName'); - if (isUnifiedCheckRun) { - await UnifiedCheckRun.reInitializeInProgressJob( - firestoreService: _firestore, - completedJob: PresubmitCompletedJob.fromBuild( - build, - userData, - summaryPrepend: - '### ⚠️ Test failed but automatically rescheduled', - ), - ); - } + await UnifiedCheckRun.reInitializeInProgressJob( + firestoreService: _firestore, + completedJob: PresubmitCompletedJob.fromBuild( + build, + userData, + summaryPrepend: '### ⚠️ Test failed but automatically rescheduled', + ), + ); await _luciBuildService.reschedulePresubmitBuild( builderName: builderName, build: build, @@ -199,21 +196,6 @@ base class PresubmitSubscription extends SubscriptionHandler { '### ⚠️ Test failed but marked as suppressed on dashboard'; } } - if (!isUnifiedCheckRun) { - if (userData.checkRunId == null) { - log.error('checkRunId is null for non-unified check run'); - return; - } - await _githubChecksService.updateCheckStatus( - checkRunId: userData.checkRunId!, - build: build, - luciBuildService: _luciBuildService, - slug: userData.commit.slug, - rescheduled: rescheduled, - conclusionOverride: override, - summaryPrepend: suppressedMessage, - ); - } if (!rescheduled) { final check = PresubmitCompletedJob.fromBuild( build, diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 4d6ecf9fa7..bc0ee89aa4 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -19,12 +19,10 @@ import '../foundation/utils.dart'; import '../model/ci_yaml/ci_yaml.dart'; import '../model/ci_yaml/target.dart'; import '../model/commit_ref.dart'; -import '../model/common/checks_extension.dart'; import '../model/common/presubmit_completed_check.dart'; import '../model/common/presubmit_guard_conclusion.dart'; import '../model/common/presubmit_job_state.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'; @@ -365,7 +363,7 @@ class Scheduler { sha, detailsUrl: 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${slug.name}&sha=$sha', - isUnifiedCheckRun: true, + isPresubmit: true, ); final dashboardChecks = lockResult.dashboardChecks; final mergeQueueGuard = lockResult.mergeQueueGuard; @@ -513,9 +511,9 @@ class Scheduler { // there are situations (see code above) when it needs to be unlocked // immediately. if (unlockMergeGroup) { - await unlockMergeQueueGuard(slug, sha, dashboardChecks); - if (mergeQueueGuard != null) { - await unlockMergeQueueGuard(slug, sha, mergeQueueGuard); + await unlockCheckRun(slug, sha, dashboardChecks); + if (mergeQueueGuard != null) { + await unlockCheckRun(slug, sha, mergeQueueGuard); } } log.info( @@ -638,7 +636,7 @@ class Scheduler { final lockResult = await lockMergeGroupChecks( slug, headSha, - isUnifiedCheckRun: false, + isPresubmit: false, ); final dashboardChecks = lockResult.dashboardChecks; final mergeQueueGuard = lockResult.mergeQueueGuard!; @@ -646,7 +644,7 @@ class Scheduler { // If the repo is not fusion, it doesn't run anything in the MQ, so just // close the merge group guard. if (!isFusion) { - await unlockMergeQueueGuard(slug, headSha, mergeQueueGuard); + await unlockCheckRun(slug, headSha, mergeQueueGuard); return; } @@ -851,7 +849,7 @@ $s RepositorySlug slug, String headSha, { String? detailsUrl, - required bool isUnifiedCheckRun, + required bool isPresubmit, }) async { final mergeQueueGuard = await _githubChecksService.githubChecksUtil .createCheckRun( @@ -863,7 +861,7 @@ $s title: Config.kMergeQueueLockName, summary: kMergeQueueLockDescription, ), - detailsUrl: isUnifiedCheckRun ? null : detailsUrl, + detailsUrl: isPresubmit ? null : detailsUrl, ); final dashboardChecks = await _githubChecksService.githubChecksUtil @@ -876,10 +874,10 @@ $s title: Config.kDashboardCheckName, summary: kDashboardChecksDescription, ), - detailsUrl: isUnifiedCheckRun ? detailsUrl : null, + detailsUrl: isPresubmit ? detailsUrl : null, ); - if (!isUnifiedCheckRun) { + if (!isPresubmit) { // Skip Dashboard Checks await _githubChecksService.githubChecksUtil.updateCheckRun( _config, @@ -954,7 +952,7 @@ $s /// /// If the guard is guarding a pull request, this immediately makes the pull /// request eligible for enqueuing into the merge queue. - Future unlockMergeQueueGuard( + Future unlockCheckRun( RepositorySlug slug, String headSha, CheckRun lock, @@ -1109,56 +1107,23 @@ detailsUrl: $detailsUrl if (kCheckRunsToIgnore.contains(check.name)) { return true; } - final flow = check.isUnifiedCheckRun ? 'unified' : 'github'; final requestor = check.isMergeGroup ? 'merge group' : 'pull request'; final logCrumb = - 'checkCompleted(${check.name}, $flow, $requestor, ${check.slug}, ${check.sha}, ${check.status})'; - - final isFusion = check.slug == Config.flutterSlug; - if (!isFusion && !check.isUnifiedCheckRun) { - return true; - } + 'checkCompleted(${check.name}, $requestor, ${check.slug}, ${check.sha}, ${check.status})'; late CiStage stage; late PresubmitGuardConclusion stagingConclusion; - if (check.isUnifiedCheckRun) { - stage = check.stage!; - stagingConclusion = await _markUnifiedCheckRunConclusion( - guardId: check.guardId, - state: check.state, - ); - } else { - // for github flow check runs are processed only if the build succeeded or - // some kind of failure occurred. - if (!check.status.isComplete) { - return true; - } - // Check runs are fired at every stage. However, at this point it is unknown - // if this check run belongs in the engine build stage or in the test stage. - // So first look for it in the engine stage, and if it's missing, look for - // it in the test stage. - stage = CiStage.fusionEngineBuild; - stagingConclusion = await _recordCurrentCiStage( - slug: check.slug, - sha: check.sha, - stage: stage, - name: check.name, - conclusion: check.status.toTaskConclusion(), - ); + stage = + check.stage ?? + (check.slug == Config.flutterSlug + ? CiStage.fusionTests + : CiStage.genericTests); + stagingConclusion = await _markUnifiedCheckRunConclusion( + guardId: check.guardId, + state: check.state, + ); - if (stagingConclusion.result == PresubmitGuardConclusionResult.missing) { - // Check run not found in the engine stage. Look for it in the test stage. - stage = CiStage.fusionTests; - stagingConclusion = await _recordCurrentCiStage( - slug: check.slug, - sha: check.sha, - stage: stage, - name: check.name, - conclusion: check.status.toTaskConclusion(), - ); - } - } // First; check if we even recorded anything. This can occur if we've already passed the check_run and // have moved on to running more tests (which wouldn't be present in our document). if (!stagingConclusion.isOk) { @@ -1214,7 +1179,7 @@ detailsUrl: $detailsUrl summary: stagingConclusion.summary, details: stagingConclusion.details, ); - } else if (check.isUnifiedCheckRun) { + } else { final guard = checkRunFromString(stagingConclusion.dashboardChecks!); final detailsUrl = 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${check.slug.name}&sha=${check.sha}'; @@ -1263,6 +1228,7 @@ detailsUrl: $detailsUrl logCrumb: logCrumb, ); } + break; case CiStage.fusionTests: await _closeSuccessfulTestStage( dashboardChecks: stagingConclusion.dashboardChecks, @@ -1270,23 +1236,16 @@ detailsUrl: $detailsUrl slug: check.slug, sha: check.sha, logCrumb: logCrumb, - isUnifiedCheckRun: check.isUnifiedCheckRun, ); + break; case CiStage.genericTests: - if (check.isUnifiedCheckRun) { - await _closeSuccessfulTestStage( - dashboardChecks: stagingConclusion.dashboardChecks, - mergeQueueGuard: stagingConclusion.mergeQueueGuard, - slug: check.slug, - sha: check.sha, - logCrumb: logCrumb, - isUnifiedCheckRun: check.isUnifiedCheckRun, - ); - } else { - // generic tests do not have a staging document nor are associated - // with a merge group - they are only used to collect commit stats. - log.warn('$logCrumb: generic tests have no merge queue guard.'); - } + await _closeSuccessfulTestStage( + dashboardChecks: stagingConclusion.dashboardChecks, + mergeQueueGuard: stagingConclusion.mergeQueueGuard, + slug: check.slug, + sha: check.sha, + logCrumb: logCrumb, + ); break; } return true; @@ -1344,32 +1303,13 @@ detailsUrl: $detailsUrl required RepositorySlug slug, required String sha, required String logCrumb, - required bool isUnifiedCheckRun, }) async { log.info('$logCrumb: Test stage completed'); - if (isUnifiedCheckRun) { - if (dashboardChecks != null) { - await unlockMergeQueueGuard( - slug, - sha, - checkRunFromString(dashboardChecks), - ); - } - if (mergeQueueGuard != null) { - await unlockMergeQueueGuard( - slug, - sha, - checkRunFromString(mergeQueueGuard), - ); - } - } else { - if (mergeQueueGuard != null) { - await unlockMergeQueueGuard( - slug, - sha, - checkRunFromString(mergeQueueGuard), - ); - } + if (dashboardChecks != null) { + await unlockCheckRun(slug, sha, checkRunFromString(dashboardChecks)); + } + if (mergeQueueGuard != null) { + await unlockCheckRun(slug, sha, checkRunFromString(mergeQueueGuard)); } } @@ -1408,7 +1348,7 @@ detailsUrl: $detailsUrl // Unlock the guarding check_run. final checkRunGuard = checkRunFromString(mergeQueueGuard); - await unlockMergeQueueGuard(slug, sha, checkRunGuard); + await unlockCheckRun(slug, sha, checkRunGuard); } /// Schedules post-engine build tests (i.e. engine tests, and framework tests). @@ -1575,37 +1515,6 @@ $stacktrace } } - Future _recordCurrentCiStage({ - required RepositorySlug slug, - required String sha, - required CiStage stage, - required String name, - required TaskConclusion conclusion, - }) async { - final logCrumb = 'checkCompleted($name, $slug, $sha, $conclusion)'; - final documentName = CiStaging.documentNameFor( - slug: slug, - sha: sha, - stage: stage, - ); - log.info('$logCrumb: $documentName'); - - // We're doing a transactional update, which could fail if multiple tasks are running at the same time; so retry - // a sane amount of times before giving up. - const r = RetryOptions(maxAttempts: 3, delayFactor: Duration(seconds: 2)); - - return r.retry(() { - return CiStaging.markConclusion( - firestoreService: _firestore, - slug: slug, - sha: sha, - stage: stage, - checkRun: name, - conclusion: conclusion, - ); - }); - } - Future _markUnifiedCheckRunConclusion({ required PresubmitGuardId guardId, required PresubmitJobState state, 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 a2b9de29da..c89ba0b4d7 100644 --- a/app_dart/test/model/common/presubmit_completed_check_test.dart +++ b/app_dart/test/model/common/presubmit_completed_check_test.dart @@ -50,48 +50,11 @@ void main() { expect(check.checkRunId, 123); expect(check.checkSuiteId, 456); expect(check.headBranch, 'gh-readonly-queue/master/pr-123-abc'); - expect(check.isUnifiedCheckRun, true); expect(check.checkRun.name, Config.kDashboardCheckName); expect(check.buildNumber, 0); expect(check.buildId, Int64.MAX_VALUE); }); - test('fromBuild creates correct legacy check', () { - final build = Build( - id: Int64.MAX_VALUE, - builder: BuilderID(builder: 'test_builder'), - status: Status.SUCCESS, - number: 1234, - ); - - final userData = PresubmitUserData( - commit: CommitRef( - slug: slug, - sha: sha, - branch: 'gh-readonly-queue/master/pr-123-abc', - ), - stage: CiStage.fusionEngineBuild, - pullRequestNumber: 1, - checkRunId: 123, - checkSuiteId: 456, - ); - - final check = PresubmitCompletedJob.fromBuild(build, userData); - - expect(check.name, 'test_builder'); - expect(check.sha, sha); - expect(check.slug, slug); - expect(check.status, TaskStatus.succeeded); - expect(check.isMergeGroup, true); - expect(check.checkRunId, 123); - expect(check.checkSuiteId, 456); - expect(check.headBranch, 'gh-readonly-queue/master/pr-123-abc'); - expect(check.isUnifiedCheckRun, false); - expect(check.checkRun.name, 'test_builder'); - expect(check.buildNumber, 1234); - expect(check.buildId, Int64.MAX_VALUE); - }); - test('fromBuild handles custom status and summaryPrepend', () { final build = Build( id: Int64.MAX_VALUE, diff --git a/app_dart/test/request_handlers/github/webhook_subscription_test.dart b/app_dart/test/request_handlers/github/webhook_subscription_test.dart index 04b521abc8..ef462c1f51 100644 --- a/app_dart/test/request_handlers/github/webhook_subscription_test.dart +++ b/app_dart/test/request_handlers/github/webhook_subscription_test.dart @@ -2796,7 +2796,10 @@ void foo() { number: 1, headSha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c', ); - final checkRunGuard = generateCheckRun(1, name: Config.kDashboardCheckName); + final checkRunGuard = generateCheckRun( + 1, + name: Config.kDashboardCheckName, + ); await UnifiedCheckRun.initializeCiStagingDocument( firestoreService: firestore, slug: Config.flutterSlug, 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 058cc17668..2580914dde 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -78,15 +78,6 @@ void main() { }); test('Requests with repo_owner and repo_name update checks', () async { - when( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).thenAnswer((_) async => true); - when( mockGithubChecksService.conclusionForResult(any), ).thenAnswer((_) => github.CheckRunConclusion.empty); @@ -110,28 +101,11 @@ void main() { ); await tester.post(handler); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).called(1); verify(mockScheduler.processCheckRunCompleted(any)).called(1); }); test('Requests when task failed but no need to reschedule', () async { - when( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).thenAnswer((_) async => true); - when( mockGithubChecksService.conclusionForResult(any), ).thenAnswer((_) => github.CheckRunConclusion.empty); @@ -170,27 +144,19 @@ void main() { userData: userData, ), ); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).called(1); verify(mockScheduler.processCheckRunCompleted(any)).called(1); }); test('Requests when task failed but need to reschedule', () async { - when( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: true, + firestore.putDocument( + PresubmitJob.init( + slug: RepositorySlug('flutter', 'flutter'), + jobName: 'Linux presubmit_max_attempts=2', + checkRunId: 1, + creationTime: 12345, + attemptNumber: 1, ), - ).thenAnswer((_) async => true); + ); tester.message = createPushMessage( Int64(1), @@ -208,19 +174,19 @@ void main() { ); await tester.post(handler); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: true, - ), - ).called(1); verifyNever(mockScheduler.processCheckRunCompleted(any)); }); test('Build rescheduled when in merge queue', () async { + firestore.putDocument( + PresubmitJob.init( + slug: RepositorySlug('flutter', 'flutter'), + jobName: 'Linux A', + checkRunId: 1, + creationTime: 12345, + attemptNumber: 1, + ), + ); when( mockGithubChecksService.updateCheckStatus( build: anyNamed('build'), @@ -290,15 +256,6 @@ void main() { userData: anyNamed('userData'), ), ).called(1); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: true, - ), - ).called(1); verifyNever(mockScheduler.processCheckRunCompleted(any)); }); @@ -352,15 +309,6 @@ void main() { nextAttempt: 1, ), ); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: false, - ), - ).called(1); verify(mockScheduler.processCheckRunCompleted(any)).called(1); }); @@ -414,15 +362,6 @@ void main() { nextAttempt: 1, ), ); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: false, - ), - ).called(1); verify(mockScheduler.processCheckRunCompleted(any)).called(1); }); @@ -456,6 +395,15 @@ void main() { }); test('Build contains data from build_large_fields', () async { + firestore.putDocument( + PresubmitJob.init( + slug: RepositorySlug('flutter', 'flutter'), + jobName: 'Linux presubmit_max_attempts=2', + checkRunId: 1, + creationTime: 12345, + attemptNumber: 1, + ), + ); when( mockGithubChecksService.updateCheckStatus( build: anyNamed('build'), @@ -584,91 +532,6 @@ void main() { ); }); - test('Requests when task failed and is suppressed', () async { - final userData = PresubmitUserData( - commit: CommitRef( - sha: 'abc', - branch: 'master', - slug: RepositorySlug('flutter', 'flutter'), - ), - checkRunId: 1, - checkSuiteId: 2, - ); - - // Setup Firestore - firestore.putDocument( - SuppressedTest( - name: 'Linux A', - repository: 'flutter/flutter', - issueLink: 'https://github.com/flutter/flutter/issues/123', - isSuppressed: true, - createTimestamp: DateTime.now(), - ) - ..name = firestore.resolveDocumentName( - SuppressedTest.kCollectionId, - 'suppressed_1', - ), - ); - - when( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - conclusionOverride: github.CheckRunConclusion.neutral, - summaryPrepend: argThat( - contains('marked as suppressed'), - named: 'summaryPrepend', - ), - ), - ).thenAnswer((_) async => true); - - when( - mockScheduler.processCheckRunCompleted(any), - ).thenAnswer((_) async => true); - - tester.message = createPushMessage( - Int64(1), - status: bbv2.Status.FAILURE, - builder: 'Linux A', - userData: userData, - ); - - await tester.post(handler); - - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - conclusionOverride: github.CheckRunConclusion.neutral, - summaryPrepend: argThat( - contains('### ⚠️ Test failed but marked as suppressed on dashboard'), - named: 'summaryPrepend', - ), - ), - ).called(1); - - final captured = verify( - mockScheduler.processCheckRunCompleted(captureAny), - ).captured; - expect(captured, hasLength(1)); - expect( - captured[0], - isA() - .having((e) => e.status, 'status', TaskStatus.neutral) - .having( - (e) => e.summary, - 'summary', - contains( - '### ⚠️ Test failed but marked as suppressed on dashboard', - ), - ), - ); - }); - test('Requests when unified check run failed and is suppressed', () async { final userData = PresubmitUserData( commit: CommitRef( @@ -807,49 +670,6 @@ void main() { }, ); - test('Suppression check skipped when rescheduled', () async { - tester.message = createPushMessage( - Int64(1), - status: bbv2.Status.FAILURE, - builder: 'Linux presubmit_max_attempts=2', - userData: PresubmitUserData( - commit: CommitRef( - sha: 'abc', - branch: 'master', - slug: RepositorySlug('flutter', 'flutter'), - ), - checkRunId: 1, - checkSuiteId: 2, - ), - ); - - when( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: true, - conclusionOverride: null, - summaryPrepend: null, - ), - ).thenAnswer((_) async => true); - - await tester.post(handler); - - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - rescheduled: true, - conclusionOverride: null, - summaryPrepend: null, - ), - ).called(1); - }); - test('Unified Suppression check skipped when rescheduled', () async { buildBucketClient.getBuildResponse = Future.value( bbv2.Build() @@ -1006,14 +826,7 @@ void main() { expect(response, Response.emptyOk); expect(pubSub.topics, isEmpty); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).called(1); + verify(mockScheduler.processCheckRunCompleted(any)).called(1); }, ); } diff --git a/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart b/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart index b0eee7f8c4..13efb12e0d 100644 --- a/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart @@ -105,14 +105,6 @@ void main() { final response = await tester.post(handler); expect(response, Response.emptyOk); - verify( - mockGithubChecksService.updateCheckStatus( - build: anyNamed('build'), - checkRunId: anyNamed('checkRunId'), - luciBuildService: anyNamed('luciBuildService'), - slug: anyNamed('slug'), - ), - ).called(1); verify(mockScheduler.processCheckRunCompleted(any)).called(1); }, ); diff --git a/app_dart/test/service/firestore/unified_check_run_test.dart b/app_dart/test/service/firestore/unified_check_run_test.dart index 02e056e523..2a8ad462de 100644 --- a/app_dart/test/service/firestore/unified_check_run_test.dart +++ b/app_dart/test/service/firestore/unified_check_run_test.dart @@ -83,7 +83,6 @@ void main() { ); expect(checkDoc.name, endsWith(checkId.documentId)); }); - }); group('markConclusion', () { @@ -690,7 +689,6 @@ void main() { checkRunId: 123, checkSuiteId: 234, headBranch: 'master', - isUnifiedCheckRun: true, prNum: 567, attempt: 1, endTime: 2000, diff --git a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart index ffa7135696..86692dbbef 100644 --- a/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart +++ b/app_dart/test/service/luci_build_service/schedule_try_builds_test.dart @@ -419,9 +419,7 @@ void main() { // Enable Unified Check Run Flow luci = LuciBuildService( - config: FakeConfig( - dynamicConfig: DynamicConfig(), - ), + config: FakeConfig(dynamicConfig: DynamicConfig()), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, githubChecksUtil: mockGithubChecksUtil, @@ -489,9 +487,7 @@ void main() { // Enable Unified Check Run Flow but provide NO guard luci = LuciBuildService( - config: FakeConfig( - dynamicConfig: DynamicConfig(), - ), + config: FakeConfig(dynamicConfig: DynamicConfig()), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, githubChecksUtil: mockGithubChecksUtil, diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index 4949f83436..e67bab0db5 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -1624,15 +1624,6 @@ targets: test( 'ignores default check runs that have no side effects', () async { - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'abc123', - stage: CiStage.fusionTests, - tasks: ['foo', 'bar'], - checkRunGuard: '{}', - ); - for (final ignored in Scheduler.kCheckRunsToIgnore) { expect( await scheduler.processCheckRunCompleted( @@ -1645,584 +1636,14 @@ targets: checkRunId: 1, checkSuiteId: 668083231, headBranch: 'master', - isUnifiedCheckRun: false, ), ), isTrue, ); } - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasCheckRuns({ - 'foo': TaskConclusion.scheduled, - 'bar': TaskConclusion.scheduled, - }), - ]), - ); - }, - ); - - test('ignores invalid conclusions', () async { - final document = await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'abc123', - stage: CiStage.fusionTests, - tasks: ['Bar bar'], - checkRunGuard: '{}', - ); - - firestore.failOnWriteDocument(document); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'abc123', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isFalse, - ); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasCheckRuns({'Bar bar': TaskConclusion.scheduled}), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - }); - - test('does not complete with remaining tests', () async { - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'abc123', - stage: CiStage.fusionEngineBuild, - tasks: ['Foo foo', 'Bar bar'], - checkRunGuard: '{}', - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'abc123', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isFalse, - ); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasCheckRuns({ - 'Foo foo': TaskConclusion.scheduled, - 'Bar bar': TaskConclusion.success, - }), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - }); - - // The merge guard is not closed until both engine build and tests - // complete and are successful. - // This behavior is explained here: - // https://github.com/flutter/flutter/issues/159898#issuecomment-2597209435 - test( - 'failed tests neither unlock merge queue guard nor schedule test stage', - () async { - await PrCheckRuns.initializeDocument( - firestoreService: firestore, - pullRequest: pullRequest, - checks: [createGithubCheckRun(name: 'Bar bar')], - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'abc123', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'abc123', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasCheckRuns({'Bar bar': TaskConclusion.success}), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); }, ); - test('schedules tests after engine stage', () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - when(githubService.github).thenReturn(githubClient); - when( - githubService.searchIssuesAndPRs( - any, - any, - sort: anyNamed('sort'), - pages: anyNamed('pages'), - ), - ).thenAnswer((_) async => [generateIssue(42)]); - - final pullRequest = generatePullRequest(); - when( - githubService.getPullRequest(any, any), - ).thenAnswer((_) async => pullRequest); - getFilesChanged.cannedFiles = ['abc/def']; - when( - mockGithubChecksUtil.listCheckSuitesForRef( - any, - any, - ref: anyNamed('ref'), - ), - ).thenAnswer( - (_) async => [ - // From check_run.check_suite.id in [checkRunString]. - generateCheckSuite(668083231), - ], - ); - - ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml); - final luci = MockLuciBuildService(); - when( - luci.scheduleTryBuilds( - targets: anyNamed('targets'), - pullRequest: anyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ).thenAnswer((inv) async { - return []; - }); - - final gitHubChecksService = MockGithubChecksService(); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - when( - gitHubChecksService.findMatchingPullRequest(any, any, any), - ).thenAnswer((inv) async { - return pullRequest; - }); - - // Cocoon creates a Firestore document to track the tasks in the - // test stage. - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - verify( - gitHubChecksService.findMatchingPullRequest( - Config.flutterSlug, - 'testSha', - 668083231, - ), - ).called(1); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasStage(CiStage.fusionEngineBuild).hasCheckRuns({ - 'Bar bar': TaskConclusion.success, - }), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Linux A': TaskConclusion.scheduled, - 'Linux Z': TaskConclusion.scheduled, - 'Linux engine_presubmit': TaskConclusion.scheduled, - }), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - - final result = verify( - luci.scheduleTryBuilds( - targets: captureAnyNamed('targets'), - pullRequest: captureAnyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ); - expect(result.callCount, 1); - final captured = result.captured; - expect(captured[0], hasLength(3)); - // see the blend of fusionCiYaml and singleCiYaml - expect(captured[0][0].name, 'Linux A'); - expect(captured[0][1].name, 'Linux Z'); - expect(captured[0][2].name, 'Linux engine_presubmit'); - expect(captured[1], pullRequest); - }); - - test( - 'processCheckRunCompleted not failed when check suite id is 0', - () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - when(githubService.github).thenReturn(githubClient); - when( - githubService.searchIssuesAndPRs( - any, - any, - sort: anyNamed('sort'), - pages: anyNamed('pages'), - ), - ).thenAnswer((_) async => [generateIssue(42)]); - - final pullRequest = generatePullRequest(); - when( - githubService.getPullRequest(any, any), - ).thenAnswer((_) async => pullRequest); - getFilesChanged.cannedFiles = ['abc/def']; - when( - mockGithubChecksUtil.listCheckSuitesForRef( - any, - any, - ref: anyNamed('ref'), - ), - ).thenAnswer( - (_) async => [ - // From check_run.check_suite.id in [checkRunString]. - generateCheckSuite(668083231), - ], - ); - - ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml); - final luci = MockLuciBuildService(); - when( - luci.scheduleTryBuilds( - targets: anyNamed('targets'), - pullRequest: anyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ).thenAnswer((inv) async { - return []; - }); - - final gitHubChecksService = MockGithubChecksService(); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - when( - gitHubChecksService.findMatchingPullRequest(any, any, any), - ).thenAnswer((inv) async { - return pullRequest; - }); - - // Cocoon creates a Firestore document to track the tasks in the - // test stage. - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 0, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - verify( - gitHubChecksService.findMatchingPullRequest( - Config.flutterSlug, - 'testSha', - 0, - ), - ).called(1); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasStage(CiStage.fusionEngineBuild).hasCheckRuns({ - 'Bar bar': TaskConclusion.success, - }), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Linux A': TaskConclusion.scheduled, - 'Linux Z': TaskConclusion.scheduled, - 'Linux engine_presubmit': TaskConclusion.scheduled, - }), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - - final result = verify( - luci.scheduleTryBuilds( - targets: captureAnyNamed('targets'), - pullRequest: captureAnyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ); - expect(result.callCount, 1); - final captured = result.captured; - expect(captured[0], hasLength(3)); - // see the blend of fusionCiYaml and singleCiYaml - expect(captured[0][0].name, 'Linux A'); - expect(captured[0][1].name, 'Linux Z'); - expect(captured[0][2].name, 'Linux engine_presubmit'); - expect(captured[1], pullRequest); - }, - ); - - test('tracks test check runs in firestore', () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - final luci = MockLuciBuildService(); - final gitHubChecksService = MockGithubChecksService(); - - when(githubService.github).thenReturn(githubClient); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: [], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionTests, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - // The first invocation looks in the fusionEngineBuild stage, which - // returns "missing" result. - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging - .hasStage(CiStage.fusionEngineBuild) - .hasCheckRuns(isEmpty), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Bar bar': TaskConclusion.success, - }), - ]), - ); - - // Because tests completed, and completed successfully, the guard is - // unlocked, allowing the PR to land. - verify( - mockGithubChecksUtil.updateCheckRun( - any, - argThat(equals(RepositorySlug('flutter', 'flutter'))), - argThat( - predicate((arg) { - expect(arg.name, 'GUARD TEST'); - return true; - }), - ), - status: argThat( - equals(CheckRunStatus.completed), - named: 'status', - ), - conclusion: argThat( - equals(CheckRunConclusion.success), - named: 'conclusion', - ), - output: anyNamed('output'), - ), - ).called(1); - }); - test( 'writes failure comment if moving to next phase fails', () async { @@ -2395,421 +1816,6 @@ targets: ); }); - test( - 'does not fail the merge queue guard when a test check run fails (presubmit)', - () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - final luci = MockLuciBuildService(); - final gitHubChecksService = MockGithubChecksService(); - - when(githubService.github).thenReturn(githubClient); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: [], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionTests, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.failed, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - // The first invocation looks in the fusionEngineBuild stage, which - // returns "missing" result. - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging - .hasStage(CiStage.fusionEngineBuild) - .hasCheckRuns(isEmpty), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Bar bar': TaskConclusion.failure, - }), - ]), - ); - - // The test stage completed, but with failures. The merge queue - // guard should stay open to prevent the pull request from landing. - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - }, - ); - - test( - 'fails the merge queue guard when a test check run fails (merge group)', - () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - final luci = MockLuciBuildService(); - final gitHubChecksService = MockGithubChecksService(); - - when(githubService.github).thenReturn(githubClient); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - const headBranch = - 'gh-readonly-queue/master/pr-15-c9affbbb12aa40cb3afbe94b9ea6b119a256bebf'; - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor( - name: 'GUARD TEST', - headBranch: headBranch, - ), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.failed, - isMergeGroup: true, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: headBranch, - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - // The first invocation looks in the fusionEngineBuild stage, which - // returns "missing" result. - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasStage(CiStage.fusionEngineBuild).hasCheckRuns({ - 'Bar bar': TaskConclusion.failure, - }), - ]), - ); - - // The test stage completed, but with failures. The merge queue - // guard should stay open to prevent the pull request from landing. - verify( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: CheckRunConclusion.failure, - output: anyNamed('output'), - ), - ).called(1); - - expect(fakeContentAwareHash.completedShas, [ - (commitSha: 'testSha', successful: false), - ]); - }, - ); - - test('closes merge queue guard in merge group success', () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - final luci = MockLuciBuildService(); - final gitHubChecksService = MockGithubChecksService(); - - when(githubService.github).thenReturn(githubClient); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - getFilesChanged: getFilesChanged, - githubChecksService: gitHubChecksService, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - const headBranch = - 'gh-readonly-queue/master/pr-15-c9affbbb12aa40cb3afbe94b9ea6b119a256bebf'; - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor( - name: 'GUARD TEST', - headBranch: headBranch, - ), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: true, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: headBranch, - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - // The first invocation looks in the fusionEngineBuild stage, which - // returns "missing" result. - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasStage(CiStage.fusionEngineBuild).hasCheckRuns({ - 'Bar bar': TaskConclusion.success, - }), - ]), - ); - - // The test stage completed, but with failures. The merge queue - // guard should stay open to prevent the pull request from landing. - verify( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: CheckRunConclusion.success, - output: anyNamed('output'), - ), - ).called(1); - - expect(fakeContentAwareHash.completedShas, [ - (commitSha: 'testSha', successful: true), - ]); - }); - - test( - 'schedules tests after engine stage - with pr caching', - () async { - final githubService = config.githubService = MockGithubService(); - final githubClient = MockGitHub(); - when(githubService.github).thenReturn(githubClient); - when( - githubService.searchIssuesAndPRs( - any, - any, - sort: anyNamed('sort'), - pages: anyNamed('pages'), - ), - ).thenAnswer((_) async => [generateIssue(42)]); - - final pullRequest = generatePullRequest(); - when( - githubService.getPullRequest(any, any), - ).thenAnswer((_) async => pullRequest); - getFilesChanged.cannedFiles = ['abc/def']; - when( - mockGithubChecksUtil.listCheckSuitesForRef( - any, - any, - ref: anyNamed('ref'), - ), - ).thenAnswer( - (_) async => [ - // From check_run.check_suite.id in [checkRunString]. - generateCheckSuite(668083231), - ], - ); - - await PrCheckRuns.initializeDocument( - firestoreService: firestore, - checks: [generateCheckRun(1, name: 'Bar bar')], - pullRequest: pullRequest, - ); - - ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml); - final luci = MockLuciBuildService(); - when( - luci.scheduleTryBuilds( - targets: anyNamed('targets'), - pullRequest: anyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ).thenAnswer((inv) async { - return []; - }); - - final gitHubChecksService = MockGithubChecksService(); - when( - gitHubChecksService.githubChecksUtil, - ).thenReturn(mockGithubChecksUtil); - - scheduler = Scheduler( - githubService: config.githubService ?? FakeGithubService(), - cache: cache, - config: config, - githubChecksService: gitHubChecksService, - getFilesChanged: getFilesChanged, - ciYamlFetcher: ciYamlFetcher, - luciBuildService: luci, - contentAwareHash: fakeContentAwareHash, - firestore: firestore, - bigQuery: bigQuery, - ); - - await CiStaging.initializeDocument( - firestoreService: firestore, - slug: Config.flutterSlug, - sha: 'testSha', - stage: CiStage.fusionEngineBuild, - tasks: ['Bar bar'], - checkRunGuard: checkRunFor(name: 'GUARD TEST'), - ); - - expect( - await scheduler.processCheckRunCompleted( - PresubmitCompletedJob( - name: 'Bar bar', - sha: 'testSha', - slug: createGithubRepository().slug(), - status: TaskStatus.succeeded, - isMergeGroup: false, - checkRunId: 1, - checkSuiteId: 668083231, - headBranch: 'master', - isUnifiedCheckRun: false, - ), - ), - isTrue, - ); - - verifyNever( - gitHubChecksService.findMatchingPullRequest(any, any, any), - ); - - expect( - firestore, - existsInStorage(CiStaging.metadata, [ - isCiStaging.hasStage(CiStage.fusionEngineBuild).hasCheckRuns({ - 'Bar bar': TaskConclusion.success, - }), - isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ - 'Linux A': TaskConclusion.scheduled, - 'Linux Z': TaskConclusion.scheduled, - 'Linux engine_presubmit': TaskConclusion.scheduled, - }), - ]), - ); - - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - any, - any, - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); - - final result = verify( - luci.scheduleTryBuilds( - targets: captureAnyNamed('targets'), - pullRequest: captureAnyNamed('pullRequest'), - engineArtifacts: anyNamed('engineArtifacts'), - dashboardChecks: anyNamed('dashboardChecks'), - mergeQueueGuard: anyNamed('mergeQueueGuard'), - stage: anyNamed('stage'), - ), - ); - expect(result.callCount, 1); - final captured = result.captured; - expect(captured[0], hasLength(3)); - // see the blend of fusionCiYaml and singleCiYaml - expect(captured[0][0].name, 'Linux A'); - expect(captured[0][1].name, 'Linux Z'); - expect(captured[0][2].name, 'Linux engine_presubmit'); - expect( - captured[1], - isA().having( - (p) => p.number, - 'number', - pullRequest.number, - ), - ); - }, - ); // end of group }); }); @@ -3057,7 +2063,7 @@ targets: final lockResult = await scheduler.lockMergeGroupChecks( Config.flutterSlug, 'sha123', - isUnifiedCheckRun: true, + isPresubmit: true, ); expect(lockResult.dashboardChecks.name, Config.kDashboardCheckName); @@ -3276,10 +2282,7 @@ targets: output: anyNamed('output'), ), ).captured, - [ - CheckRunStatus.completed, - CheckRunConclusion.success, - ], + [CheckRunStatus.completed, CheckRunConclusion.success], ); }); @@ -3332,10 +2335,7 @@ targets: output: anyNamed('output'), ), ).captured, - [ - CheckRunStatus.completed, - CheckRunConclusion.failure, - ], + [CheckRunStatus.completed, CheckRunConclusion.failure], ); }); diff --git a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart index 8d4568ae7c..9926c61ad0 100644 --- a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart +++ b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart @@ -5731,13 +5731,13 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { _i7.RepositorySlug? slug, String? headSha, { String? detailsUrl, - required bool? isUnifiedCheckRun, + required bool? isPresubmit, }) => (super.noSuchMethod( Invocation.method( #lockMergeGroupChecks, [slug, headSha], - {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isUnifiedCheckRun}, + {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isPresubmit}, ), returnValue: _i13.Future<_i16.CheckRunLockResult>.value( _FakeCheckRunLockResult_58( @@ -5747,7 +5747,7 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { [slug, headSha], { #detailsUrl: detailsUrl, - #isUnifiedCheckRun: isUnifiedCheckRun, + #isUnifiedCheckRun: isPresubmit, }, ), ), @@ -5785,7 +5785,7 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { as _i13.Future); @override - _i13.Future unlockMergeQueueGuard( + _i13.Future unlockCheckRun( _i7.RepositorySlug? slug, String? headSha, _i7.CheckRun? lock, From 88c4db13c358473baef32746ae7573c1d42d17dc Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Fri, 4 Sep 2026 16:40:39 -0700 Subject: [PATCH 03/10] refactoring --- app_dart/lib/src/service/scheduler.dart | 45 ++++++++++++------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index bc0ee89aa4..8acf86c2a4 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -433,49 +433,48 @@ class Scheduler { builderTriggerList, ); + final stage = isFlutterRepo + ? CiStage.fusionEngineBuild + : CiStage.genericTests; + // When running presubmits for a fusion PR; create a new staging document to track tasks needed // to complete before we can schedule more tests (i.e. build engine artifacts before testing against them). await UnifiedCheckRun.initializeCiStagingDocument( firestoreService: _firestore, slug: slug, sha: sha, - stage: isFlutterRepo - ? CiStage.fusionEngineBuild - : CiStage.genericTests, + stage: stage, tasks: [...presubmitTriggerTargets.map((t) => t.name)], pullRequest: pullRequest, config: _config, dashboardChecks: dashboardChecks, mergeQueueGuard: mergeQueueGuard, ); - final EngineArtifacts engineArtifacts; - if (isFlutterRepo) { - // Even though this appears to be an engine build, it could be a - // release candidate build, where the engine artifacts are built - // via the dart-internal builder. - // - // In either case, providing FLUTTER_PREBUILT_ENGINE_VERSION has no - // consequences for engine builds, as it just won't be used (it is - // only understood by the Flutter CLI). - // - // See https://github.com/flutter/flutter/issues/165810. - engineArtifacts = EngineArtifacts.usingExistingEngine(commitSha: sha); - } else { - // For non-flutter repos create a presubmit_guard document - // to track presubmit tests. - engineArtifacts = const EngineArtifacts.noFrameworkTests( + // Even though this appears to be an engine build, it could be a + // release candidate build, where the engine artifacts are built + // via the dart-internal builder. + // + // In either case, providing FLUTTER_PREBUILT_ENGINE_VERSION has no + // consequences for engine builds, as it just won't be used (it is + // only understood by the Flutter CLI). + // + // See https://github.com/flutter/flutter/issues/165810. + // + // For non-flutter repos create a presubmit_guard document + // to track presubmit tests. + final engineArtifacts = isFlutterRepo + ? EngineArtifacts.usingExistingEngine(commitSha: sha) + : const EngineArtifacts.noFrameworkTests( reason: 'This is not the flutter/flutter repository', ); - } + await _luciBuildService.scheduleTryBuilds( targets: presubmitTriggerTargets, pullRequest: pullRequest, engineArtifacts: engineArtifacts, dashboardChecks: dashboardChecks, mergeQueueGuard: mergeQueueGuard, - stage: isFlutterRepo - ? CiStage.fusionEngineBuild - : CiStage.genericTests, + stage: stage, ); } on FormatException catch (e, s) { log.warn( From 7879553b0f6efa4e8f3cdbd62473583cb45457c5 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Fri, 4 Sep 2026 17:09:06 -0700 Subject: [PATCH 04/10] fix AI review comments --- .../model/common/presubmit_completed_check.dart | 6 +++++- .../request_handlers/presubmit_subscription.dart | 8 +------- app_dart/lib/src/service/scheduler.dart | 14 ++++---------- 3 files changed, 10 insertions(+), 18 deletions(-) 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 65b518542a..8bafb12740 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -118,7 +118,11 @@ class PresubmitCompletedJob { slug: slug, prNum: prNum ?? 0, checkRunId: checkRunId, - stage: stage ?? CiStage.fusionTests, + stage: + stage ?? + (slug == Config.flutterSlug + ? CiStage.fusionTests + : CiStage.genericTests), ); } diff --git a/app_dart/lib/src/request_handlers/presubmit_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_subscription.dart index 4b23ab63b7..ad4c1b09a6 100644 --- a/app_dart/lib/src/request_handlers/presubmit_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_subscription.dart @@ -144,19 +144,13 @@ base class PresubmitSubscription extends SubscriptionHandler { if (build.status.isTaskFailed()) { // If failed we need summaryMarkdown. For github check run flow this // called in [GithubChecksService.updateCheckStatus(...)] - final fullBuild = await _luciBuildService.getBuildById( + build = await _luciBuildService.getBuildById( build.id, buildMask: bbv2.BuildMask( // Need to use allFields as there is a bug with fieldMask and summaryMarkdown. allFields: true, ), ); - if (fullBuild.hasStatus() && - fullBuild.status != bbv2.Status.STATUS_UNSPECIFIED) { - build = fullBuild; - } else if (fullBuild.summaryMarkdown.isNotEmpty) { - build.summaryMarkdown = fullBuild.summaryMarkdown; - } final maxAttempt = await _getMaxAttempt( userData.commit, builderName, diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 8acf86c2a4..86a56f10c5 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -459,14 +459,11 @@ class Scheduler { // only understood by the Flutter CLI). // // See https://github.com/flutter/flutter/issues/165810. - // - // For non-flutter repos create a presubmit_guard document - // to track presubmit tests. final engineArtifacts = isFlutterRepo ? EngineArtifacts.usingExistingEngine(commitSha: sha) : const EngineArtifacts.noFrameworkTests( - reason: 'This is not the flutter/flutter repository', - ); + reason: 'This is not the flutter/flutter repository', + ); await _luciBuildService.scheduleTryBuilds( targets: presubmitTriggerTargets, @@ -1110,15 +1107,12 @@ detailsUrl: $detailsUrl final logCrumb = 'checkCompleted(${check.name}, $requestor, ${check.slug}, ${check.sha}, ${check.status})'; - late CiStage stage; - late PresubmitGuardConclusion stagingConclusion; - - stage = + final stage = check.stage ?? (check.slug == Config.flutterSlug ? CiStage.fusionTests : CiStage.genericTests); - stagingConclusion = await _markUnifiedCheckRunConclusion( + final stagingConclusion = await _markUnifiedCheckRunConclusion( guardId: check.guardId, state: check.state, ); From 619cec60ee2de3eda6a9ef6d38b5a397cd72f790 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Fri, 4 Sep 2026 17:19:47 -0700 Subject: [PATCH 05/10] fix unit tests --- .../presubmit_luci_subscription_test.dart | 57 ++++++++++++++++++- 1 file changed, 55 insertions(+), 2 deletions(-) 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 2580914dde..ead791d05f 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -1,6 +1,7 @@ // Copyright 2019 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:archive/archive.dart'; import 'package:buildbucket/buildbucket_pb.dart' as bbv2; import 'package:cocoon_common/task_status.dart'; @@ -106,6 +107,14 @@ void main() { }); test('Requests when task failed but no need to reschedule', () async { + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build( + id: Int64(1), + builder: bbv2.BuilderID(builder: 'Linux A'), + status: bbv2.Status.FAILURE, + summaryMarkdown: 'test summary', + ), + ); when( mockGithubChecksService.conclusionForResult(any), ).thenAnswer((_) => github.CheckRunConclusion.empty); @@ -148,6 +157,14 @@ void main() { }); test('Requests when task failed but need to reschedule', () async { + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build( + id: Int64(1), + builder: bbv2.BuilderID(builder: 'Linux presubmit_max_attempts=2'), + status: bbv2.Status.FAILURE, + summaryMarkdown: 'test summary', + ), + ); firestore.putDocument( PresubmitJob.init( slug: RepositorySlug('flutter', 'flutter'), @@ -198,7 +215,14 @@ void main() { ).thenAnswer((_) async => true); when( mockLuciBuildService.getBuildById(any, buildMask: anyNamed('buildMask')), - ).thenAnswer((_) async => bbv2.Build(summaryMarkdown: 'test summary')); + ).thenAnswer( + (_) async => bbv2.Build( + id: Int64(1), + builder: bbv2.BuilderID(builder: 'Linux A'), + status: bbv2.Status.INFRA_FAILURE, + summaryMarkdown: 'test summary', + ), + ); tester.message = createPushMessage( Int64(1), @@ -260,6 +284,14 @@ void main() { }); test('Build not rescheduled if not found in ciYaml list.', () async { + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build( + id: Int64(1), + builder: bbv2.BuilderID(builder: 'Linux C'), + status: bbv2.Status.FAILURE, + summaryMarkdown: 'test summary', + ), + ); when( mockGithubChecksService.updateCheckStatus( build: anyNamed('build'), @@ -314,6 +346,14 @@ void main() { }); test('Build not rescheduled if ci.yaml fails validation.', () async { + buildBucketClient.getBuildResponse = Future.value( + bbv2.Build( + id: Int64(1), + builder: bbv2.BuilderID(builder: 'Linux C'), + status: bbv2.Status.FAILURE, + summaryMarkdown: 'test summary', + ), + ); when( mockGithubChecksService.updateCheckStatus( build: anyNamed('build'), @@ -413,9 +453,22 @@ void main() { rescheduled: anyNamed('rescheduled'), ), ).thenAnswer((_) async => true); + final fullBuild = + createBuild( + Int64(1), + status: bbv2.Status.FAILURE, + builder: 'Linux presubmit_max_attempts=2', + ).build + ..mergeFromBuffer( + const ZLibDecoder().decodeBytes( + createBuild(Int64(1)).buildLargeFields, + ), + ) + ..summaryMarkdown = 'test summary'; + when( mockLuciBuildService.getBuildById(any, buildMask: anyNamed('buildMask')), - ).thenAnswer((_) async => bbv2.Build(summaryMarkdown: 'test summary')); + ).thenAnswer((_) async => fullBuild); tester.message = createPushMessage( Int64(1), From aed2379f40f65d3a5ee24738a23add824de2b7e2 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Fri, 4 Sep 2026 17:28:05 -0700 Subject: [PATCH 06/10] fix license header --- .../test/request_handlers/presubmit_luci_subscription_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ead791d05f..59a5e5c41e 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -1,8 +1,8 @@ // Copyright 2019 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:archive/archive.dart'; +import 'package:archive/archive.dart'; import 'package:buildbucket/buildbucket_pb.dart' as bbv2; import 'package:cocoon_common/task_status.dart'; import 'package:cocoon_integration_test/testing.dart'; From 958d9231b9b9eb9cfc15a75beeff2db5796cca70 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Fri, 4 Sep 2026 17:37:35 -0700 Subject: [PATCH 07/10] format --- .../lib/src/utilities/mocks.mocks.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart index 9926c61ad0..85ff525abd 100644 --- a/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart +++ b/packages/cocoon_integration_test/lib/src/utilities/mocks.mocks.dart @@ -5745,10 +5745,7 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { Invocation.method( #lockMergeGroupChecks, [slug, headSha], - { - #detailsUrl: detailsUrl, - #isUnifiedCheckRun: isPresubmit, - }, + {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isPresubmit}, ), ), ), From 274eae5f2fd8311b856a2103773e14995c726a69 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Tue, 8 Sep 2026 11:32:29 -0700 Subject: [PATCH 08/10] Update app_dart/lib/src/service/scheduler.dart Co-authored-by: Jackson Gardner --- app_dart/lib/src/service/scheduler.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 86a56f10c5..d7550dbae8 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -380,8 +380,8 @@ class Scheduler { final isPackagesRepo = slug == Config.packagesSlug; do { try { - //if its not flutter or packages, unlock the merge group lock. - if (!(isFlutterRepo || isPackagesRepo)) { + // If it's not flutter or packages, unlock the merge group lock. + if (!isFlutterRepo && !isPackagesRepo) { unlockMergeGroup = true; } From ee58d9c81abe9866f9428f2414c261a9ed9f4144 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Tue, 8 Sep 2026 13:30:34 -0700 Subject: [PATCH 09/10] fixed `dart analyze --fatal-infos` for cipd --- cipd_packages/device_doctor/lib/src/health.dart | 11 ++++++----- .../lib/src/ios_debug_symbol_doctor.dart | 4 ++-- cipd_packages/device_doctor/test/src/health_test.dart | 3 ++- .../test/src/ios_debug_symbol_doctor_test.dart | 7 ++++--- cipd_packages/device_doctor/test/src/utils.dart | 6 ------ 5 files changed, 14 insertions(+), 17 deletions(-) diff --git a/cipd_packages/device_doctor/lib/src/health.dart b/cipd_packages/device_doctor/lib/src/health.dart index 1b658803bc..0ea948f9d7 100644 --- a/cipd_packages/device_doctor/lib/src/health.dart +++ b/cipd_packages/device_doctor/lib/src/health.dart @@ -17,7 +17,7 @@ import 'utils.dart'; Future closeIosDialog({ ProcessManager pm = const LocalProcessManager(), String? deviceId, - platform.Platform pl = const platform.LocalPlatform(), + platform.Platform pl = const platform.Platform(), String infraDialog = 'infra-dialog', }) async { var dialogDir = dir(path.dirname(Platform.script.path), 'tool', infraDialog); @@ -36,15 +36,16 @@ Future closeIosDialog({ // By default the above command relies on automatic code signing, while on devicelab machines // it should utilize manual code signing as that is more stable. Below overwrites the code // signing config if one exists in the environment. - if (pl.environment['FLUTTER_XCODE_CODE_SIGN_STYLE'] != null) { + if (pl.nativePlatform!.environment['FLUTTER_XCODE_CODE_SIGN_STYLE'] != + null) { command.add( - "CODE_SIGN_STYLE=${pl.environment['FLUTTER_XCODE_CODE_SIGN_STYLE']}", + "CODE_SIGN_STYLE=${pl.nativePlatform!.environment['FLUTTER_XCODE_CODE_SIGN_STYLE']}", ); command.add( - "DEVELOPMENT_TEAM=${pl.environment['FLUTTER_XCODE_DEVELOPMENT_TEAM']}", + "DEVELOPMENT_TEAM=${pl.nativePlatform!.environment['FLUTTER_XCODE_DEVELOPMENT_TEAM']}", ); command.add( - "PROVISIONING_PROFILE_SPECIFIER=${pl.environment['FLUTTER_XCODE_PROVISIONING_PROFILE_SPECIFIER']}", + "PROVISIONING_PROFILE_SPECIFIER=${pl.nativePlatform!.environment['FLUTTER_XCODE_PROVISIONING_PROFILE_SPECIFIER']}", ); } final proc = await pm.start(command, workingDirectory: dialogDir.path); diff --git a/cipd_packages/device_doctor/lib/src/ios_debug_symbol_doctor.dart b/cipd_packages/device_doctor/lib/src/ios_debug_symbol_doctor.dart index fff64d374a..a88820019a 100644 --- a/cipd_packages/device_doctor/lib/src/ios_debug_symbol_doctor.dart +++ b/cipd_packages/device_doctor/lib/src/ios_debug_symbol_doctor.dart @@ -64,7 +64,7 @@ class RecoverCommand extends Command { this.processManager = const LocalProcessManager(), Logger? loggerOverride, this.fs = const LocalFileSystem(), - this.platform = const LocalPlatform(), + this.platform = const Platform(), }) : logger = loggerOverride ?? Logger.root { argParser ..addOption( @@ -197,7 +197,7 @@ class RecoverCommand extends Command { /// Xcode will regenerate this folder and symbols for connected devices /// when Xcode is opened. void _deleteSymbols() { - final home = platform.environment['HOME']; + final home = platform.nativePlatform!.environment['HOME']; if (home == null) { logger.warning('\$HOME path was not found'); return; diff --git a/cipd_packages/device_doctor/test/src/health_test.dart b/cipd_packages/device_doctor/test/src/health_test.dart index 50954eeea8..9495be2fad 100644 --- a/cipd_packages/device_doctor/test/src/health_test.dart +++ b/cipd_packages/device_doctor/test/src/health_test.dart @@ -8,6 +8,7 @@ import 'package:device_doctor/src/health.dart'; import 'package:device_doctor/src/utils.dart'; import 'package:mockito/mockito.dart'; import 'package:platform/platform.dart' as platform; +import 'package:platform/testing.dart'; import 'package:test/test.dart'; import 'utils.dart'; @@ -36,7 +37,7 @@ void main() { when( pm.start(any, workingDirectory: anyNamed('workingDirectory')), ).thenAnswer((_) => Future.value(proc)); - final platform.Platform pl = platform.FakePlatform( + final platform.Platform pl = TestPlatform.native( environment: { 'FLUTTER_XCODE_CODE_SIGN_STYLE': 'Manual', 'FLUTTER_XCODE_DEVELOPMENT_TEAM': 'S8QB4VV633', diff --git a/cipd_packages/device_doctor/test/src/ios_debug_symbol_doctor_test.dart b/cipd_packages/device_doctor/test/src/ios_debug_symbol_doctor_test.dart index 4c91f6536a..4387ec3037 100644 --- a/cipd_packages/device_doctor/test/src/ios_debug_symbol_doctor_test.dart +++ b/cipd_packages/device_doctor/test/src/ios_debug_symbol_doctor_test.dart @@ -11,7 +11,7 @@ import 'package:fake_async/fake_async.dart'; import 'package:file/memory.dart'; import 'package:logging/logging.dart'; import 'package:mockito/mockito.dart'; -import 'package:platform/platform.dart'; +import 'package:platform/testing.dart'; import 'package:test/test.dart'; @@ -102,8 +102,9 @@ Future main() async { logger = TestLogger(); fs = MemoryFileSystem(); fs.directory(xcworkspacePath).createSync(recursive: true); - platform = MockPlatform(); - platform.environment['HOME'] = '/User/username'; + platform = TestPlatform.native( + environment: {'HOME': '/User/username'}, + ); }); test('diagnose logs output of xcdevice list', () async { diff --git a/cipd_packages/device_doctor/test/src/utils.dart b/cipd_packages/device_doctor/test/src/utils.dart index d308810d01..4d15aebc17 100644 --- a/cipd_packages/device_doctor/test/src/utils.dart +++ b/cipd_packages/device_doctor/test/src/utils.dart @@ -8,14 +8,8 @@ import 'dart:io'; import 'package:logging/logging.dart'; import 'package:mockito/mockito.dart'; -import 'package:platform/platform.dart'; import 'package:process/process.dart'; -class MockPlatform extends Mock implements Platform { - @override - Map environment = {}; -} - class MockProcessManager extends Mock implements ProcessManager { @override Future start( From ef10340ad051682eb7c1a9c0fab08f10b591e319 Mon Sep 17 00:00:00 2001 From: "Dmitry Grand (dmgr)" Date: Tue, 8 Sep 2026 16:10:55 -0700 Subject: [PATCH 10/10] revert configuration as it should go after changes in code --- app_dart/config.yaml | 14 +++++ app_dart/lib/src/generated_config.dart | 14 +++++ .../lib/src/service/flags/dynamic_config.dart | 17 ++++++ .../src/service/flags/dynamic_config.g.dart | 6 +++ .../flags/unified_check_run_flow_flags.dart | 53 +++++++++++++++++++ .../flags/unified_check_run_flow_flags.g.dart | 24 +++++++++ 6 files changed, 128 insertions(+) create mode 100644 app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart create mode 100644 app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart diff --git a/app_dart/config.yaml b/app_dart/config.yaml index 58d35222f9..275a1fff4a 100644 --- a/app_dart/config.yaml +++ b/app_dart/config.yaml @@ -28,6 +28,20 @@ dynamicTestSuppression: true # The Gemini model to use for log analysis. geminiModel: gemini-3-flash-preview +# Whether to allow unified check run flow to specific users or to everyone. +unifiedCheckRunFlow: + useForAll: true + useForUsers: + - ievdokdm + - eyebrowsoffire + - andywolff + - camsim99 + - walley892 + - loic-sharma + - vashworth + - mboetger + - justinmc + # Whether to process LUCI notifications of builds progress ordered within check run. orderedPresubmit: useForAll: true diff --git a/app_dart/lib/src/generated_config.dart b/app_dart/lib/src/generated_config.dart index 6b9561636d..8337901621 100644 --- a/app_dart/lib/src/generated_config.dart +++ b/app_dart/lib/src/generated_config.dart @@ -32,6 +32,20 @@ dynamicTestSuppression: true # The Gemini model to use for log analysis. geminiModel: gemini-3-flash-preview +# Whether to allow unified check run flow to specific users or to everyone. +unifiedCheckRunFlow: + useForAll: true + useForUsers: + - ievdokdm + - eyebrowsoffire + - andywolff + - camsim99 + - walley892 + - loic-sharma + - vashworth + - mboetger + - justinmc + # Whether to process LUCI notifications of builds progress ordered within check run. orderedPresubmit: useForAll: true diff --git a/app_dart/lib/src/service/flags/dynamic_config.dart b/app_dart/lib/src/service/flags/dynamic_config.dart index 2cf5f5d4ac..0e735074a0 100644 --- a/app_dart/lib/src/service/flags/dynamic_config.dart +++ b/app_dart/lib/src/service/flags/dynamic_config.dart @@ -15,6 +15,7 @@ import 'ci_yaml_flags.dart'; import 'content_aware_hashing_flags.dart'; import 'dynamic_config_updater.dart'; import 'ordered_presubmit_flags.dart'; +import 'unified_check_run_flow_flags.dart'; part 'dynamic_config.g.dart'; @@ -38,6 +39,7 @@ final class DynamicConfig { contentAwareHashing: ContentAwareHashing.defaultInstance, closeMqGuardAfterPresubmit: false, enableGeminiLogAnalysis: false, + unifiedCheckRunFlow: UnifiedCheckRunFlow.defaultInstance, orderedPresubmit: OrderedPresubmit.defaultInstance, dynamicTestSuppression: false, geminiModel: 'gemini-3-flash-preview', @@ -67,6 +69,10 @@ final class DynamicConfig { @JsonKey() final bool enableGeminiLogAnalysis; + /// Flags related tp unified check-run flow configuration. + @JsonKey() + final UnifiedCheckRunFlow unifiedCheckRunFlow; + /// Flags related to ordered presubmit configuration. @JsonKey() final OrderedPresubmit orderedPresubmit; @@ -85,6 +91,7 @@ final class DynamicConfig { required this.contentAwareHashing, required this.closeMqGuardAfterPresubmit, required this.enableGeminiLogAnalysis, + required this.unifiedCheckRunFlow, required this.orderedPresubmit, required this.dynamicTestSuppression, required this.geminiModel, @@ -99,6 +106,7 @@ final class DynamicConfig { ContentAwareHashing? contentAwareHashing, bool? closeMqGuardAfterPresubmit, bool? enableGeminiLogAnalysis, + UnifiedCheckRunFlow? unifiedCheckRunFlow, OrderedPresubmit? orderedPresubmit, bool? dynamicTestSuppression, String? geminiModel, @@ -114,6 +122,8 @@ final class DynamicConfig { defaultInstance.closeMqGuardAfterPresubmit, enableGeminiLogAnalysis: enableGeminiLogAnalysis ?? defaultInstance.enableGeminiLogAnalysis, + unifiedCheckRunFlow: + unifiedCheckRunFlow ?? defaultInstance.unifiedCheckRunFlow, orderedPresubmit: orderedPresubmit ?? defaultInstance.orderedPresubmit, dynamicTestSuppression: dynamicTestSuppression ?? defaultInstance.dynamicTestSuppression, @@ -149,6 +159,13 @@ final class DynamicConfig { /// The inverse operation of [DynamicConfig.fromJson]. Map toJson() => _$DynamicConfigToJson(this); + bool isUnifiedCheckRunFlowEnabledForUser(String githubUsername) { + if (unifiedCheckRunFlow.useForAll) { + return true; + } + return unifiedCheckRunFlow.useForUsers.contains(githubUsername); + } + bool isOrderedPresubmitEnabledForUser(String githubUsername) { if (orderedPresubmit.useForAll) { return true; diff --git a/app_dart/lib/src/service/flags/dynamic_config.g.dart b/app_dart/lib/src/service/flags/dynamic_config.g.dart index a760949baf..d958c97c10 100644 --- a/app_dart/lib/src/service/flags/dynamic_config.g.dart +++ b/app_dart/lib/src/service/flags/dynamic_config.g.dart @@ -21,6 +21,11 @@ DynamicConfig _$DynamicConfigFromJson(Map json) => ), closeMqGuardAfterPresubmit: json['closeMqGuardAfterPresubmit'] as bool?, enableGeminiLogAnalysis: json['enableGeminiLogAnalysis'] as bool?, + unifiedCheckRunFlow: json['unifiedCheckRunFlow'] == null + ? null + : UnifiedCheckRunFlow.fromJson( + json['unifiedCheckRunFlow'] as Map?, + ), orderedPresubmit: json['orderedPresubmit'] == null ? null : OrderedPresubmit.fromJson( @@ -37,6 +42,7 @@ Map _$DynamicConfigToJson(DynamicConfig instance) => 'ciYaml': instance.ciYaml.toJson(), 'closeMqGuardAfterPresubmit': instance.closeMqGuardAfterPresubmit, 'enableGeminiLogAnalysis': instance.enableGeminiLogAnalysis, + 'unifiedCheckRunFlow': instance.unifiedCheckRunFlow.toJson(), 'orderedPresubmit': instance.orderedPresubmit.toJson(), 'dynamicTestSuppression': instance.dynamicTestSuppression, 'geminiModel': instance.geminiModel, diff --git a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart new file mode 100644 index 0000000000..85cfc47524 --- /dev/null +++ b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.dart @@ -0,0 +1,53 @@ +// Copyright 2025 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:json_annotation/json_annotation.dart'; +import 'package:meta/meta.dart'; + +part 'unified_check_run_flow_flags.g.dart'; + +/// Flags related to content-aware hashing. +@JsonSerializable() +@immutable +final class UnifiedCheckRunFlow { + /// Default configuration for [UnifiedCheckRunFlow] flags. + static const defaultInstance = UnifiedCheckRunFlow._( + useForAll: false, + useForUsers: [], + ); + + /// Whether to use unified check-run flow with only one check-run created + /// for all LUCI tests or github check-run flow. + @JsonKey() + final bool useForAll; + + /// List of users to use unified check-run flow. + @JsonKey() + final List useForUsers; + + const UnifiedCheckRunFlow._({ + required this.useForAll, // + required this.useForUsers, // + }); + + /// Creates [UnifiedCheckRunFlow] flags from the provided fields. + /// + /// Any omitted fields default to the values in [defaultInstance]. + factory UnifiedCheckRunFlow({bool? useForAll, List? useForUsers}) { + return UnifiedCheckRunFlow._( + useForAll: useForAll ?? defaultInstance.useForAll, + useForUsers: useForUsers ?? defaultInstance.useForUsers, + ); + } + + /// Creates [UnifiedCheckRunFlow] flags from a [json] object. + /// + /// Any omitted fields default to the values in [defaultInstance]. + factory UnifiedCheckRunFlow.fromJson(Map? json) { + return _$UnifiedCheckRunFlowFromJson(json ?? {}); + } + + /// The inverse operation of [UnifiedCheckRunFlow.fromJson]. + Map toJson() => _$UnifiedCheckRunFlowToJson(this); +} diff --git a/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart new file mode 100644 index 0000000000..7c8879fe3e --- /dev/null +++ b/app_dart/lib/src/service/flags/unified_check_run_flow_flags.g.dart @@ -0,0 +1,24 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: always_specify_types, implicit_dynamic_parameter + +part of 'unified_check_run_flow_flags.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +UnifiedCheckRunFlow _$UnifiedCheckRunFlowFromJson(Map json) => + UnifiedCheckRunFlow( + useForAll: json['useForAll'] as bool?, + useForUsers: (json['useForUsers'] as List?) + ?.map((e) => e as String) + .toList(), + ); + +Map _$UnifiedCheckRunFlowToJson( + UnifiedCheckRunFlow instance, +) => { + 'useForAll': instance.useForAll, + 'useForUsers': instance.useForUsers, +};