diff --git a/src/Maestro/SubscriptionActorService/PullRequestActor.cs b/src/Maestro/SubscriptionActorService/PullRequestActor.cs index 3f66247936..39b0d71ac4 100644 --- a/src/Maestro/SubscriptionActorService/PullRequestActor.cs +++ b/src/Maestro/SubscriptionActorService/PullRequestActor.cs @@ -708,7 +708,7 @@ public async Task> UpdateAssetsAsync( IRemote darcRemote = await _remoteFactory.GetRemoteAsync(targetRepository, _logger); TargetRepoDependencyUpdate repoDependencyUpdate = - await GetRequiredUpdates(updates, _remoteFactory, targetRepository, targetBranch); + await GetRequiredUpdates(updates, _remoteFactory, targetRepository, prBranch: null, targetBranch); if (repoDependencyUpdate.CoherencyCheckSuccessful && repoDependencyUpdate.RequiredUpdates.Count < 1) { @@ -807,9 +807,10 @@ private async Task UpdatePullRequestAsync(InProgressPullRequest pr, List previousSubscriptions = [.. pr.ContainedSubscriptions]; // Update the list of contained subscriptions with the new subscription update. @@ -877,7 +875,7 @@ await AddDependencyFlowEventsAsync( targetRepositoryUpdates.RequiredUpdates, pullRequest.Description, targetRepository, - headBranch); + pullRequest.HeadBranch); pullRequest.Title = await _pullRequestBuilder.GeneratePRTitleAsync(pr, targetBranch); @@ -950,16 +948,17 @@ private async Task GetRequiredUpdates( List updates, IRemoteFactory remoteFactory, string targetRepository, - string branch) + string? prBranch, + string targetBranch) { - _logger.LogInformation("Getting Required Updates from {branch} to {targetRepository}", branch, targetRepository); + _logger.LogInformation("Getting Required Updates for {branch} of {targetRepository}", targetBranch, targetRepository); // Get a remote factory for the target repo IRemote darc = await remoteFactory.GetRemoteAsync(targetRepository, _logger); TargetRepoDependencyUpdate repoDependencyUpdate = new(); // Existing details - List existingDependencies = (await darc.GetDependenciesAsync(targetRepository, branch)).ToList(); + List existingDependencies = (await darc.GetDependenciesAsync(targetRepository, prBranch ?? targetBranch)).ToList(); foreach (UpdateAssetsParameters update in updates) { @@ -1005,13 +1004,13 @@ await UpdateSubscriptionsForMergedPRAsync( List coherencyUpdates = []; try { - _logger.LogInformation($"Running a coherency check on the existing dependencies for branch {branch} of repo {targetRepository}"); + _logger.LogInformation($"Running a coherency check on the existing dependencies for branch {targetBranch} of repo {targetRepository}"); coherencyUpdates = await _coherencyUpdateResolver.GetRequiredCoherencyUpdatesAsync(existingDependencies, remoteFactory); } catch (DarcCoherencyException e) { _logger.LogInformation("Failed attempting strict coherency update on branch '{strictCoherencyFailedBranch}' of repo '{strictCoherencyFailedRepo}'.", - branch, targetRepository); + targetBranch, targetRepository); repoDependencyUpdate.CoherencyCheckSuccessful = false; repoDependencyUpdate.CoherencyErrors = e.Errors.Select(e => new CoherencyErrorDetails { @@ -1031,7 +1030,7 @@ await UpdateSubscriptionsForMergedPRAsync( repoDependencyUpdate.RequiredUpdates.Add((coherencyUpdateParameters, coherencyUpdates.ToList())); } - _logger.LogInformation("Finished getting Required Updates from {branch} to {targetRepository}", branch, targetRepository); + _logger.LogInformation("Finished getting Required Updates for {branch} of {targetRepository}", targetBranch, targetRepository); return repoDependencyUpdate; } diff --git a/test/SubscriptionActorService.Tests/PendingUpdatesTests.cs b/test/SubscriptionActorService.Tests/PendingUpdatesTests.cs index 4476d28d21..dfc3854f51 100644 --- a/test/SubscriptionActorService.Tests/PendingUpdatesTests.cs +++ b/test/SubscriptionActorService.Tests/PendingUpdatesTests.cs @@ -69,7 +69,7 @@ public async Task PendingUpdatesUpdatablePr() await WhenProcessPendingUpdatesAsyncIsCalled(); ThenUpdateReminderIsRemoved(); AndPendingUpdateIsRemoved(); - ThenGetRequiredUpdatesShouldHaveBeenCalled(b); + ThenGetRequiredUpdatesShouldHaveBeenCalled(b, true); AndCommitUpdatesShouldHaveBeenCalled(b); AndUpdatePullRequestShouldHaveBeenCalled(); AndShouldHavePullRequestCheckReminder(); diff --git a/test/SubscriptionActorService.Tests/PullRequestActorTests.cs b/test/SubscriptionActorService.Tests/PullRequestActorTests.cs index 917814843c..0f1bad393c 100644 --- a/test/SubscriptionActorService.Tests/PullRequestActorTests.cs +++ b/test/SubscriptionActorService.Tests/PullRequestActorTests.cs @@ -122,14 +122,14 @@ protected void GivenAPullRequestCheckReminder() Reminders.Data[PullRequestActorImplementation.PullRequestCheckKey] = reminder; } - protected void ThenGetRequiredUpdatesShouldHaveBeenCalled(Build withBuild) + protected void ThenGetRequiredUpdatesShouldHaveBeenCalled(Build withBuild, bool prExists) { var assets = new List>(); var dependencies = new List>(); _updateResolver .Verify(r => r.GetRequiredNonCoherencyUpdates(SourceRepo, NewCommit, Capture.In(assets), Capture.In(dependencies))); _darcRemotes[TargetRepo] - .Verify(r => r.GetDependenciesAsync(TargetRepo, TargetBranch, null)); + .Verify(r => r.GetDependenciesAsync(TargetRepo, prExists ? InProgressPrHeadBranch : TargetBranch, null)); _updateResolver .Verify(r => r.GetRequiredCoherencyUpdatesAsync(Capture.In(dependencies), _remoteFactory.Object)); assets.Should() diff --git a/test/SubscriptionActorService.Tests/UpdateAssetsTests.cs b/test/SubscriptionActorService.Tests/UpdateAssetsTests.cs index 747d3892a3..c00d810efe 100644 --- a/test/SubscriptionActorService.Tests/UpdateAssetsTests.cs +++ b/test/SubscriptionActorService.Tests/UpdateAssetsTests.cs @@ -34,7 +34,7 @@ public async Task UpdateWithAssetsNoExistingPR(bool batchable) await WhenUpdateAssetsAsyncIsCalled(b); - ThenGetRequiredUpdatesShouldHaveBeenCalled(b); + ThenGetRequiredUpdatesShouldHaveBeenCalled(b, false); AndCreateNewBranchShouldHaveBeenCalled(); AndCommitUpdatesShouldHaveBeenCalled(b); AndCreatePullRequestShouldHaveBeenCalled(); @@ -62,7 +62,7 @@ public async Task UpdateWithAssetsExistingPR(bool batchable) using (WithExistingPullRequest(SynchronizePullRequestResult.InProgressCanUpdate)) { await WhenUpdateAssetsAsyncIsCalled(b); - ThenGetRequiredUpdatesShouldHaveBeenCalled(b); + ThenGetRequiredUpdatesShouldHaveBeenCalled(b, true); AndCommitUpdatesShouldHaveBeenCalled(b); AndUpdatePullRequestShouldHaveBeenCalled(); AndShouldHavePullRequestCheckReminder(); @@ -110,7 +110,7 @@ public async Task UpdateWithNoAssets(bool batchable) await WhenUpdateAssetsAsyncIsCalled(b); - ThenGetRequiredUpdatesShouldHaveBeenCalled(b); + ThenGetRequiredUpdatesShouldHaveBeenCalled(b, false); AndSubscriptionShouldBeUpdatedForMergedPullRequest(b); } @@ -134,7 +134,7 @@ public async Task UpdateWithAssetsWhenStrictAlgorithmFails(bool batchable) await WhenUpdateAssetsAsyncIsCalled(b); - ThenGetRequiredUpdatesShouldHaveBeenCalled(b); + ThenGetRequiredUpdatesShouldHaveBeenCalled(b, false); AndCreateNewBranchShouldHaveBeenCalled(); AndCommitUpdatesShouldHaveBeenCalled(b); AndCreatePullRequestShouldHaveBeenCalled();