From 5dd2c727fa300c978121bfc8b0dc20e3f34d34d9 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 27 Nov 2024 12:54:44 -0800 Subject: [PATCH 01/10] WIP; schedule merge queue targets --- app_dart/.gitignore | 2 + .../github/webhook_subscription.dart | 18 +- .../lib/src/service/luci_build_service.dart | 165 +++++++++++++++++- app_dart/lib/src/service/scheduler.dart | 141 +++++++++++++-- 4 files changed, 310 insertions(+), 16 deletions(-) diff --git a/app_dart/.gitignore b/app_dart/.gitignore index 3c8a15727..0bb411c69 100644 --- a/app_dart/.gitignore +++ b/app_dart/.gitignore @@ -4,3 +4,5 @@ # Conventional directory for build output. build/ + +coverage/ diff --git a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart index 6e18686d7..4cac8f4a0 100644 --- a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart +++ b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart @@ -224,6 +224,7 @@ class GithubWebhookSubscription extends SubscriptionHandler { final mergeGroupEvent = MergeGroupEvent.fromJson(request); final MergeGroupEvent(:mergeGroup, :action) = mergeGroupEvent; final headSha = mergeGroup.headSha; + final slug = mergeGroupEvent.repository!.slug(); // See the API reference: // https://docs.github.com/en/webhooks/webhook-events-and-payloads#merge_group @@ -234,24 +235,33 @@ class GithubWebhookSubscription extends SubscriptionHandler { // into the main branch. Cocoon should kick off CI jobs needed to verify // the PR group. case 'checks_requested': - log.fine('Simulating checks requests for merge queue @ $headSha'); + log.fine('Checks requests for merge queue @ $headSha'); + + if (!await _shaExistsInGob(slug, headSha)) { + throw InternalServerError( + '$slug/$headSha was not found on GoB. Failing so this event can be retried...', + ); + } + log.fine('$slug/$headSha was found on GoB mirror. Scheduling merge group tasks...'); await scheduler.triggerMergeGroupTargets(mergeGroupEvent: mergeGroupEvent); - break; // A merge group was deleted. This can happen when a PR is pulled from the // merge queue. All CI jobs pertaining to this merge group should be // stopped to save CI resources, as Github will no longer merge this group // into the main branch. case 'destroyed': - log.fine('Simulating destruction of a merge group @ $headSha'); + log.fine('Destroying the merge group for $slug/$headSha'); await scheduler.cancelMergeGroupTargets(headSha: headSha); - break; } } Future _commitExistsInGob(PullRequest pr) async { final RepositorySlug slug = pr.base!.repo!.slug(); final String sha = pr.mergeCommitSha!; + return _shaExistsInGob(slug, sha); + } + + Future _shaExistsInGob(RepositorySlug slug, String sha) async { final GerritCommit? gobCommit = await gerritService.findMirroredCommit(slug, sha); return gobCommit != null; } diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 7ac3bc1e3..7eb63d54e 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -774,6 +774,62 @@ class LuciBuildService { return >[]; } + /// Schedules [targets] for building of prod artifacts while in a merge queue. + Future scheduleMergeGroupBuilds({ + required Commit commit, + required List targets, + }) async { + final List buildRequests = []; + + Set availableBuilderSet; + try { + availableBuilderSet = await getAvailableBuilderSet( + project: 'flutter', + bucket: 'prod', + ); + } catch (error) { + throw 'Failed to get buildbucket builder list due to $error'; + } + log.info('Available builder list: $availableBuilderSet'); + for (var target in targets) { + // Non-existing builder target will be skipped from scheduling. + if (!availableBuilderSet.contains(target.value.name)) { + log.warning( + 'Found no available builder for ${target.value.name}, commit ${commit.sha}', + ); + continue; + } + log.info( + 'create postsubmit schedule request for target: ${target.value} in commit ${commit.sha}', + ); + + final scheduleBuildRequest = await _createMergeGroupScheduleBuild( + commit: commit, + target: target, + ); + buildRequests.add(bbv2.BatchRequest_Request(scheduleBuild: scheduleBuildRequest)); + log.info( + 'created postsubmit schedule request for target: ${target.value} in commit ${commit.sha}', + ); + } + + final bbv2.BatchRequest batchRequest = bbv2.BatchRequest(requests: buildRequests); + log.fine(batchRequest); + List messageIds; + + try { + messageIds = await pubsub.publish( + 'cocoon-scheduler-requests', + batchRequest.toProto3Json(), + ); + log.info('Published $messageIds for commit ${commit.sha}'); + } catch (error) { + log.severe('Failed to publish message to pub/sub due to $error'); + rethrow; + } + log.info('Published a request with ${buildRequests.length} builds'); + } + /// Create a Presubmit ScheduleBuildRequest using the [slug], [sha], and /// [checkName] for the provided [build] with the provided [checkRunId]. bbv2.ScheduleBuildRequest _createPresubmitScheduleBuild({ @@ -991,6 +1047,112 @@ class LuciBuildService { ); } + /// Creates a build request for a commit in a merge queue which will notify + /// presubmit channels. + Future _createMergeGroupScheduleBuild({ + required Commit commit, + required Target target, + Map? properties, + List? tags, + int priority = kDefaultPriority, + }) async { + log.info( + 'Creating merge group schedule builder for ${target.value.name} on commit ${commit.sha}', + ); + tags ??= []; + tags.addAll([ + bbv2.StringPair( + key: 'buildset', + value: 'commit/git/${commit.sha}', + ), + bbv2.StringPair( + key: 'buildset', + value: 'commit/gitiles/flutter.googlesource.com/mirrors/${commit.slug.name}/+/${commit.sha}', + ), + ]); + + log.info( + 'Scheduling builder: ${target.value.name} for commit ${commit.sha}', + ); + + final Map rawUserData = {}; + + await createPostsubmitCheckRun( + commit, + target, + rawUserData, + ); + + tags.add( + bbv2.StringPair( + key: 'user_agent', + value: 'flutter-cocoon', + ), + ); + // Tag `scheduler_job_id` is needed when calling buildbucket search build API. + tags.add( + bbv2.StringPair( + key: 'scheduler_job_id', + value: 'flutter/${target.value.name}', + ), + ); + // Default attempt is the initial attempt, which is 1. + final bbv2.StringPair? attemptTag = tags.singleWhereOrNull((tag) => tag.key == 'current_attempt'); + if (attemptTag == null) { + tags.add( + bbv2.StringPair( + key: 'current_attempt', + value: '1', + ), + ); + } + + final String currentAttemptStr = tags.firstWhere((tag) => tag.key == 'current_attempt').value; + rawUserData['firestore_task_document_name'] = '${commit.sha}_${target.value.name}_$currentAttemptStr'; + + final Map processedProperties = target.getProperties().cast(); + processedProperties.addAll(properties ?? {}); + processedProperties['git_branch'] = commit.branch!; + final String cipdExe = 'refs/heads/${commit.branch}'; + processedProperties['exe_cipd_version'] = cipdExe; + + final bbv2.Struct propertiesStruct = bbv2.Struct.create(); + propertiesStruct.mergeFromProto3Json(processedProperties); + + final List requestedDimensions = target.getDimensions(); + + final bbv2.Executable executable = bbv2.Executable(cipdVersion: cipdExe); + + log.info( + 'Constructing the merge group schedule build request for ${target.value.name} on commit ${commit.sha}.', + ); + + return bbv2.ScheduleBuildRequest( + builder: bbv2.BuilderID( + project: 'flutter', + bucket: target.getBucket(), + builder: target.value.name, + ), + dimensions: requestedDimensions, + exe: executable, + gitilesCommit: bbv2.GitilesCommit( + project: 'mirrors/${commit.slug.name}', + host: 'flutter.googlesource.com', + ref: 'refs/heads/${commit.branch}', + id: commit.sha, + ), + notify: bbv2.NotificationConfig( + // IMPORTANT: We're not post-submit yet, so we want to handle updates to + // the MQ differently. + pubsubTopic: 'projects/flutter-dashboard/topics/build-bucket-presubmit', + userData: UserData.encodeUserDataToBytes(rawUserData), + ), + tags: tags, + properties: propertiesStruct, + priority: priority, + ); + } + /// Creates postsubmit check runs for prod targets in supported repositories. Future createPostsubmitCheckRun( Commit commit, @@ -998,7 +1160,8 @@ class LuciBuildService { Map rawUserData, ) async { // We are not tracking this check run in the PrCheckRuns firestore doc because - // there is no PR to track here. + // there is no PR to look up later. The check run is important because it + // informs the staging document setup for Merge Groups in triggerMergeGroupTargets. final github.CheckRun checkRun = await githubChecksUtil.createCheckRun( config, target.slug, diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 6d83efe0f..5af5ccf08 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -456,7 +456,7 @@ class Scheduler { } // Update validate ci.yaml check - await closeCiYamlCheckRun(pullRequest, exception, slug, ciValidationCheckRun); + await closeCiYamlCheckRun('PR ${pullRequest.number}', exception, slug, ciValidationCheckRun); // The 'lock' will be unlocked later in processCheckRunCompletion after all engine builds are processed. if (!isFusion) { @@ -468,15 +468,15 @@ class Scheduler { } Future closeCiYamlCheckRun( - PullRequest pullRequest, + String description, exception, RepositorySlug slug, CheckRun ciValidationCheckRun, ) async { - log.info('Updating ci.yaml validation check for ${pullRequest.number}'); + log.info('Updating ci.yaml validation check for $description'); if (exception == null) { // Success in validating ci.yaml - log.info('ci.yaml validation check was successful for ${pullRequest.number}'); + log.info('ci.yaml validation check was successful for $description'); await githubChecksService.githubChecksUtil.updateCheckRun( config, slug, @@ -485,8 +485,7 @@ class Scheduler { conclusion: CheckRunConclusion.success, ); } else { - log.warning('Marking PR #${pullRequest.number} $kCiYamlCheckName as failed'); - log.warning(exception.toString()); + log.warning('Marking $description $kCiYamlCheckName as failed', e); // Failure when validating ci.yaml await githubChecksService.githubChecksUtil.updateCheckRun( config, @@ -523,12 +522,29 @@ class Scheduler { Future triggerMergeGroupTargets({ required cocoon_checks.MergeGroupEvent mergeGroupEvent, }) async { + /* + Behave similar to addPullRequest, except we're not yet merged into master. + - We are mirrored in to GoB + - We want PROD builds + - We want check_runs as well + - this might mean we want a different pubsub? + + We don't need "Task" objects because I beleive these are used by flutter-dashboard, + we're tracking with github. So really we need _batchScheduleBuilds + + batchSCheduleBuilds just calls schedulePostsubmitBuilds - which creates but doesn't + return the check run? + + CODEFU - left off here. + */ final mergeGroup = mergeGroupEvent.mergeGroup; final headSha = mergeGroup.headSha; + final slug = mergeGroupEvent.repository!.slug(); + final isFusion = await fusionTester.isFusionBasedRef(slug, headSha); - log.info('Simulating merge group checks for @ $headSha'); + final logCrumb = 'triggerMergeGroupTargets($slug, $headSha, ${isFusion ? 'simulated' : 'real'})'; - final slug = mergeGroupEvent.repository!.slug(); + log.info('$logCrumb: Scheduling merge group checks'); final lock = await lockMergeGroupChecks(slug, headSha); @@ -536,13 +552,117 @@ class Scheduler { config, slug, headSha, - 'Simulated merge queue check', + 'Merge queue check', output: const CheckRunOutput( - title: 'Simulated merge queue check', + title: 'Merge queue check', summary: 'If this check is stuck pending, push an empty commit to retrigger the checks', ), ); + if (!isFusion) { + await simulateMergeGroupUnlock(mergeGroup, slug, ciValidationCheckRun, headSha, lock); + return; + } + + final mergeGroupTargets = await getMergeGroupTargetsForStage( + mergeGroup.baseRef, + slug, + headSha, + CiStage.fusionEngineBuild, + ); + + Object? exception; + try { + // Create the staging doc that will track our engine progress and allow us to unlock + // the merge group lock later. + await initializeCiStagingDocument( + firestoreService: firestoreService, + slug: slug, + sha: headSha, + stage: CiStage.fusionEngineBuild, + tasks: [...mergeGroupTargets.map((t) => t.value.name)], + checkRunGuard: '$lock', + ); + + // Create the minimal Commit needed to pass the next stage. + // Note: headRef encodes refs/heads/... and what we want is the branch + final commit = Commit( + branch: mergeGroup.headRef.substring('refs/heads/'.length), + repository: slug.fullName, + sha: headSha, + ); + + await luciBuildService.scheduleMergeGroupBuilds( + targets: mergeGroupTargets, + commit: commit, + ); + } catch (e, s) { + log.warning('$logCrumb: error encountered when scheduling presubmit targets', e, s); + exception = e; + } + + await closeCiYamlCheckRun('MQ $slug/$headSha', exception, slug, ciValidationCheckRun); + + // Do not unlock the merge group `lock` - that will be done by staging checks. + + log.info('$logCrumb: Finished merge group checks'); + } + + Future> getMergeGroupTargetsForStage( + String baseRef, + RepositorySlug slug, + String headSha, + CiStage stage, + ) async { + final mergeGroupTargets = [ + ...await getMergeGroupTargets(baseRef, slug, headSha), + ...await getMergeGroupTargets(baseRef, slug, headSha, type: CiType.fusionEngine), + ].where( + (Target target) => switch (stage) { + CiStage.fusionEngineBuild => target.value.properties['release_build'] == 'true', + CiStage.fusionTests => target.value.properties['release_build'] != 'true' + }, + ); + + return [...mergeGroupTargets]; + } + + Future> getMergeGroupTargets( + String baseRef, + RepositorySlug slug, + String headSha, { + CiType type = CiType.any, + }) async { + log.info('Attempting to read merge group targets from ci.yaml for $headSha'); + + final Commit commit = Commit( + branch: baseRef, + repository: slug.fullName, + sha: headSha, + ); + final ciYaml = await getCiYaml(commit, validate: true); + log.info('ci.yaml loaded successfully.'); + log.info('Collecting merge group targets for $headSha'); + final inner = ciYaml.ciYamlFor(type); + final List postSubmitTargets = [ + ...inner.postsubmitTargets.where( + (Target target) => + target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon, + ), + ]; + + log.info('Collected ${postSubmitTargets.length} merge group targets.'); + return postSubmitTargets; + } + + // Pretend the check took 1 minute to run + Future simulateMergeGroupUnlock( + cocoon_checks.MergeGroup mergeGroup, + RepositorySlug slug, + CheckRun ciValidationCheckRun, + String headSha, + CheckRun lock, + ) async { // Pretend the check took 1 minute to run await Future.delayed(debugCheckPretendDelay); @@ -563,7 +683,6 @@ class Scheduler { lock, conclusion == CheckRunConclusion.success ? null : 'Some checks failed', ); - log.info('Finished Simulating merge group checks for @ $headSha'); } From 594c4e1209ecd16835552c33a004e2fae7077978 Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 27 Nov 2024 13:26:51 -0800 Subject: [PATCH 02/10] Tests --- app_dart/lib/src/service/scheduler.dart | 2 +- .../github/webhook_subscription_test.dart | 46 ++++++++++++------- .../test/src/service/fake_gerrit_service.dart | 6 +-- .../test/src/utilities/entity_generators.dart | 6 +-- 4 files changed, 37 insertions(+), 23 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 5af5ccf08..a2871246d 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -542,7 +542,7 @@ class Scheduler { final slug = mergeGroupEvent.repository!.slug(); final isFusion = await fusionTester.isFusionBasedRef(slug, headSha); - final logCrumb = 'triggerMergeGroupTargets($slug, $headSha, ${isFusion ? 'simulated' : 'real'})'; + final logCrumb = 'triggerMergeGroupTargets($slug, $headSha, ${isFusion ? 'real' : 'simulated'})'; log.info('$logCrumb: Scheduling merge group checks'); 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 0c44eda29..9c4e6b0c9 100644 --- a/app_dart/test/request_handlers/github/webhook_subscription_test.dart +++ b/app_dart/test/request_handlers/github/webhook_subscription_test.dart @@ -28,6 +28,7 @@ import '../../src/service/fake_fusion_tester.dart'; import '../../src/service/fake_github_service.dart'; import '../../src/service/fake_gerrit_service.dart'; import '../../src/service/fake_scheduler.dart'; +import '../../src/utilities/entity_generators.dart'; import '../../src/utilities/mocks.dart'; import '../../src/utilities/webhook_generators.dart'; @@ -96,8 +97,12 @@ void main() { fakeFusionTester = FakeFusionTester(); fakeFusionTester.isFusion = (_, __) => false; mockGithubChecksUtil = MockGithubChecksUtil(); - scheduler = - FakeScheduler(config: config, buildbucket: fakeBuildBucketClient, githubChecksUtil: mockGithubChecksUtil); + scheduler = FakeScheduler( + config: config, + buildbucket: fakeBuildBucketClient, + githubChecksUtil: mockGithubChecksUtil, + fusionTester: fakeFusionTester, + ); tester = SubscriptionTester(request: request); when(gitHubClient.issues).thenReturn(issuesService); @@ -264,8 +269,8 @@ void main() { number: issueNumber, baseRef: 'dev', merged: true, - baseSha: 'sha1', - mergeCommitSha: 'sha2', + baseSha: 'abc', + mergeCommitSha: 'cde', // Just spelling this out here, because this test specifically tests a // non-revert PR. @@ -289,8 +294,8 @@ void main() { number: issueNumber, baseRef: 'dev', merged: true, - baseSha: 'sha1', - mergeCommitSha: 'sha2', + baseSha: 'abc', + mergeCommitSha: 'cde', withRevertOf: true, headRef: 'test/headref', ); @@ -2202,8 +2207,8 @@ void foo() { action: 'closed', number: issueNumber, merged: true, - baseSha: 'sha1', // Found in pre-populated commits in FakeGerritService. - mergeCommitSha: 'sha2', + baseSha: 'abc', // Found in pre-populated commits in FakeGerritService. + mergeCommitSha: 'cde', ); expect(db.values.values.whereType().length, 0); @@ -2609,7 +2614,13 @@ void foo() { Scheduler.debugCheckPretendDelay = Duration.zero; }); - test('checks_requested success', () async { + setUp(() { + gerritService.commitsValue = [ + generateGerritCommit('c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', 1), + ]; + }); + + test('checks_requested success for non-fusion repository (simulated)', () async { final records = []; final subscription = log.onRecord.listen((record) { if (record.level >= Level.FINE) { @@ -2621,6 +2632,7 @@ void foo() { action: 'checks_requested', message: 'Implement an amazing feature', ); + await tester.post(webhook); await subscription.cancel(); @@ -2629,7 +2641,7 @@ void foo() { any, any, 'c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulated merge queue check', + 'Merge queue check', output: anyNamed('output'), ), ).called(1); @@ -2649,8 +2661,9 @@ void foo() { [ 'Processing merge_group', 'Processing checks_requested for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulating checks requests for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulating merge group checks for @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', + 'Checks requests for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', + 'flutter/flutter/c9affbbb12aa40cb3afbe94b9ea6b119a256bebf was found on GoB mirror. Scheduling merge group tasks...', + 'triggerMergeGroupTargets(flutter/flutter, c9affbbb12aa40cb3afbe94b9ea6b119a256bebf, simulated): Scheduling merge group checks', 'All required tests passed for c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', 'Finished Simulating merge group checks for @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', ], @@ -2679,7 +2692,7 @@ void foo() { any, any, 'c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulated merge queue check', + 'Merge queue check', output: anyNamed('output'), ), ).called(1); @@ -2699,8 +2712,9 @@ void foo() { [ 'Processing merge_group', 'Processing checks_requested for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulating checks requests for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulating merge group checks for @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', + 'Checks requests for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', + 'flutter/flutter/c9affbbb12aa40cb3afbe94b9ea6b119a256bebf was found on GoB mirror. Scheduling merge group tasks...', + 'triggerMergeGroupTargets(flutter/flutter, c9affbbb12aa40cb3afbe94b9ea6b119a256bebf, simulated): Scheduling merge group checks', 'Some required tests failed for c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', 'Some checks failed', 'Finished Simulating merge group checks for @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', @@ -2728,7 +2742,7 @@ void foo() { [ 'Processing merge_group', 'Processing destroyed for merge queue @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', - 'Simulating destruction of a merge group @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', + 'Destroying the merge group for flutter/flutter/c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', 'Simulating cancellation of merge group CI targets for @ c9affbbb12aa40cb3afbe94b9ea6b119a256bebf', ], ); diff --git a/app_dart/test/src/service/fake_gerrit_service.dart b/app_dart/test/src/service/fake_gerrit_service.dart index e1f684e0e..a49a0fd83 100644 --- a/app_dart/test/src/service/fake_gerrit_service.dart +++ b/app_dart/test/src/service/fake_gerrit_service.dart @@ -28,9 +28,9 @@ class FakeGerritService extends GerritService { List? commitsValue; final List _defaultCommits = [ - generateGerritCommit(1), - generateGerritCommit(2), - generateGerritCommit(3), + generateGerritCommit('abc', 1), + generateGerritCommit('cde', 2), + generateGerritCommit('efg', 3), ]; @override diff --git a/app_dart/test/src/utilities/entity_generators.dart b/app_dart/test/src/utilities/entity_generators.dart index f9da9559d..74f4499ce 100644 --- a/app_dart/test/src/utilities/entity_generators.dart +++ b/app_dart/test/src/utilities/entity_generators.dart @@ -345,12 +345,12 @@ github.PullRequest generatePullRequest({ ); } -GerritCommit generateGerritCommit(int i) => GerritCommit( - commit: 'sha$i', +GerritCommit generateGerritCommit(String sha, int milliseconds) => GerritCommit( + commit: sha, tree: 'main', author: GerritUser( email: 'dash@flutter.dev', - time: DateTime.fromMillisecondsSinceEpoch(i), + time: DateTime.fromMillisecondsSinceEpoch(milliseconds), ), ); From 51b1ac66f8e891c41be47ea3f74af4e4e6167f9b Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 27 Nov 2024 15:27:18 -0800 Subject: [PATCH 03/10] scheduler tests --- app_dart/lib/src/service/scheduler.dart | 30 +- app_dart/test/service/scheduler_test.dart | 109 ++++++ app_dart/test/src/utilities/mocks.mocks.dart | 40 +++ .../src/utilities/webhook_generators.dart | 332 +++++++++--------- 4 files changed, 337 insertions(+), 174 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index a2871246d..a533a3213 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -636,23 +636,33 @@ class Scheduler { log.info('Attempting to read merge group targets from ci.yaml for $headSha'); final Commit commit = Commit( - branch: baseRef, + branch: baseRef.substring('refs/heads/'.length), repository: slug.fullName, sha: headSha, ); - final ciYaml = await getCiYaml(commit, validate: true); + + late CiYamlSet ciYaml; + log.info('Attempting to read merge group targets from ci.yaml for $headSha'); + if (commit.branch == Config.defaultBranch(commit.slug)) { + ciYaml = await getCiYaml(commit, validate: true); + } else { + ciYaml = await getCiYaml(commit); + } log.info('ci.yaml loaded successfully.'); log.info('Collecting merge group targets for $headSha'); + final inner = ciYaml.ciYamlFor(type); - final List postSubmitTargets = [ - ...inner.postsubmitTargets.where( - (Target target) => - target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon, - ), - ]; - log.info('Collected ${postSubmitTargets.length} merge group targets.'); - return postSubmitTargets; + // Filter out schedulers targets with schedulers different than luci or cocoon. + final List mergeGroupTargets = inner.presubmitTargets + .where( + (Target target) => + target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon, + ) + .toList(); + + log.info('Collected ${mergeGroupTargets.length} presubmit targets.'); + return mergeGroupTargets; } // Pretend the check took 1 minute to run diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index d148dbf05..03569b58f 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -41,6 +41,7 @@ import '../src/service/fake_github_service.dart'; import '../src/service/fake_luci_build_service.dart'; import '../src/utilities/entity_generators.dart'; import '../src/utilities/mocks.dart'; +import '../src/utilities/webhook_generators.dart'; const String singleCiYaml = r''' enabled_branches: @@ -80,6 +81,7 @@ targets: - stable scheduler: luci - name: Linux engine_build + scheduler: luci properties: release_build: "true" - name: Linux runIf engine @@ -2040,6 +2042,113 @@ targets: ); }); }); + + group('merge groups', () { + test('schedule some work on prod', () async { + httpClient = MockClient((http.Request request) async { + if (request.url.path.endsWith('engine/src/flutter/.ci.yaml')) { + return http.Response(fusionCiYaml, 200); + } else if (request.url.path.endsWith('.ci.yaml')) { + return http.Response(singleCiYaml, 200); + } + throw Exception('Failed to find ${request.url.path}'); + }); + final luci = MockLuciBuildService(); + when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest'))) + .thenAnswer((inv) async { + return []; + }); + final MockGithubService mockGithubService = MockGithubService(); + final checkRuns = []; + when(mockGithubChecksUtil.createCheckRun(any, any, any, any, output: anyNamed('output'))) + .thenAnswer((inv) async { + final slug = inv.positionalArguments[1] as RepositorySlug; + final sha = inv.positionalArguments[2]; + final name = inv.positionalArguments[3]; + checkRuns.add(createCheckRun(id: 1, owner: slug.owner, repo: slug.name, sha: sha, name: name)); + return checkRuns.last; + }); + when(mockGithubService.listFiles(any)).thenAnswer((_) async => ['abc/def']); + + fakeFusion.isFusion = (_, __) => true; + + when( + callbacks.initializeDocument( + firestoreService: anyNamed('firestoreService'), + slug: anyNamed('slug'), + sha: anyNamed('sha'), + stage: anyNamed('stage'), + tasks: anyNamed('tasks'), + checkRunGuard: anyNamed('checkRunGuard'), + ), + ).thenAnswer((_) async => CiStaging()); + + scheduler = Scheduler( + cache: cache, + config: FakeConfig( + // tabledataResource: tabledataResource, + dbValue: db, + githubService: mockGithubService, + githubClient: MockGitHub(), + firestoreService: mockFirestoreService, + ), + buildStatusProvider: (_, __) => buildStatusService, + datastoreProvider: (DatastoreDB db) => DatastoreService(db, 2), + githubChecksService: GithubChecksService(config, githubChecksUtil: mockGithubChecksUtil), + httpClientProvider: () => httpClient, + luciBuildService: luci, + fusionTester: fakeFusion, + initializeCiStagingDocument: callbacks.initializeDocument, + ); + + final mergeGroupEvent = cocoon_checks.MergeGroupEvent.fromJson( + json.decode( + generateMergeGroupEventString( + repository: 'flutter/flutter', + action: 'checks_requested', + message: 'Implement an amazing feature', + ), + ), + ); + + await scheduler.triggerMergeGroupTargets(mergeGroupEvent: mergeGroupEvent); + final results = + verify(mockGithubChecksUtil.createCheckRun(any, any, any, captureAny, output: captureAnyNamed('output'))) + .captured; + stdout.writeAll(results); + + final result = + verify(luci.scheduleMergeGroupBuilds(targets: captureAnyNamed('targets'), commit: anyNamed('commit'))); + expect(result.callCount, 1); + final captured = result.captured; + expect(captured[0], hasLength(1)); + // see the blend of fusionCiYaml and singleCiYaml + expect(captured[0][0].getTestName, 'engine_build'); + + expect(checkRuns, hasLength(2)); + verify( + mockGithubChecksUtil.updateCheckRun( + any, + Config.flutterSlug, + checkRuns[1], + status: argThat(equals(CheckRunStatus.completed), named: 'status'), + conclusion: argThat(equals(CheckRunConclusion.success), named: 'conclusion'), + output: anyNamed('output'), + ), + ).called(1); + + verifyNever( + mockGithubChecksUtil.updateCheckRun( + any, + Config.flutterSlug, + checkRuns[0], + status: anyNamed('status'), + conclusion: anyNamed('conclusion'), + output: anyNamed('output'), + ), + ); + }); + }); }); } diff --git a/app_dart/test/src/utilities/mocks.mocks.dart b/app_dart/test/src/utilities/mocks.mocks.dart index bc4c12492..0e2c5ed82 100644 --- a/app_dart/test/src/utilities/mocks.mocks.dart +++ b/app_dart/test/src/utilities/mocks.mocks.dart @@ -6601,6 +6601,28 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { required _i13.PullRequest pullRequest, })); + @override + _i20.Future<_i13.PullRequest> Function( + _i15.FirestoreService, + int, + String, + ) get findPullRequestFor => (super.noSuchMethod( + Invocation.getter(#findPullRequestFor), + returnValue: ( + _i15.FirestoreService firestoreService, + int checkRunId, + String checkRunName, + ) => + _i20.Future<_i13.PullRequest>.value(_FakePullRequest_41( + this, + Invocation.getter(#findPullRequestFor), + )), + ) as _i20.Future<_i13.PullRequest> Function( + _i15.FirestoreService, + int, + String, + )); + @override _i20.Future>> shard({ required List<_i8.BatchRequest_Request>? requests, @@ -6877,6 +6899,24 @@ class MockLuciBuildService extends _i1.Mock implements _i15.LuciBuildService { <_i15.Tuple<_i43.Target, _i37.Task, int>>[]), ) as _i20.Future>>); + @override + _i20.Future scheduleMergeGroupBuilds({ + required _i36.Commit? commit, + required List<_i43.Target>? targets, + }) => + (super.noSuchMethod( + Invocation.method( + #scheduleMergeGroupBuilds, + [], + { + #commit: commit, + #targets: targets, + }, + ), + returnValue: _i20.Future.value(), + returnValueForMissingStub: _i20.Future.value(), + ) as _i20.Future); + @override _i20.Future createPostsubmitCheckRun( _i36.Commit? commit, diff --git a/app_dart/test/src/utilities/webhook_generators.dart b/app_dart/test/src/utilities/webhook_generators.dart index e8b161a5f..6f9efed34 100644 --- a/app_dart/test/src/utilities/webhook_generators.dart +++ b/app_dart/test/src/utilities/webhook_generators.dart @@ -1119,175 +1119,179 @@ PushMessage generateMergeGroupMessage({ }) { final pb.GithubWebhookMessage webhookMessage = pb.GithubWebhookMessage( event: 'merge_group', - payload: ''' + payload: generateMergeGroupEventString(action: action, message: message, repository: repository), + ); + return PushMessage(data: webhookMessage.writeToJson(), messageId: 'abc123'); +} + +String generateMergeGroupEventString({required String action, required String message, required String repository}) { + return ''' { - "action": "$action", - "merge_group": { - "head_sha": "c9affbbb12aa40cb3afbe94b9ea6b119a256bebf", - "head_ref": "refs/heads/gh-readonly-queue/main/pr-15-172355550dde5881b0269972ea4cbe5a6d0561bc", - "base_sha": "172355550dde5881b0269972ea4cbe5a6d0561bc", - "base_ref": "refs/heads/main", - "head_commit": { - "id": "c9affbbb12aa40cb3afbe94b9ea6b119a256bebf", - "tree_id": "556b9a8db18c974738d9d5e15988ae9a67e96b91", - "message": "$message", - "timestamp": "2024-10-15T20:24:16Z", - "author": { - "name": "John Doe", - "email": "johndoe@example.org" - }, - "committer": { - "name": "GitHub", - "email": "noreply@github.com" - } - } - }, - "repository": { - "id": 186853002, - "node_id": "MDEwOlJlcG9zaXRvcnkxODY4NTMwMDI=", - "name": "${repository.split('/')[1]}", - "full_name": "$repository", - "private": false, - "owner": { - "login": "${repository.split('/')[0]}", - "id": 21031067, - "node_id": "MDQ6VXNlcjIxMDMxMDY3", - "avatar_url": "https://avatars1.githubusercontent.com/u/21031067?v=4", - "gravatar_id": "", - "url": "https://api.github.com/users/Codertocat", - "html_url": "https://github.com/Codertocat", - "followers_url": "https://api.github.com/users/Codertocat/followers", - "following_url": "https://api.github.com/users/Codertocat/following{/other_user}", - "gists_url": "https://api.github.com/users/Codertocat/gists{/gist_id}", - "starred_url": "https://api.github.com/users/Codertocat/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/Codertocat/subscriptions", - "organizations_url": "https://api.github.com/users/Codertocat/orgs", - "repos_url": "https://api.github.com/users/Codertocat/repos", - "events_url": "https://api.github.com/users/Codertocat/events{/privacy}", - "received_events_url": "https://api.github.com/users/Codertocat/received_events", - "type": "User", - "site_admin": false +"action": "$action", +"merge_group": { + "head_sha": "c9affbbb12aa40cb3afbe94b9ea6b119a256bebf", + "head_ref": "refs/heads/gh-readonly-queue/main/pr-15-172355550dde5881b0269972ea4cbe5a6d0561bc", + "base_sha": "172355550dde5881b0269972ea4cbe5a6d0561bc", + "base_ref": "refs/heads/main", + "head_commit": { + "id": "c9affbbb12aa40cb3afbe94b9ea6b119a256bebf", + "tree_id": "556b9a8db18c974738d9d5e15988ae9a67e96b91", + "message": "$message", + "timestamp": "2024-10-15T20:24:16Z", + "author": { + "name": "John Doe", + "email": "johndoe@example.org" }, - "html_url": "https://github.com/$repository", - "description": null, - "fork": false, - "url": "https://api.github.com/repos/$repository", - "forks_url": "https://api.github.com/repos/$repository/forks", - "keys_url": "https://api.github.com/repos/$repository/keys{/key_id}", - "collaborators_url": "https://api.github.com/repos/$repository/collaborators{/collaborator}", - "teams_url": "https://api.github.com/repos/$repository/teams", - "hooks_url": "https://api.github.com/repos/$repository/hooks", - "issue_events_url": "https://api.github.com/repos/$repository/issues/events{/number}", - "events_url": "https://api.github.com/repos/$repository/events", - "assignees_url": "https://api.github.com/repos/$repository/assignees{/user}", - "branches_url": "https://api.github.com/repos/$repository/branches{/branch}", - "tags_url": "https://api.github.com/repos/$repository/tags", - "blobs_url": "https://api.github.com/repos/$repository/git/blobs{/sha}", - "git_tags_url": "https://api.github.com/repos/$repository/git/tags{/sha}", - "git_refs_url": "https://api.github.com/repos/$repository/git/refs{/sha}", - "trees_url": "https://api.github.com/repos/$repository/git/trees{/sha}", - "statuses_url": "https://api.github.com/repos/$repository/statuses/{sha}", - "languages_url": "https://api.github.com/repos/$repository/languages", - "stargazers_url": "https://api.github.com/repos/$repository/stargazers", - "contributors_url": "https://api.github.com/repos/$repository/contributors", - "subscribers_url": "https://api.github.com/repos/$repository/subscribers", - "subscription_url": "https://api.github.com/repos/$repository/subscription", - "commits_url": "https://api.github.com/repos/$repository/commits{/sha}", - "git_commits_url": "https://api.github.com/repos/$repository/git/commits{/sha}", - "comments_url": "https://api.github.com/repos/$repository/comments{/number}", - "issue_comment_url": "https://api.github.com/repos/$repository/issues/comments{/number}", - "contents_url": "https://api.github.com/repos/$repository/contents/{+path}", - "compare_url": "https://api.github.com/repos/$repository/compare/{base}...{head}", - "merges_url": "https://api.github.com/repos/$repository/merges", - "archive_url": "https://api.github.com/repos/$repository/{archive_format}{/ref}", - "downloads_url": "https://api.github.com/repos/$repository/downloads", - "issues_url": "https://api.github.com/repos/$repository/issues{/number}", - "pulls_url": "https://api.github.com/repos/$repository/pulls{/number}", - "milestones_url": "https://api.github.com/repos/$repository/milestones{/number}", - "notifications_url": "https://api.github.com/repos/$repository/notifications{?since,all,participating}", - "labels_url": "https://api.github.com/repos/$repository/labels{/name}", - "releases_url": "https://api.github.com/repos/$repository/releases{/id}", - "deployments_url": "https://api.github.com/repos/$repository/deployments", - "created_at": "2019-05-15T15:19:25Z", - "updated_at": "2019-05-15T15:20:41Z", - "pushed_at": "2019-05-15T15:20:56Z", - "git_url": "git://github.com/$repository.git", - "ssh_url": "git@github.com:Codertocat/Hello-World.git", - "clone_url": "https://github.com/$repository.git", - "svn_url": "https://github.com/$repository", - "homepage": null, - "size": 0, - "stargazers_count": 0, - "watchers_count": 0, - "language": "Ruby", - "has_issues": true, - "has_projects": true, - "has_downloads": true, - "has_wiki": true, - "has_pages": true, - "forks_count": 1, - "mirror_url": null, - "archived": false, - "disabled": false, - "open_issues_count": 2, - "license": null, - "forks": 1, - "open_issues": 2, - "watchers": 0, - "default_branch": "master" - }, - "organization": { - "login": "flutter", - "id": 14101776, - "node_id": "MDEyOk9yZ2FuaXphdGlvbjE0MTAxNzc2", - "url": "https://api.github.com/orgs/flutter", - "repos_url": "https://api.github.com/orgs/flutter/repos", - "events_url": "https://api.github.com/orgs/flutter/events", - "hooks_url": "https://api.github.com/orgs/flutter/hooks", - "issues_url": "https://api.github.com/orgs/flutter/issues", - "members_url": "https://api.github.com/orgs/flutter/members{/member}", - "public_members_url": "https://api.github.com/orgs/flutter/public_members{/member}", - "avatar_url": "https://avatars.githubusercontent.com/u/14101776?v=4", - "description": "Flutter is Google's UI toolkit for building beautiful, natively compiled applications for mobile, web, desktop, and embedded devices from a single codebase." - }, - "enterprise": { - "id": 1732, - "slug": "alphabet", - "name": "Alphabet", - "node_id": "MDEwOkVudGVycHJpc2UxNzMy", - "avatar_url": "https://avatars.githubusercontent.com/b/1732?v=4", - "description": "", - "website_url": "https://abc.xyz/", - "html_url": "https://github.com/enterprises/alphabet", - "created_at": "2019-12-19T00:30:52Z", - "updated_at": "2024-07-18T11:54:37Z" - }, - "sender": { - "login": "johndoe", - "id": 1924313, - "node_id": "MDQ6VXNlcjE5MjQzMTM=", - "avatar_url": "https://avatars.githubusercontent.com/u/1924313?v=4", + "committer": { + "name": "GitHub", + "email": "noreply@github.com" + } + } +}, +"repository": { + "id": 186853002, + "node_id": "MDEwOlJlcG9zaXRvcnkxODY4NTMwMDI=", + "name": "${repository.split('/')[1]}", + "full_name": "$repository", + "private": false, + "owner": { + "login": "${repository.split('/')[0]}", + "id": 21031067, + "node_id": "MDQ6VXNlcjIxMDMxMDY3", + "avatar_url": "https://avatars1.githubusercontent.com/u/21031067?v=4", "gravatar_id": "", - "url": "https://api.github.com/users/johndoe", - "html_url": "https://github.com/johndoe", - "followers_url": "https://api.github.com/users/johndoe/followers", - "following_url": "https://api.github.com/users/johndoe/following{/other_user}", - "gists_url": "https://api.github.com/users/johndoe/gists{/gist_id}", - "starred_url": "https://api.github.com/users/johndoe/starred{/owner}{/repo}", - "subscriptions_url": "https://api.github.com/users/johndoe/subscriptions", - "organizations_url": "https://api.github.com/users/johndoe/orgs", - "repos_url": "https://api.github.com/users/johndoe/repos", - "events_url": "https://api.github.com/users/johndoe/events{/privacy}", - "received_events_url": "https://api.github.com/users/johndoe/received_events", + "url": "https://api.github.com/users/Codertocat", + "html_url": "https://github.com/Codertocat", + "followers_url": "https://api.github.com/users/Codertocat/followers", + "following_url": "https://api.github.com/users/Codertocat/following{/other_user}", + "gists_url": "https://api.github.com/users/Codertocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/Codertocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/Codertocat/subscriptions", + "organizations_url": "https://api.github.com/users/Codertocat/orgs", + "repos_url": "https://api.github.com/users/Codertocat/repos", + "events_url": "https://api.github.com/users/Codertocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/Codertocat/received_events", "type": "User", "site_admin": false }, - "installation": { - "id": 10381585, - "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMTAzODE1ODU=" - } + "html_url": "https://github.com/$repository", + "description": null, + "fork": false, + "url": "https://api.github.com/repos/$repository", + "forks_url": "https://api.github.com/repos/$repository/forks", + "keys_url": "https://api.github.com/repos/$repository/keys{/key_id}", + "collaborators_url": "https://api.github.com/repos/$repository/collaborators{/collaborator}", + "teams_url": "https://api.github.com/repos/$repository/teams", + "hooks_url": "https://api.github.com/repos/$repository/hooks", + "issue_events_url": "https://api.github.com/repos/$repository/issues/events{/number}", + "events_url": "https://api.github.com/repos/$repository/events", + "assignees_url": "https://api.github.com/repos/$repository/assignees{/user}", + "branches_url": "https://api.github.com/repos/$repository/branches{/branch}", + "tags_url": "https://api.github.com/repos/$repository/tags", + "blobs_url": "https://api.github.com/repos/$repository/git/blobs{/sha}", + "git_tags_url": "https://api.github.com/repos/$repository/git/tags{/sha}", + "git_refs_url": "https://api.github.com/repos/$repository/git/refs{/sha}", + "trees_url": "https://api.github.com/repos/$repository/git/trees{/sha}", + "statuses_url": "https://api.github.com/repos/$repository/statuses/{sha}", + "languages_url": "https://api.github.com/repos/$repository/languages", + "stargazers_url": "https://api.github.com/repos/$repository/stargazers", + "contributors_url": "https://api.github.com/repos/$repository/contributors", + "subscribers_url": "https://api.github.com/repos/$repository/subscribers", + "subscription_url": "https://api.github.com/repos/$repository/subscription", + "commits_url": "https://api.github.com/repos/$repository/commits{/sha}", + "git_commits_url": "https://api.github.com/repos/$repository/git/commits{/sha}", + "comments_url": "https://api.github.com/repos/$repository/comments{/number}", + "issue_comment_url": "https://api.github.com/repos/$repository/issues/comments{/number}", + "contents_url": "https://api.github.com/repos/$repository/contents/{+path}", + "compare_url": "https://api.github.com/repos/$repository/compare/{base}...{head}", + "merges_url": "https://api.github.com/repos/$repository/merges", + "archive_url": "https://api.github.com/repos/$repository/{archive_format}{/ref}", + "downloads_url": "https://api.github.com/repos/$repository/downloads", + "issues_url": "https://api.github.com/repos/$repository/issues{/number}", + "pulls_url": "https://api.github.com/repos/$repository/pulls{/number}", + "milestones_url": "https://api.github.com/repos/$repository/milestones{/number}", + "notifications_url": "https://api.github.com/repos/$repository/notifications{?since,all,participating}", + "labels_url": "https://api.github.com/repos/$repository/labels{/name}", + "releases_url": "https://api.github.com/repos/$repository/releases{/id}", + "deployments_url": "https://api.github.com/repos/$repository/deployments", + "created_at": "2019-05-15T15:19:25Z", + "updated_at": "2019-05-15T15:20:41Z", + "pushed_at": "2019-05-15T15:20:56Z", + "git_url": "git://github.com/$repository.git", + "ssh_url": "git@github.com:Codertocat/Hello-World.git", + "clone_url": "https://github.com/$repository.git", + "svn_url": "https://github.com/$repository", + "homepage": null, + "size": 0, + "stargazers_count": 0, + "watchers_count": 0, + "language": "Ruby", + "has_issues": true, + "has_projects": true, + "has_downloads": true, + "has_wiki": true, + "has_pages": true, + "forks_count": 1, + "mirror_url": null, + "archived": false, + "disabled": false, + "open_issues_count": 2, + "license": null, + "forks": 1, + "open_issues": 2, + "watchers": 0, + "default_branch": "master" +}, +"organization": { + "login": "flutter", + "id": 14101776, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjE0MTAxNzc2", + "url": "https://api.github.com/orgs/flutter", + "repos_url": "https://api.github.com/orgs/flutter/repos", + "events_url": "https://api.github.com/orgs/flutter/events", + "hooks_url": "https://api.github.com/orgs/flutter/hooks", + "issues_url": "https://api.github.com/orgs/flutter/issues", + "members_url": "https://api.github.com/orgs/flutter/members{/member}", + "public_members_url": "https://api.github.com/orgs/flutter/public_members{/member}", + "avatar_url": "https://avatars.githubusercontent.com/u/14101776?v=4", + "description": "Flutter is Google's UI toolkit for building beautiful, natively compiled applications for mobile, web, desktop, and embedded devices from a single codebase." +}, +"enterprise": { + "id": 1732, + "slug": "alphabet", + "name": "Alphabet", + "node_id": "MDEwOkVudGVycHJpc2UxNzMy", + "avatar_url": "https://avatars.githubusercontent.com/b/1732?v=4", + "description": "", + "website_url": "https://abc.xyz/", + "html_url": "https://github.com/enterprises/alphabet", + "created_at": "2019-12-19T00:30:52Z", + "updated_at": "2024-07-18T11:54:37Z" +}, +"sender": { + "login": "johndoe", + "id": 1924313, + "node_id": "MDQ6VXNlcjE5MjQzMTM=", + "avatar_url": "https://avatars.githubusercontent.com/u/1924313?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/johndoe", + "html_url": "https://github.com/johndoe", + "followers_url": "https://api.github.com/users/johndoe/followers", + "following_url": "https://api.github.com/users/johndoe/following{/other_user}", + "gists_url": "https://api.github.com/users/johndoe/gists{/gist_id}", + "starred_url": "https://api.github.com/users/johndoe/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/johndoe/subscriptions", + "organizations_url": "https://api.github.com/users/johndoe/orgs", + "repos_url": "https://api.github.com/users/johndoe/repos", + "events_url": "https://api.github.com/users/johndoe/events{/privacy}", + "received_events_url": "https://api.github.com/users/johndoe/received_events", + "type": "User", + "site_admin": false +}, +"installation": { + "id": 10381585, + "node_id": "MDIzOkludGVncmF0aW9uSW5zdGFsbGF0aW9uMTAzODE1ODU=" } -''', - ); - return PushMessage(data: webhookMessage.writeToJson(), messageId: 'abc123'); +} +'''; } From a985fb40cee4814ad6c7a27ef45b19aefb2b7d8c Mon Sep 17 00:00:00 2001 From: John McDole Date: Mon, 2 Dec 2024 16:50:22 -0800 Subject: [PATCH 04/10] Add test for luci --- .../test/service/luci_build_service_test.dart | 167 ++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart index e252bbc1a..6dbcb55dc 100644 --- a/app_dart/test/service/luci_build_service_test.dart +++ b/app_dart/test/service/luci_build_service_test.dart @@ -1495,4 +1495,171 @@ void main() { expect(firestoreTask!.status, firestore.Task.statusInProgress); }); }); + + group('scheduleMergeGroupBuilds', () { + late MockGithubChecksUtil mockGithubChecksUtil; + late MockFirestoreService mockFirestoreService; + firestore_commit.Commit? firestoreCommit; + setUp(() { + cache = CacheService(inMemory: true); + config = FakeConfig(); + firestoreCommit = null; + mockBuildBucketClient = MockBuildBucketClient(); + when(mockBuildBucketClient.listBuilders(any)).thenAnswer((_) async { + return bbv2.ListBuildersResponse( + builders: [ + bbv2.BuilderItem( + id: bbv2.BuilderID( + bucket: 'prod', + project: 'flutter', + builder: 'Linux 1', + ), + ), + bbv2.BuilderItem( + id: bbv2.BuilderID( + bucket: 'prod', + project: 'flutter', + builder: 'Linux 2', + ), + ), + ], + ); + }); + + mockGithubChecksUtil = MockGithubChecksUtil(); + when( + mockGithubChecksUtil.createCheckRun( + any, + any, + any, + any, + output: anyNamed('output'), + ), + ).thenAnswer((realInvocation) async => generateCheckRun(1)); + + mockFirestoreService = MockFirestoreService(); + when( + mockFirestoreService.batchWriteDocuments( + captureAny, + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value(BatchWriteResponse()); + }); + when( + mockFirestoreService.getDocument( + captureAny, + ), + ).thenAnswer((Invocation invocation) { + return Future.value( + firestoreCommit, + ); + }); + when( + mockFirestoreService.queryRecentCommits( + limit: captureAnyNamed('limit'), + slug: captureAnyNamed('slug'), + branch: captureAnyNamed('branch'), + ), + ).thenAnswer((Invocation invocation) { + return Future>.value( + [firestoreCommit!], + ); + }); + pubsub = FakePubSub(); + service = LuciBuildService( + config: config, + cache: cache, + buildBucketClient: mockBuildBucketClient, + githubChecksUtil: mockGithubChecksUtil, + pubsub: pubsub, + ); + }); + + test('schedules prod builds for commit', () async { + final Commit commit = + generateCommit(100, sha: 'abc1234', repo: 'flaux', branch: 'gh-readonly-queue/master/pr-1234-abcd'); + final List targets = [ + generateTarget(1, properties: {'os': 'abc'}, slug: RepositorySlug('flutter', 'flaux')), + generateTarget(2, properties: {'os': 'abc'}, slug: RepositorySlug('flutter', 'flaux')), + ]; + await service.scheduleMergeGroupBuilds(commit: commit, targets: targets); + + verify(mockGithubChecksUtil.createCheckRun(any, RepositorySlug('flutter', 'flaux'), 'abc1234', 'Linux 1')) + .called(1); + verify(mockGithubChecksUtil.createCheckRun(any, RepositorySlug('flutter', 'flaux'), 'abc1234', 'Linux 2')) + .called(1); + expect(pubsub.messages, hasLength(1)); + final batchRequest = bbv2.BatchRequest()..mergeFromProto3Json(pubsub.messages.first); + expect(batchRequest.requests, hasLength(2)); + + validateSchedule(bbv2.ScheduleBuildRequest scheduleBuild, String builderName) { + expect(scheduleBuild.builder.bucket, 'prod'); + expect(scheduleBuild.builder.builder, builderName); + expect( + scheduleBuild.notify.pubsubTopic, + 'projects/flutter-dashboard/topics/build-bucket-presubmit', + ); + + final userDataMap = UserData.decodeUserDataBytes(scheduleBuild.notify.userData); + expect(userDataMap, { + 'repo_owner': 'flutter', + 'repo_name': 'flaux', + 'check_run_id': 1, + 'commit_sha': 'abc1234', + 'commit_branch': 'gh-readonly-queue/master/pr-1234-abcd', + 'builder_name': builderName, + 'firestore_task_document_name': 'abc1234_${builderName}_1', + }); + + final properties = scheduleBuild.properties.fields; + final dimensions = scheduleBuild.dimensions; + + expect(properties, { + 'os': bbv2.Value(stringValue: 'abc'), + 'dependencies': bbv2.Value(listValue: bbv2.ListValue()), + 'bringup': bbv2.Value(boolValue: false), + 'git_branch': bbv2.Value(stringValue: 'gh-readonly-queue/master/pr-1234-abcd'), + 'exe_cipd_version': bbv2.Value(stringValue: 'refs/heads/gh-readonly-queue/master/pr-1234-abcd'), + 'recipe': bbv2.Value(stringValue: 'devicelab/devicelab'), + }); + expect(dimensions.length, 1); + expect(dimensions[0].key, 'os'); + expect(dimensions[0].value, 'abc'); + } + + validateSchedule(batchRequest.requests[0].scheduleBuild, 'Linux 1'); + validateSchedule(batchRequest.requests[1].scheduleBuild, 'Linux 2'); + }); + + test('skips unmatched builders', () async { + when(mockBuildBucketClient.listBuilders(any)).thenAnswer((_) async { + return bbv2.ListBuildersResponse( + builders: [ + bbv2.BuilderItem( + id: bbv2.BuilderID( + bucket: 'prod', + project: 'flutter', + builder: 'Linux 1', + ), + ), + ], + ); + }); + final Commit commit = + generateCommit(100, sha: 'abc1234', repo: 'flaux', branch: 'gh-readonly-queue/master/pr-1234-abcd'); + final List targets = [ + generateTarget(1, properties: {'os': 'abc'}, slug: RepositorySlug('flutter', 'flaux')), + generateTarget(2, properties: {'os': 'abc'}, slug: RepositorySlug('flutter', 'flaux')), + ]; + await service.scheduleMergeGroupBuilds(commit: commit, targets: targets); + + verify(mockGithubChecksUtil.createCheckRun(any, RepositorySlug('flutter', 'flaux'), 'abc1234', 'Linux 1')) + .called(1); + verifyNever(mockGithubChecksUtil.createCheckRun(any, RepositorySlug('flutter', 'flaux'), 'abc1234', 'Linux 2')); + expect(pubsub.messages, hasLength(1)); + final batchRequest = bbv2.BatchRequest()..mergeFromProto3Json(pubsub.messages.first); + expect(batchRequest.requests, hasLength(1)); + }); + }); } From b750df87efe8a3aa3e054c2779f724dbebc77015 Mon Sep 17 00:00:00 2001 From: John McDole Date: Mon, 2 Dec 2024 16:53:49 -0800 Subject: [PATCH 05/10] Fix comment --- app_dart/lib/src/service/scheduler.dart | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index a533a3213..3be72f3db 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -522,21 +522,12 @@ class Scheduler { Future triggerMergeGroupTargets({ required cocoon_checks.MergeGroupEvent mergeGroupEvent, }) async { - /* - Behave similar to addPullRequest, except we're not yet merged into master. - - We are mirrored in to GoB - - We want PROD builds - - We want check_runs as well - - this might mean we want a different pubsub? - - We don't need "Task" objects because I beleive these are used by flutter-dashboard, - we're tracking with github. So really we need _batchScheduleBuilds - - batchSCheduleBuilds just calls schedulePostsubmitBuilds - which creates but doesn't - return the check run? - - CODEFU - left off here. - */ + // Behave similar to addPullRequest, except we're not yet merged into master. + // - We are mirrored in to GoB + // - We want PROD builds + // - We want check_runs as well + // - We want updates on check_runs to the presubmit pubsub. + // We do not want "Task" objects because these are for flutter-dashboard tracking (post submit) final mergeGroup = mergeGroupEvent.mergeGroup; final headSha = mergeGroup.headSha; final slug = mergeGroupEvent.repository!.slug(); From fb2ce0d604cb562c6b11f801588351df77d7c5d1 Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 3 Dec 2024 08:47:08 -0800 Subject: [PATCH 06/10] Nits --- .../lib/src/service/luci_build_service.dart | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 7eb63d54e..a1faa51c5 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -779,7 +779,7 @@ class LuciBuildService { required Commit commit, required List targets, }) async { - final List buildRequests = []; + final buildRequests = []; Set availableBuilderSet; try { @@ -813,7 +813,7 @@ class LuciBuildService { ); } - final bbv2.BatchRequest batchRequest = bbv2.BatchRequest(requests: buildRequests); + final batchRequest = bbv2.BatchRequest(requests: buildRequests); log.fine(batchRequest); List messageIds; @@ -1059,7 +1059,7 @@ class LuciBuildService { log.info( 'Creating merge group schedule builder for ${target.value.name} on commit ${commit.sha}', ); - tags ??= []; + tags ??= []; tags.addAll([ bbv2.StringPair( key: 'buildset', @@ -1075,7 +1075,7 @@ class LuciBuildService { 'Scheduling builder: ${target.value.name} for commit ${commit.sha}', ); - final Map rawUserData = {}; + final rawUserData = {}; await createPostsubmitCheckRun( commit, @@ -1097,7 +1097,7 @@ class LuciBuildService { ), ); // Default attempt is the initial attempt, which is 1. - final bbv2.StringPair? attemptTag = tags.singleWhereOrNull((tag) => tag.key == 'current_attempt'); + final attemptTag = tags.singleWhereOrNull((tag) => tag.key == 'current_attempt'); if (attemptTag == null) { tags.add( bbv2.StringPair( @@ -1107,21 +1107,18 @@ class LuciBuildService { ); } - final String currentAttemptStr = tags.firstWhere((tag) => tag.key == 'current_attempt').value; + final currentAttemptStr = tags.firstWhere((tag) => tag.key == 'current_attempt').value; rawUserData['firestore_task_document_name'] = '${commit.sha}_${target.value.name}_$currentAttemptStr'; - final Map processedProperties = target.getProperties().cast(); + final processedProperties = target.getProperties().cast(); processedProperties.addAll(properties ?? {}); processedProperties['git_branch'] = commit.branch!; - final String cipdExe = 'refs/heads/${commit.branch}'; + final cipdExe = 'refs/heads/${commit.branch}'; processedProperties['exe_cipd_version'] = cipdExe; - final bbv2.Struct propertiesStruct = bbv2.Struct.create(); - propertiesStruct.mergeFromProto3Json(processedProperties); - - final List requestedDimensions = target.getDimensions(); - - final bbv2.Executable executable = bbv2.Executable(cipdVersion: cipdExe); + final propertiesStruct = bbv2.Struct()..mergeFromProto3Json(processedProperties); + final requestedDimensions = target.getDimensions(); + final executable = bbv2.Executable(cipdVersion: cipdExe); log.info( 'Constructing the merge group schedule build request for ${target.value.name} on commit ${commit.sha}.', @@ -1162,7 +1159,7 @@ class LuciBuildService { // We are not tracking this check run in the PrCheckRuns firestore doc because // there is no PR to look up later. The check run is important because it // informs the staging document setup for Merge Groups in triggerMergeGroupTargets. - final github.CheckRun checkRun = await githubChecksUtil.createCheckRun( + final checkRun = await githubChecksUtil.createCheckRun( config, target.slug, commit.sha!, From 5d4734256b65d0307a5d1ed4d375d310ed1c7e1f Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 3 Dec 2024 11:04:33 -0800 Subject: [PATCH 07/10] Tests for unschedulable builds --- app_dart/lib/src/service/scheduler.dart | 27 +++- app_dart/test/service/scheduler_test.dart | 179 ++++++++++++++++++++-- 2 files changed, 189 insertions(+), 17 deletions(-) diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index 3be72f3db..bcefeaa3e 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -555,15 +555,26 @@ class Scheduler { return; } - final mergeGroupTargets = await getMergeGroupTargetsForStage( - mergeGroup.baseRef, - slug, - headSha, - CiStage.fusionEngineBuild, - ); + final mergeGroupTargets = { + ...await getMergeGroupTargetsForStage( + mergeGroup.baseRef, + slug, + headSha, + CiStage.fusionEngineBuild, + ), + }; Object? exception; try { + // Filter out targets missing builders - we cannot wait to complete the merge group if we will never complete. + final availableBuilders = await luciBuildService.getAvailableBuilderSet( + project: 'flutter', + bucket: 'prod', + ); + final availableTargets = {...mergeGroupTargets.where((target) => availableBuilders.contains(target.value.name))}; + if (availableTargets.length != mergeGroupTargets.length) { + log.warning('$logCrumb: missing builders for targtets: ${mergeGroupTargets.difference(availableTargets)}'); + } // Create the staging doc that will track our engine progress and allow us to unlock // the merge group lock later. await initializeCiStagingDocument( @@ -571,7 +582,7 @@ class Scheduler { slug: slug, sha: headSha, stage: CiStage.fusionEngineBuild, - tasks: [...mergeGroupTargets.map((t) => t.value.name)], + tasks: [...availableTargets.map((t) => t.value.name)], checkRunGuard: '$lock', ); @@ -584,7 +595,7 @@ class Scheduler { ); await luciBuildService.scheduleMergeGroupBuilds( - targets: mergeGroupTargets, + targets: [...availableTargets], commit: commit, ); } catch (e, s) { diff --git a/app_dart/test/service/scheduler_test.dart b/app_dart/test/service/scheduler_test.dart index 03569b58f..7c3c71d1d 100644 --- a/app_dart/test/service/scheduler_test.dart +++ b/app_dart/test/service/scheduler_test.dart @@ -91,6 +91,35 @@ targets: - engine/src/flutter/dev/** '''; +const String fusionDualCiYaml = r''' +enabled_branches: + - master + - main + - codefu + - flutter-\d+\.\d+-candidate\.\d+ +targets: + - name: Linux Z + properties: + custom: abc + - name: Linux Y + enabled_branches: + - stable + scheduler: luci + - name: Linux engine_build + scheduler: luci + properties: + release_build: "true" + - name: Mac engine_build + scheduler: luci + properties: + release_build: "true" + - name: Linux runIf engine + runIf: + - DEPS + - engine/src/flutter/.ci.yaml + - engine/src/flutter/dev/** +'''; + void main() { late CacheService cache; late FakeConfig config; @@ -2047,13 +2076,20 @@ targets: test('schedule some work on prod', () async { httpClient = MockClient((http.Request request) async { if (request.url.path.endsWith('engine/src/flutter/.ci.yaml')) { - return http.Response(fusionCiYaml, 200); + return http.Response(fusionDualCiYaml, 200); } else if (request.url.path.endsWith('.ci.yaml')) { return http.Response(singleCiYaml, 200); } throw Exception('Failed to find ${request.url.path}'); }); final luci = MockLuciBuildService(); + when(luci.getAvailableBuilderSet(project: anyNamed('project'), bucket: anyNamed('bucket'))) + .thenAnswer((inv) async { + return { + 'Mac engine_build', + 'Linux engine_build', + }; + }); when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest'))) .thenAnswer((inv) async { return []; @@ -2112,18 +2148,143 @@ targets: ); await scheduler.triggerMergeGroupTargets(mergeGroupEvent: mergeGroupEvent); - final results = - verify(mockGithubChecksUtil.createCheckRun(any, any, any, captureAny, output: captureAnyNamed('output'))) - .captured; - stdout.writeAll(results); + verify( + callbacks.initializeDocument( + firestoreService: anyNamed('firestoreService'), + slug: anyNamed('slug'), + sha: argThat(equals('c9affbbb12aa40cb3afbe94b9ea6b119a256bebf'), named: 'sha'), + stage: anyNamed('stage'), + tasks: argThat(equals(['Linux engine_build', 'Mac engine_build']), named: 'tasks'), + checkRunGuard: anyNamed('checkRunGuard'), + ), + ).called(1); + verify( + luci.getAvailableBuilderSet( + project: argThat(equals('flutter'), named: 'project'), + bucket: argThat(equals('prod'), named: 'bucket'), + ), + ).called(1); + verify(mockGithubChecksUtil.createCheckRun(any, any, any, captureAny, output: captureAnyNamed('output'))) + .called(2); final result = verify(luci.scheduleMergeGroupBuilds(targets: captureAnyNamed('targets'), commit: anyNamed('commit'))); expect(result.callCount, 1); - final captured = result.captured; - expect(captured[0], hasLength(1)); - // see the blend of fusionCiYaml and singleCiYaml - expect(captured[0][0].getTestName, 'engine_build'); + expect(result.captured[0].map((target) => target.value.name), ['Linux engine_build', 'Mac engine_build']); + + expect(checkRuns, hasLength(2)); + verify( + mockGithubChecksUtil.updateCheckRun( + any, + Config.flutterSlug, + checkRuns[1], + status: argThat(equals(CheckRunStatus.completed), named: 'status'), + conclusion: argThat(equals(CheckRunConclusion.success), named: 'conclusion'), + output: anyNamed('output'), + ), + ).called(1); + + verifyNever( + mockGithubChecksUtil.updateCheckRun( + any, + Config.flutterSlug, + checkRuns[0], + status: anyNamed('status'), + conclusion: anyNamed('conclusion'), + output: anyNamed('output'), + ), + ); + }); + + test('handles missing builders', () async { + httpClient = MockClient((http.Request request) async { + if (request.url.path.endsWith('engine/src/flutter/.ci.yaml')) { + return http.Response(fusionDualCiYaml, 200); + } else if (request.url.path.endsWith('.ci.yaml')) { + return http.Response(singleCiYaml, 200); + } + throw Exception('Failed to find ${request.url.path}'); + }); + final luci = MockLuciBuildService(); + when(luci.getAvailableBuilderSet(project: anyNamed('project'), bucket: anyNamed('bucket'))) + .thenAnswer((inv) async { + return { + 'Mac engine_build', + }; + }); + when(luci.scheduleTryBuilds(targets: anyNamed('targets'), pullRequest: anyNamed('pullRequest'))) + .thenAnswer((inv) async { + return []; + }); + final MockGithubService mockGithubService = MockGithubService(); + final checkRuns = []; + when(mockGithubChecksUtil.createCheckRun(any, any, any, any, output: anyNamed('output'))) + .thenAnswer((inv) async { + final slug = inv.positionalArguments[1] as RepositorySlug; + final sha = inv.positionalArguments[2]; + final name = inv.positionalArguments[3]; + checkRuns.add(createCheckRun(id: 1, owner: slug.owner, repo: slug.name, sha: sha, name: name)); + return checkRuns.last; + }); + when(mockGithubService.listFiles(any)).thenAnswer((_) async => ['abc/def']); + + fakeFusion.isFusion = (_, __) => true; + when( + callbacks.initializeDocument( + firestoreService: anyNamed('firestoreService'), + slug: anyNamed('slug'), + sha: anyNamed('sha'), + stage: anyNamed('stage'), + tasks: anyNamed('tasks'), + checkRunGuard: anyNamed('checkRunGuard'), + ), + ).thenAnswer((_) async => CiStaging()); + + scheduler = Scheduler( + cache: cache, + config: FakeConfig( + // tabledataResource: tabledataResource, + dbValue: db, + githubService: mockGithubService, + githubClient: MockGitHub(), + firestoreService: mockFirestoreService, + ), + buildStatusProvider: (_, __) => buildStatusService, + datastoreProvider: (DatastoreDB db) => DatastoreService(db, 2), + githubChecksService: GithubChecksService(config, githubChecksUtil: mockGithubChecksUtil), + httpClientProvider: () => httpClient, + luciBuildService: luci, + fusionTester: fakeFusion, + initializeCiStagingDocument: callbacks.initializeDocument, + ); + + final mergeGroupEvent = cocoon_checks.MergeGroupEvent.fromJson( + json.decode( + generateMergeGroupEventString( + repository: 'flutter/flutter', + action: 'checks_requested', + message: 'Implement an amazing feature', + ), + ), + ); + + await scheduler.triggerMergeGroupTargets(mergeGroupEvent: mergeGroupEvent); + verify( + callbacks.initializeDocument( + firestoreService: anyNamed('firestoreService'), + slug: anyNamed('slug'), + sha: argThat(equals('c9affbbb12aa40cb3afbe94b9ea6b119a256bebf'), named: 'sha'), + stage: anyNamed('stage'), + tasks: argThat(equals(['Mac engine_build']), named: 'tasks'), + checkRunGuard: anyNamed('checkRunGuard'), + ), + ).called(1); + verify(mockGithubChecksUtil.createCheckRun(any, any, any, captureAny, output: captureAnyNamed('output'))) + .called(2); + final result = + verify(luci.scheduleMergeGroupBuilds(targets: captureAnyNamed('targets'), commit: anyNamed('commit'))); + expect(result.callCount, 1); + expect(result.captured[0].map((target) => target.value.name), ['Mac engine_build']); expect(checkRuns, hasLength(2)); verify( From e02b009e2a0ba75f9c34b315d500240fc2df3017 Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 3 Dec 2024 12:29:00 -0800 Subject: [PATCH 08/10] Feedback --- .../lib/src/service/luci_build_service.dart | 49 ++++++------------- .../test/service/luci_build_service_test.dart | 1 - 2 files changed, 16 insertions(+), 34 deletions(-) diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index a1faa51c5..41902e2c6 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -781,13 +781,14 @@ class LuciBuildService { }) async { final buildRequests = []; - Set availableBuilderSet; + final Set availableBuilderSet; try { availableBuilderSet = await getAvailableBuilderSet( project: 'flutter', bucket: 'prod', ); } catch (error) { + log.warning('Failed to get buildbucket builder list', error); throw 'Failed to get buildbucket builder list due to $error'; } log.info('Available builder list: $availableBuilderSet'); @@ -815,7 +816,7 @@ class LuciBuildService { final batchRequest = bbv2.BatchRequest(requests: buildRequests); log.fine(batchRequest); - List messageIds; + final List messageIds; try { messageIds = await pubsub.publish( @@ -1052,14 +1053,12 @@ class LuciBuildService { Future _createMergeGroupScheduleBuild({ required Commit commit, required Target target, - Map? properties, - List? tags, int priority = kDefaultPriority, }) async { log.info( 'Creating merge group schedule builder for ${target.value.name} on commit ${commit.sha}', ); - tags ??= []; + final tags = []; tags.addAll([ bbv2.StringPair( key: 'buildset', @@ -1069,6 +1068,18 @@ class LuciBuildService { key: 'buildset', value: 'commit/gitiles/flutter.googlesource.com/mirrors/${commit.slug.name}/+/${commit.sha}', ), + bbv2.StringPair( + key: 'user_agent', + value: 'flutter-cocoon', + ), + bbv2.StringPair( + key: 'scheduler_job_id', + value: 'flutter/${target.value.name}', + ), + bbv2.StringPair( + key: 'current_attempt', + value: '1', + ), ]); log.info( @@ -1083,35 +1094,7 @@ class LuciBuildService { rawUserData, ); - tags.add( - bbv2.StringPair( - key: 'user_agent', - value: 'flutter-cocoon', - ), - ); - // Tag `scheduler_job_id` is needed when calling buildbucket search build API. - tags.add( - bbv2.StringPair( - key: 'scheduler_job_id', - value: 'flutter/${target.value.name}', - ), - ); - // Default attempt is the initial attempt, which is 1. - final attemptTag = tags.singleWhereOrNull((tag) => tag.key == 'current_attempt'); - if (attemptTag == null) { - tags.add( - bbv2.StringPair( - key: 'current_attempt', - value: '1', - ), - ); - } - - final currentAttemptStr = tags.firstWhere((tag) => tag.key == 'current_attempt').value; - rawUserData['firestore_task_document_name'] = '${commit.sha}_${target.value.name}_$currentAttemptStr'; - final processedProperties = target.getProperties().cast(); - processedProperties.addAll(properties ?? {}); processedProperties['git_branch'] = commit.branch!; final cipdExe = 'refs/heads/${commit.branch}'; processedProperties['exe_cipd_version'] = cipdExe; diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart index 6dbcb55dc..6dbb9bd4e 100644 --- a/app_dart/test/service/luci_build_service_test.dart +++ b/app_dart/test/service/luci_build_service_test.dart @@ -1609,7 +1609,6 @@ void main() { 'commit_sha': 'abc1234', 'commit_branch': 'gh-readonly-queue/master/pr-1234-abcd', 'builder_name': builderName, - 'firestore_task_document_name': 'abc1234_${builderName}_1', }); final properties = scheduleBuild.properties.fields; From e9928bf648b780684f1d7a6f09b8df32b6924896 Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 3 Dec 2024 15:41:32 -0800 Subject: [PATCH 09/10] Add is_fusion property to build args --- app_dart/lib/src/service/luci_build_service.dart | 1 + app_dart/test/service/luci_build_service_test.dart | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app_dart/lib/src/service/luci_build_service.dart b/app_dart/lib/src/service/luci_build_service.dart index 41902e2c6..f8090c38c 100644 --- a/app_dart/lib/src/service/luci_build_service.dart +++ b/app_dart/lib/src/service/luci_build_service.dart @@ -1098,6 +1098,7 @@ class LuciBuildService { processedProperties['git_branch'] = commit.branch!; final cipdExe = 'refs/heads/${commit.branch}'; processedProperties['exe_cipd_version'] = cipdExe; + processedProperties['is_fusion'] = 'true'; final propertiesStruct = bbv2.Struct()..mergeFromProto3Json(processedProperties); final requestedDimensions = target.getDimensions(); diff --git a/app_dart/test/service/luci_build_service_test.dart b/app_dart/test/service/luci_build_service_test.dart index 6dbb9bd4e..d96b025bb 100644 --- a/app_dart/test/service/luci_build_service_test.dart +++ b/app_dart/test/service/luci_build_service_test.dart @@ -1567,12 +1567,14 @@ void main() { ); }); pubsub = FakePubSub(); + service = LuciBuildService( config: config, cache: cache, buildBucketClient: mockBuildBucketClient, githubChecksUtil: mockGithubChecksUtil, pubsub: pubsub, + fusionTester: FakeFusionTester()..isFusion = (_, __) => true, ); }); @@ -1621,6 +1623,7 @@ void main() { 'git_branch': bbv2.Value(stringValue: 'gh-readonly-queue/master/pr-1234-abcd'), 'exe_cipd_version': bbv2.Value(stringValue: 'refs/heads/gh-readonly-queue/master/pr-1234-abcd'), 'recipe': bbv2.Value(stringValue: 'devicelab/devicelab'), + 'is_fusion': bbv2.Value(stringValue: 'true'), }); expect(dimensions.length, 1); expect(dimensions[0].key, 'os'); From 2e897ec55795a5ff6a9676ed24c822550991efea Mon Sep 17 00:00:00 2001 From: John McDole Date: Wed, 4 Dec 2024 08:23:05 -0800 Subject: [PATCH 10/10] Feedback --- .../github/webhook_subscription.dart | 6 +++--- app_dart/lib/src/service/scheduler.dart | 21 ++++++------------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart index 4cac8f4a0..463e76e7c 100644 --- a/app_dart/lib/src/request_handlers/github/webhook_subscription.dart +++ b/app_dart/lib/src/request_handlers/github/webhook_subscription.dart @@ -239,10 +239,10 @@ class GithubWebhookSubscription extends SubscriptionHandler { if (!await _shaExistsInGob(slug, headSha)) { throw InternalServerError( - '$slug/$headSha was not found on GoB. Failing so this event can be retried...', + '$slug/$headSha was not found on GoB. Failing so this event can be retried', ); } - log.fine('$slug/$headSha was found on GoB mirror. Scheduling merge group tasks...'); + log.fine('$slug/$headSha was found on GoB mirror. Scheduling merge group tasks'); await scheduler.triggerMergeGroupTargets(mergeGroupEvent: mergeGroupEvent); // A merge group was deleted. This can happen when a PR is pulled from the @@ -250,7 +250,7 @@ class GithubWebhookSubscription extends SubscriptionHandler { // stopped to save CI resources, as Github will no longer merge this group // into the main branch. case 'destroyed': - log.fine('Destroying the merge group for $slug/$headSha'); + log.fine('Merge group destroyed for $slug/$headSha'); await scheduler.cancelMergeGroupTargets(headSha: headSha); } } diff --git a/app_dart/lib/src/service/scheduler.dart b/app_dart/lib/src/service/scheduler.dart index bcefeaa3e..e55f4cb4c 100644 --- a/app_dart/lib/src/service/scheduler.dart +++ b/app_dart/lib/src/service/scheduler.dart @@ -644,30 +644,22 @@ class Scheduler { ); late CiYamlSet ciYaml; - log.info('Attempting to read merge group targets from ci.yaml for $headSha'); if (commit.branch == Config.defaultBranch(commit.slug)) { ciYaml = await getCiYaml(commit, validate: true); } else { ciYaml = await getCiYaml(commit); } - log.info('ci.yaml loaded successfully.'); - log.info('Collecting merge group targets for $headSha'); + log.info('ci.yaml loaded successfully; collecting merge group targets for $headSha'); final inner = ciYaml.ciYamlFor(type); - // Filter out schedulers targets with schedulers different than luci or cocoon. - final List mergeGroupTargets = inner.presubmitTargets - .where( - (Target target) => - target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon, - ) - .toList(); - - log.info('Collected ${mergeGroupTargets.length} presubmit targets.'); - return mergeGroupTargets; + // Filter out targets with schedulers different than luci or cocoon. + bool filter(Target target) => + target.value.scheduler == pb.SchedulerSystem.luci || target.value.scheduler == pb.SchedulerSystem.cocoon; + return [...inner.presubmitTargets.where(filter)]; } - // Pretend the check took 1 minute to run + /// Pretends the merge group check took 1 minute to run. Future simulateMergeGroupUnlock( cocoon_checks.MergeGroup mergeGroup, RepositorySlug slug, @@ -675,7 +667,6 @@ class Scheduler { String headSha, CheckRun lock, ) async { - // Pretend the check took 1 minute to run await Future.delayed(debugCheckPretendDelay); final conclusion =