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/cocoon_service.dart b/app_dart/lib/cocoon_service.dart index e54286cac0..2fd0263f54 100644 --- a/app_dart/lib/cocoon_service.dart +++ b/app_dart/lib/cocoon_service.dart @@ -60,6 +60,7 @@ 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 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/model/common/presubmit_completed_check.dart b/app_dart/lib/src/model/common/presubmit_completed_check.dart index 8bafb12740..d7a3139bc6 100644 --- a/app_dart/lib/src/model/common/presubmit_completed_check.dart +++ b/app_dart/lib/src/model/common/presubmit_completed_check.dart @@ -34,6 +34,7 @@ class PresubmitCompletedJob { final int checkRunId; final int? checkSuiteId; final String? headBranch; + final bool isUnifiedCheckRun; final CiStage? stage; final int? prNum; final int attempt; @@ -52,6 +53,7 @@ class PresubmitCompletedJob { required this.checkRunId, required this.checkSuiteId, required this.headBranch, + required this.isUnifiedCheckRun, this.stage, this.prNum, this.attempt = 1, @@ -78,6 +80,7 @@ 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), @@ -100,7 +103,7 @@ class PresubmitCompletedJob { cocoon_checks.CheckRun get checkRun { return cocoon_checks.CheckRun( id: checkRunId, - name: Config.kDashboardCheckName, + name: isUnifiedCheckRun ? Config.kDashboardCheckName : name, headSha: sha, conclusion: status.toConclusion(), checkSuite: CheckSuite( @@ -118,11 +121,7 @@ class PresubmitCompletedJob { slug: slug, prNum: prNum ?? 0, checkRunId: checkRunId, - stage: - stage ?? - (slug == Config.flutterSlug - ? CiStage.fusionTests - : CiStage.genericTests), + stage: stage ?? CiStage.fusionTests, ); } diff --git a/app_dart/lib/src/request_handlers/presubmit_subscription.dart b/app_dart/lib/src/request_handlers/presubmit_subscription.dart index ad4c1b09a6..be629c233a 100644 --- a/app_dart/lib/src/request_handlers/presubmit_subscription.dart +++ b/app_dart/lib/src/request_handlers/presubmit_subscription.dart @@ -47,11 +47,13 @@ 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; @@ -141,16 +143,20 @@ 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 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 (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, + ), + ); + } final maxAttempt = await _getMaxAttempt( userData.commit, builderName, @@ -159,14 +165,17 @@ base class PresubmitSubscription extends SubscriptionHandler { if (tagSet.currentAttempt < maxAttempt) { rescheduled = true; log.info('Rerunning failed task: $builderName'); - await UnifiedCheckRun.reInitializeInProgressJob( - firestoreService: _firestore, - completedJob: PresubmitCompletedJob.fromBuild( - build, - userData, - summaryPrepend: '### ⚠️ Test failed but automatically rescheduled', - ), - ); + if (isUnifiedCheckRun) { + await UnifiedCheckRun.reInitializeInProgressJob( + firestoreService: _firestore, + completedJob: PresubmitCompletedJob.fromBuild( + build, + userData, + summaryPrepend: + '### ⚠️ Test failed but automatically rescheduled', + ), + ); + } await _luciBuildService.reschedulePresubmitBuild( builderName: builderName, build: build, @@ -190,6 +199,21 @@ 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/firestore/unified_check_run.dart b/app_dart/lib/src/service/firestore/unified_check_run.dart index b9b272f88a..b763671efc 100644 --- a/app_dart/lib/src/service/firestore/unified_check_run.dart +++ b/app_dart/lib/src/service/firestore/unified_check_run.dart @@ -36,10 +36,14 @@ final class UnifiedCheckRun { CheckRun? mergeQueueGuard, @visibleForTesting DateTime Function() utcNow = DateTime.timestamp, }) async { - if (dashboardChecks != null && pullRequest != null) { + if (dashboardChecks != null && + pullRequest != null && + config.flags.isUnifiedCheckRunFlowEnabledForUser( + pullRequest.user!.login!, + )) { // Create the presubmit_guard and associated presubmit_job documents. log.info( - 'Storing UnifiedCheckRun data for ${slug.fullName}#${pullRequest.number}.', + 'Storing UnifiedCheckRun data for ${slug.fullName}#${pullRequest.number} as it enabled for user ${pullRequest.user!.login}.', ); // 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 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, +}; diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index a26f0807e5..01bc943d1a 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -293,6 +293,8 @@ 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!, ); @@ -305,7 +307,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 (dashboardChecks != null) { + if (isUnifiedCheckRunFlow && dashboardChecks != null) { userData = PresubmitUserData( commit: CommitRef(slug: slug, sha: commitSha, branch: commitBranch), guardCheckRunId: dashboardChecks.id!, @@ -324,7 +326,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 (dashboardChecks == null) { + if (!isUnifiedCheckRunFlow || dashboardChecks == null) { final checkRun = await _githubChecksUtil.createCheckRun( _config, target.slug, @@ -394,7 +396,7 @@ class LuciBuildService { userData: userData, properties: properties, // if unified check run flow is enabled, use guard check run othervise check run id. - tags: dashboardChecks != null + tags: isUnifiedCheckRunFlow && dashboardChecks != null ? BuildTags([ GuardCheckRunIdBuildTag( guardCheckRunId: dashboardChecks.id!, @@ -447,7 +449,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 (dashboardChecks != null) { + if (isUnifiedCheckRunFlow && 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 d7550dbae8..ab984096d9 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -19,10 +19,12 @@ 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'; @@ -347,6 +349,10 @@ 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}', @@ -361,9 +367,12 @@ class Scheduler { final lockResult = await lockMergeGroupChecks( slug, sha, - detailsUrl: - 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${slug.name}&sha=$sha', - isPresubmit: true, + // 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, ); final dashboardChecks = lockResult.dashboardChecks; final mergeQueueGuard = lockResult.mergeQueueGuard; @@ -376,12 +385,11 @@ class Scheduler { log.info('Creating presubmit targets for ${pullRequest.number}'); Object? exception; - final isFlutterRepo = slug == Config.flutterSlug; - final isPackagesRepo = slug == Config.packagesSlug; + final isFusion = slug == Config.flutterSlug; + final isPackages = slug == Config.packagesSlug; do { try { - // If it's not flutter or packages, unlock the merge group lock. - if (!isFlutterRepo && !isPackagesRepo) { + if (!isFusion && !(isPackages && isUnifiedCheckRun)) { unlockMergeGroup = true; } @@ -425,7 +433,7 @@ class Scheduler { ); break; } - final presubmitTargets = isFlutterRepo + final presubmitTargets = isFusion ? await _getTestsForStage(pullRequest, CiStage.fusionEngineBuild) : await getPresubmitTargets(pullRequest); final presubmitTriggerTargets = filterTargets( @@ -433,45 +441,59 @@ 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: stage, - tasks: [...presubmitTriggerTargets.map((t) => t.name)], - pullRequest: pullRequest, - config: _config, - dashboardChecks: dashboardChecks, - mergeQueueGuard: mergeQueueGuard, - ); - // 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. - final engineArtifacts = isFlutterRepo - ? EngineArtifacts.usingExistingEngine(commitSha: sha) - : const EngineArtifacts.noFrameworkTests( - reason: 'This is not the flutter/flutter repository', - ); + 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, + ); + // 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, 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, + ); + } + engineArtifacts = 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: stage, + stage: isFusion ? CiStage.fusionEngineBuild : CiStage.genericTests, ); } on FormatException catch (e, s) { log.warn( @@ -507,9 +529,13 @@ class Scheduler { // there are situations (see code above) when it needs to be unlocked // immediately. if (unlockMergeGroup) { - await unlockCheckRun(slug, sha, dashboardChecks); - if (mergeQueueGuard != null) { - await unlockCheckRun(slug, sha, mergeQueueGuard); + 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( @@ -632,7 +658,7 @@ class Scheduler { final lockResult = await lockMergeGroupChecks( slug, headSha, - isPresubmit: false, + isUnifiedCheckRun: false, ); final dashboardChecks = lockResult.dashboardChecks; final mergeQueueGuard = lockResult.mergeQueueGuard!; @@ -640,7 +666,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 unlockCheckRun(slug, headSha, mergeQueueGuard); + await unlockMergeQueueGuard(slug, headSha, mergeQueueGuard); return; } @@ -845,7 +871,7 @@ $s RepositorySlug slug, String headSha, { String? detailsUrl, - required bool isPresubmit, + required bool isUnifiedCheckRun, }) async { final mergeQueueGuard = await _githubChecksService.githubChecksUtil .createCheckRun( @@ -857,7 +883,7 @@ $s title: Config.kMergeQueueLockName, summary: kMergeQueueLockDescription, ), - detailsUrl: isPresubmit ? null : detailsUrl, + detailsUrl: isUnifiedCheckRun ? null : detailsUrl, ); final dashboardChecks = await _githubChecksService.githubChecksUtil @@ -870,10 +896,10 @@ $s title: Config.kDashboardCheckName, summary: kDashboardChecksDescription, ), - detailsUrl: isPresubmit ? detailsUrl : null, + detailsUrl: isUnifiedCheckRun ? detailsUrl : null, ); - if (!isPresubmit) { + if (!isUnifiedCheckRun) { // Skip Dashboard Checks await _githubChecksService.githubChecksUtil.updateCheckRun( _config, @@ -948,7 +974,7 @@ $s /// /// If the guard is guarding a pull request, this immediately makes the pull /// request eligible for enqueuing into the merge queue. - Future unlockCheckRun( + Future unlockMergeQueueGuard( RepositorySlug slug, String headSha, CheckRun lock, @@ -1103,20 +1129,56 @@ 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}, $requestor, ${check.slug}, ${check.sha}, ${check.status})'; - - final stage = - check.stage ?? - (check.slug == Config.flutterSlug - ? CiStage.fusionTests - : CiStage.genericTests); - final stagingConclusion = await _markUnifiedCheckRunConclusion( - guardId: check.guardId, - state: check.state, - ); + 'checkCompleted(${check.name}, $flow, $requestor, ${check.slug}, ${check.sha}, ${check.status})'; + + final isFusion = check.slug == Config.flutterSlug; + if (!isFusion && !check.isUnifiedCheckRun) { + return true; + } + + 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(), + ); + + 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) { @@ -1172,7 +1234,7 @@ detailsUrl: $detailsUrl summary: stagingConclusion.summary, details: stagingConclusion.details, ); - } else { + } else if (check.isUnifiedCheckRun) { final guard = checkRunFromString(stagingConclusion.dashboardChecks!); final detailsUrl = 'https://flutter-dashboard.appspot.com/#/presubmit?repo=${check.slug.name}&sha=${check.sha}'; @@ -1221,7 +1283,6 @@ detailsUrl: $detailsUrl logCrumb: logCrumb, ); } - break; case CiStage.fusionTests: await _closeSuccessfulTestStage( dashboardChecks: stagingConclusion.dashboardChecks, @@ -1229,16 +1290,23 @@ detailsUrl: $detailsUrl slug: check.slug, sha: check.sha, logCrumb: logCrumb, + isUnifiedCheckRun: check.isUnifiedCheckRun, ); - break; case CiStage.genericTests: - await _closeSuccessfulTestStage( - dashboardChecks: stagingConclusion.dashboardChecks, - mergeQueueGuard: stagingConclusion.mergeQueueGuard, - slug: check.slug, - sha: check.sha, - logCrumb: logCrumb, - ); + 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.'); + } break; } return true; @@ -1296,13 +1364,32 @@ detailsUrl: $detailsUrl required RepositorySlug slug, required String sha, required String logCrumb, + required bool isUnifiedCheckRun, }) async { log.info('$logCrumb: Test stage completed'); - if (dashboardChecks != null) { - await unlockCheckRun(slug, sha, checkRunFromString(dashboardChecks)); - } - if (mergeQueueGuard != null) { - await unlockCheckRun(slug, sha, checkRunFromString(mergeQueueGuard)); + 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), + ); + } } } @@ -1341,7 +1428,7 @@ detailsUrl: $detailsUrl // Unlock the guarding check_run. final checkRunGuard = checkRunFromString(mergeQueueGuard); - await unlockCheckRun(slug, sha, checkRunGuard); + await unlockMergeQueueGuard(slug, sha, checkRunGuard); } /// Schedules post-engine build tests (i.e. engine tests, and framework tests). @@ -1508,6 +1595,37 @@ $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 c89ba0b4d7..a2b9de29da 100644 --- a/app_dart/test/model/common/presubmit_completed_check_test.dart +++ b/app_dart/test/model/common/presubmit_completed_check_test.dart @@ -50,11 +50,48 @@ 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 ef462c1f51..85f01d46f0 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,7 +96,9 @@ void main() { wrongBaseBranchPullRequestMessageValue: '{{target_branch}} -> {{default_branch}}', ); - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ); issuesService = MockIssuesService(); when( // ignore: discarded_futures @@ -147,8 +149,6 @@ void main() { any, any, output: anyNamed('output'), - conclusion: anyNamed('conclusion'), - detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((_) async { return CheckRun.fromJson(const { @@ -2792,23 +2792,13 @@ void foo() { }); test('Tries to schedule tests for a duplicate SHA warns', () async { - final pr = generatePullRequest( - number: 1, - headSha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c', - ); - final checkRunGuard = generateCheckRun( - 1, - name: Config.kDashboardCheckName, - ); - await UnifiedCheckRun.initializeCiStagingDocument( + await CiStaging.initializeDocument( firestoreService: firestore, slug: Config.flutterSlug, sha: '66d6bd9a3f79a36fe4f5178ccefbc781488a596c', stage: CiStage.fusionEngineBuild, tasks: [], - config: config, - pullRequest: pr, - dashboardChecks: checkRunGuard, + checkRunGuard: '', ); config.maxFilesChangedForSkippingEnginePhaseValue = 1; await testActions( 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 59a5e5c41e..058cc17668 100644 --- a/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_luci_subscription_test.dart @@ -2,7 +2,6 @@ // 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'; import 'package:cocoon_integration_test/testing.dart'; @@ -79,6 +78,15 @@ 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); @@ -102,19 +110,28 @@ 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 { - buildBucketClient.getBuildResponse = Future.value( - bbv2.Build( - id: Int64(1), - builder: bbv2.BuilderID(builder: 'Linux A'), - status: bbv2.Status.FAILURE, - summaryMarkdown: 'test summary', + 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); @@ -153,27 +170,27 @@ 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 { - 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'), - jobName: 'Linux presubmit_max_attempts=2', - checkRunId: 1, - creationTime: 12345, - attemptNumber: 1, + when( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + rescheduled: true, ), - ); + ).thenAnswer((_) async => true); tester.message = createPushMessage( Int64(1), @@ -191,19 +208,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'), @@ -215,14 +232,7 @@ void main() { ).thenAnswer((_) async => true); when( mockLuciBuildService.getBuildById(any, buildMask: anyNamed('buildMask')), - ).thenAnswer( - (_) async => bbv2.Build( - id: Int64(1), - builder: bbv2.BuilderID(builder: 'Linux A'), - status: bbv2.Status.INFRA_FAILURE, - summaryMarkdown: 'test summary', - ), - ); + ).thenAnswer((_) async => bbv2.Build(summaryMarkdown: 'test summary')); tester.message = createPushMessage( Int64(1), @@ -280,18 +290,19 @@ 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)); }); 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'), @@ -341,19 +352,20 @@ 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); }); 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'), @@ -402,6 +414,15 @@ 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); }); @@ -435,15 +456,6 @@ 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'), @@ -453,22 +465,9 @@ 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 => fullBuild); + ).thenAnswer((_) async => bbv2.Build(summaryMarkdown: 'test summary')); tester.message = createPushMessage( Int64(1), @@ -585,6 +584,91 @@ 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( @@ -723,6 +807,49 @@ 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() @@ -879,7 +1006,14 @@ void main() { expect(response, Response.emptyOk); expect(pubSub.topics, isEmpty); - verify(mockScheduler.processCheckRunCompleted(any)).called(1); + verify( + mockGithubChecksService.updateCheckStatus( + build: anyNamed('build'), + checkRunId: anyNamed('checkRunId'), + luciBuildService: anyNamed('luciBuildService'), + slug: anyNamed('slug'), + ), + ).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 13efb12e0d..b0eee7f8c4 100644 --- a/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart +++ b/app_dart/test/request_handlers/presubmit_ordered_subscription_test.dart @@ -105,6 +105,14 @@ 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 2a8ad462de..f6b486ace7 100644 --- a/app_dart/test/service/firestore/unified_check_run_test.dart +++ b/app_dart/test/service/firestore/unified_check_run_test.dart @@ -48,7 +48,11 @@ void main() { group('initializeCiStagingDocument', () { test('creates PresubmitGuard and Checks when enabled for user', () async { - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig.fromJson({ + 'unifiedCheckRunFlow': { + 'useForUsers': ['dash'], + }, + }); await UnifiedCheckRun.initializeCiStagingDocument( firestoreService: firestoreService, @@ -83,6 +87,37 @@ 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', () { @@ -689,6 +724,7 @@ 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 86692dbbef..e8caad7bcd 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,6 +16,7 @@ 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'; @@ -419,7 +420,11 @@ void main() { // Enable Unified Check Run Flow luci = LuciBuildService( - config: FakeConfig(dynamicConfig: DynamicConfig()), + config: FakeConfig( + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), + ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, githubChecksUtil: mockGithubChecksUtil, @@ -487,7 +492,11 @@ void main() { // Enable Unified Check Run Flow but provide NO guard luci = LuciBuildService( - config: FakeConfig(dynamicConfig: DynamicConfig()), + config: FakeConfig( + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), + ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, githubChecksUtil: mockGithubChecksUtil, @@ -545,6 +554,79 @@ 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 { @@ -571,7 +653,9 @@ void main() { luci = LuciBuildService( config: FakeConfig( githubClient: mockGithubClient, - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, @@ -646,7 +730,9 @@ void main() { luci = LuciBuildService( config: FakeConfig( githubClient: mockGithubClient, - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ), ), cache: CacheService.inMemory(), buildBucketClient: mockBuildBucketClient, diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index e67bab0db5..8cfd86a878 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -76,9 +76,10 @@ void main() { Config.flutterSlug, Config.packagesSlug, }, - maxFilesChangedForSkippingEnginePhaseValue: 0, ); - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ); fakeContentAwareHash = FakeContentAwareHashService(config: config); @@ -92,8 +93,6 @@ void main() { any, any, output: anyNamed('output'), - conclusion: anyNamed('conclusion'), - detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((Invocation invocation) async { return generateCheckRun( @@ -674,7 +673,9 @@ void main() { final mockGithubClient = MockGitHub(); config = FakeConfig( githubService: mockGithubService, - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ), ); scheduler = Scheduler( githubService: config.githubService ?? FakeGithubService(), @@ -727,7 +728,6 @@ void main() { any, any, output: anyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((_) async { return CheckRun.fromJson(const { @@ -757,16 +757,9 @@ void main() { output: anyNamed('output'), ), ); - // Verifies Dashboard Checks was created + // Verfies Linux A was created verify( - mockGithubChecksUtil.createCheckRun( - any, - any, - any, - Config.kDashboardCheckName, - output: anyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), - ), + mockGithubChecksUtil.createCheckRun(any, any, any, any), ).called(1); }); @@ -1624,6 +1617,15 @@ 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( @@ -1636,14 +1638,584 @@ 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 { @@ -1816,6 +2388,421 @@ 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 }); }); @@ -1909,7 +2896,6 @@ targets: any, captureAny, output: captureAnyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -1929,6 +2915,9 @@ 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 ], ); }); @@ -1937,7 +2926,9 @@ targets: 'creates presubmit_guard document for flutter/packages when unified check run flow is enabled', () async { getFilesChanged.cannedFiles = ['README.md']; - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ); when( mockGithubChecksUtil.createCheckRun( @@ -1996,7 +2987,9 @@ targets: 'unlocks merge group for cocoon when unified check run flow is enabled', () async { getFilesChanged.cannedFiles = ['README.md']; - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ); when( mockGithubChecksUtil.createCheckRun( @@ -2063,7 +3056,7 @@ targets: final lockResult = await scheduler.lockMergeGroupChecks( Config.flutterSlug, 'sha123', - isPresubmit: true, + isUnifiedCheckRun: true, ); expect(lockResult.dashboardChecks.name, Config.kDashboardCheckName); @@ -2157,7 +3150,9 @@ targets: final fakeConfig = FakeConfig( githubService: mockGithubService, githubClient: MockGitHub(), - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -2192,7 +3187,6 @@ targets: any, captureAny, output: captureAnyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -2212,12 +3206,13 @@ 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'])); }, ); @@ -2238,7 +3233,6 @@ targets: any, captureAny, output: captureAnyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).captured, [ @@ -2282,7 +3276,14 @@ targets: output: anyNamed('output'), ), ).captured, - [CheckRunStatus.completed, CheckRunConclusion.success], + [ + CheckRunStatus.completed, + CheckRunConclusion.success, + CheckRunStatus.completed, + CheckRunConclusion.success, + CheckRunStatus.completed, + CheckRunConclusion.success, + ], ); }); @@ -2313,6 +3314,11 @@ targets: await scheduler.triggerPresubmitTargets(pullRequest: pullRequest); expect(capturedUpdates, <(String, CheckRunStatus, CheckRunConclusion)>[ + ( + Config.kDashboardCheckName, + CheckRunStatus.completed, + CheckRunConclusion.success, + ), ( 'ci.yaml validation', CheckRunStatus.completed, @@ -2335,7 +3341,12 @@ targets: output: anyNamed('output'), ), ).captured, - [CheckRunStatus.completed, CheckRunConclusion.failure], + [ + CheckRunStatus.completed, + CheckRunConclusion.success, + CheckRunStatus.completed, + CheckRunConclusion.failure, + ], ); }); @@ -2433,7 +3444,6 @@ targets: any, any, output: anyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).thenAnswer((inv) async { final slug = inv.positionalArguments[1] as RepositorySlug; @@ -2457,7 +3467,9 @@ targets: githubService: mockGithubService, githubClient: MockGitHub(), maxFilesChangedForSkippingEnginePhaseValue: 0, - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -2482,7 +3494,6 @@ targets: any, captureAny, output: captureAnyNamed('output'), - detailsUrl: anyNamed('detailsUrl'), ), ).captured; stdout.writeAll(results); @@ -2508,7 +3519,7 @@ targets: mockGithubChecksUtil.updateCheckRun( any, Config.flutterSlug, - checkRuns[2], + checkRuns[1], status: argThat(equals(CheckRunStatus.completed), named: 'status'), conclusion: argThat( equals(CheckRunConclusion.success), @@ -2528,16 +3539,6 @@ targets: output: anyNamed('output'), ), ); - verifyNever( - mockGithubChecksUtil.updateCheckRun( - any, - Config.flutterSlug, - checkRuns[1], - status: anyNamed('status'), - conclusion: anyNamed('conclusion'), - output: anyNamed('output'), - ), - ); }); }); @@ -3125,7 +4126,9 @@ targets: githubService: mockGithubService, githubClient: MockGitHub(), maxFilesChangedForSkippingEnginePhaseValue: 29, - dynamicConfig: DynamicConfig(), + dynamicConfig: DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: false), + ), ); scheduler = Scheduler( githubService: fakeConfig.githubService ?? FakeGithubService(), @@ -3207,18 +4210,18 @@ targets: 'Linux analyze', ], reason: 'Should skip Linux engine_build'); - 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, - }); + expect( + firestore, + existsInStorage(CiStaging.metadata, [ + isCiStaging + .hasStage(CiStage.fusionEngineBuild) + .hasCheckRuns(isEmpty), + isCiStaging.hasStage(CiStage.fusionTests).hasCheckRuns({ + 'Linux A': TaskConclusion.scheduled, + 'Linux analyze': TaskConclusion.scheduled, + }), + ]), + ); }); // Regression test for https://github.com/flutter/flutter/issues/167124. @@ -3303,7 +4306,9 @@ targets: // Enable fusion ciYamlFetcher.setCiYamlFrom(singleCiYaml, engine: fusionCiYaml); - config.dynamicConfig = DynamicConfig(); + config.dynamicConfig = DynamicConfig( + unifiedCheckRunFlow: UnifiedCheckRunFlow(useForAll: true), + ); final userData = PresubmitUserData( commit: CommitRef( diff --git a/cipd_packages/device_doctor/lib/src/health.dart b/cipd_packages/device_doctor/lib/src/health.dart index 0ea948f9d7..1b658803bc 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.Platform(), + platform.Platform pl = const platform.LocalPlatform(), String infraDialog = 'infra-dialog', }) async { var dialogDir = dir(path.dirname(Platform.script.path), 'tool', infraDialog); @@ -36,16 +36,15 @@ 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.nativePlatform!.environment['FLUTTER_XCODE_CODE_SIGN_STYLE'] != - null) { + if (pl.environment['FLUTTER_XCODE_CODE_SIGN_STYLE'] != null) { command.add( - "CODE_SIGN_STYLE=${pl.nativePlatform!.environment['FLUTTER_XCODE_CODE_SIGN_STYLE']}", + "CODE_SIGN_STYLE=${pl.environment['FLUTTER_XCODE_CODE_SIGN_STYLE']}", ); command.add( - "DEVELOPMENT_TEAM=${pl.nativePlatform!.environment['FLUTTER_XCODE_DEVELOPMENT_TEAM']}", + "DEVELOPMENT_TEAM=${pl.environment['FLUTTER_XCODE_DEVELOPMENT_TEAM']}", ); command.add( - "PROVISIONING_PROFILE_SPECIFIER=${pl.nativePlatform!.environment['FLUTTER_XCODE_PROVISIONING_PROFILE_SPECIFIER']}", + "PROVISIONING_PROFILE_SPECIFIER=${pl.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 a88820019a..fff64d374a 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 Platform(), + this.platform = const LocalPlatform(), }) : 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.nativePlatform!.environment['HOME']; + final home = platform.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 9495be2fad..50954eeea8 100644 --- a/cipd_packages/device_doctor/test/src/health_test.dart +++ b/cipd_packages/device_doctor/test/src/health_test.dart @@ -8,7 +8,6 @@ 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'; @@ -37,7 +36,7 @@ void main() { when( pm.start(any, workingDirectory: anyNamed('workingDirectory')), ).thenAnswer((_) => Future.value(proc)); - final platform.Platform pl = TestPlatform.native( + final platform.Platform pl = platform.FakePlatform( 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 4387ec3037..4c91f6536a 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/testing.dart'; +import 'package:platform/platform.dart'; import 'package:test/test.dart'; @@ -102,9 +102,8 @@ Future main() async { logger = TestLogger(); fs = MemoryFileSystem(); fs.directory(xcworkspacePath).createSync(recursive: true); - platform = TestPlatform.native( - environment: {'HOME': '/User/username'}, - ); + platform = MockPlatform(); + platform.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 4d15aebc17..d308810d01 100644 --- a/cipd_packages/device_doctor/test/src/utils.dart +++ b/cipd_packages/device_doctor/test/src/utils.dart @@ -8,8 +8,14 @@ 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( 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 fc81f5acee..9b6601b284 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,11 +469,8 @@ abstract base class _FakeInMemoryFirestoreService 'simulate a backend failure.', ); } - final statusCode = result.any((r) => r.code == 9) - ? HttpStatus.conflict - : 500; throw DetailedApiRequestError( - statusCode, + 500, 'The transaction was aborted:\n' '${result.where((r) => r.code != 0).map((r) => r.message).join('\n')}', ); 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 85ff525abd..8d4568ae7c 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? isPresubmit, + required bool? isUnifiedCheckRun, }) => (super.noSuchMethod( Invocation.method( #lockMergeGroupChecks, [slug, headSha], - {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isPresubmit}, + {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isUnifiedCheckRun}, ), returnValue: _i13.Future<_i16.CheckRunLockResult>.value( _FakeCheckRunLockResult_58( @@ -5745,7 +5745,10 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { Invocation.method( #lockMergeGroupChecks, [slug, headSha], - {#detailsUrl: detailsUrl, #isUnifiedCheckRun: isPresubmit}, + { + #detailsUrl: detailsUrl, + #isUnifiedCheckRun: isUnifiedCheckRun, + }, ), ), ), @@ -5782,7 +5785,7 @@ class MockScheduler extends _i1.Mock implements _i16.Scheduler { as _i13.Future); @override - _i13.Future unlockCheckRun( + _i13.Future unlockMergeQueueGuard( _i7.RepositorySlug? slug, String? headSha, _i7.CheckRun? lock, 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 043f3d22b1..e0d149ee67 100644 --- a/packages/cocoon_integration_test/test/fake_firestore_service_test.dart +++ b/packages/cocoon_integration_test/test/fake_firestore_service_test.dart @@ -2,8 +2,6 @@ // 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'; @@ -526,7 +524,7 @@ void main() { isA().having( (e) => e.status, 'status', - HttpStatus.conflict, + 500, ), ), );