From 0d9758ff9d3f84a2d40422bd9fa2b8b791c12ca5 Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa Date: Mon, 3 Nov 2025 16:51:36 +0100 Subject: [PATCH 1/2] Modify Scenario Test to check tagged pull requests --- .../CodeFlowScenarioTestBase.cs | 8 ++- .../ScenarioTestBase.cs | 56 +++++++++++++++++-- .../ScenarioTests/ScenarioTests_CodeFlow.cs | 11 +++- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs index e75c41bda8..e99a6728a6 100644 --- a/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs +++ b/test/ProductConstructionService.ScenarioTests/CodeFlowScenarioTestBase.cs @@ -19,7 +19,8 @@ protected async Task CheckForwardFlowGitHubPullRequest( string targetRepoName, string targetBranch, string[] testFiles, - Dictionary testFilePatches) + Dictionary testFilePatches, + PullRequestCleanupOperation cleanupOperation = PullRequestCleanupOperation.Close) { // When we expect updates from multiple repos (batchable subscriptions), we need to wait until the PR gets updated with the second repository after it is created // Otherwise it might try to validate the contents before all updates are in place @@ -27,7 +28,7 @@ protected async Task CheckForwardFlowGitHubPullRequest( ? await WaitForUpdatedPullRequestAsync(targetRepoName, targetBranch) : await WaitForPullRequestAsync(targetRepoName, targetBranch); - await using (CleanUpPullRequestAfter(TestParameters.GitHubTestOrg, targetRepoName, pullRequest)) + await using (CleanUpPullRequestAfter(TestParameters.GitHubTestOrg, targetRepoName, pullRequest, cleanupOperation)) { IReadOnlyList files = await GitHubApi.PullRequest.Files( TestParameters.GitHubTestOrg, @@ -200,9 +201,10 @@ public async Task WaitForPullRequestComment( string targetRepo, string targetBranch, string partialComment, + ItemStateFilter prState = ItemStateFilter.Open, int attempts = 40) { - PullRequest pullRequest = await WaitForPullRequestAsync(targetRepo, targetBranch); + PullRequest pullRequest = await WaitForPullRequestAsync(targetRepo, targetBranch, prState); while (attempts-- > 0) { diff --git a/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs b/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs index ebd22e236c..1d178893cb 100644 --- a/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs +++ b/test/ProductConstructionService.ScenarioTests/ScenarioTestBase.cs @@ -12,12 +12,12 @@ using Microsoft.DotNet.DarcLib.Models.AzureDevOps; using Microsoft.DotNet.DarcLib.Models.Darc; using Microsoft.DotNet.Internal.Testing.Utility; +using Microsoft.DotNet.ProductConstructionService.Client; +using Microsoft.DotNet.ProductConstructionService.Client.Models; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; using NuGet.Configuration; using NUnit.Framework; -using Microsoft.DotNet.ProductConstructionService.Client; -using Microsoft.DotNet.ProductConstructionService.Client.Models; using ProductConstructionService.ScenarioTests.ObjectHelpers; [assembly: Parallelizable(ParallelScope.Fixtures)] @@ -46,7 +46,7 @@ public void BaseSetup() _packageNameSalt = Guid.NewGuid().ToString().Substring(0, 8); } - protected async Task WaitForPullRequestAsync(string targetRepo, string targetBranch) + protected async Task WaitForPullRequestAsync(string targetRepo, string targetBranch, Octokit.ItemStateFilter prState = Octokit.ItemStateFilter.Open) { Octokit.Repository repo = await GitHubApi.Repository.Get(TestParameters.GitHubTestOrg, targetRepo); @@ -56,6 +56,7 @@ public void BaseSetup() IReadOnlyList prs = await GitHubApi.PullRequest.GetAllForRepository(repo.Id, new Octokit.PullRequestRequest { Base = targetBranch, + State = prState }); if (prs.Count == 1) @@ -1189,10 +1190,21 @@ protected string GetUniqueAssetName(string packageName) return $"{packageName}.{_packageNameSalt}"; } - protected static IAsyncDisposable CleanUpPullRequestAfter(string owner, string repo, Octokit.PullRequest pullRequest) + protected static IAsyncDisposable CleanUpPullRequestAfter( + string owner, + string repo, + Octokit.PullRequest pullRequest, + PullRequestCleanupOperation cleanupOperation = PullRequestCleanupOperation.Close) => AsyncDisposable.Create(async () => { - await ClosePullRequest(owner, repo, pullRequest); + if (cleanupOperation == PullRequestCleanupOperation.Merge) + { + await MergePullRequest(owner, repo, pullRequest); + } + else + { + await ClosePullRequest(owner, repo, pullRequest); + } }); protected static async Task ClosePullRequest(string owner, string repo, Octokit.PullRequest pullRequest) @@ -1220,6 +1232,34 @@ protected static async Task ClosePullRequest(string owner, string repo, Octokit. } } + protected static async Task MergePullRequest(string owner, string repo, Octokit.PullRequest pullRequest) + { + try + { + await GitHubApi.PullRequest.Merge( + owner, + repo, + pullRequest.Number, + new Octokit.MergePullRequest + { + CommitMessage = "Merge pull request", + MergeMethod = Octokit.PullRequestMergeMethod.Merge + }); + } + catch + { + // Closed already + } + try + { + await GitHubApi.Git.Reference.Delete(owner, repo, $"heads/{pullRequest.Head.Ref}"); + } + catch + { + // branch already deleted + } + } + protected static async Task CreateTargetBranchAndExecuteTest(string targetBranchName, string targetDirectory, Func test) { // first create a new target branch @@ -1285,3 +1325,9 @@ protected static async Task WaitForNewCommitInPullRequest(string repo, Octokit.P return pr; } } + +internal enum PullRequestCleanupOperation +{ + Close, + Merge +} diff --git a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs index 836c15a123..157c21ebe8 100644 --- a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs +++ b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs @@ -98,12 +98,19 @@ await CreateTargetBranchAndExecuteTest(targetBranchName, vmrDirectory.Directory, TestRepository.VmrTestRepoName, targetBranchName, [$"src/{TestRepository.TestRepo1Name}/{TestFile1Name}"], - TestFilePatches); + TestFilePatches, + PullRequestCleanupOperation.Merge); await CheckIfPullRequestCommentExists( TestRepository.VmrTestRepoName, pr, - [$"https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber}"]); + [$"(https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber})"]); + + await WaitForPullRequestComment( + TestRepository.VmrTestRepoName, + targetBranchName, + $"- https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber}", + Octokit.ItemStateFilter.Closed); } } } From 139b3ad0394a2f195c5187d5ae87ad2c1a96361c Mon Sep 17 00:00:00 2001 From: Djuradj Kurepa Date: Mon, 3 Nov 2025 16:55:05 +0100 Subject: [PATCH 2/2] Add comments --- .../ScenarioTests/ScenarioTests_CodeFlow.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs index 157c21ebe8..37582e43fc 100644 --- a/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs +++ b/test/ProductConstructionService.ScenarioTests/ScenarioTests/ScenarioTests_CodeFlow.cs @@ -104,11 +104,13 @@ await CreateTargetBranchAndExecuteTest(targetBranchName, vmrDirectory.Directory, await CheckIfPullRequestCommentExists( TestRepository.VmrTestRepoName, pr, + // here we're testing for a URI with () around, since that's what we write during the codeflow udpates [$"(https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber})"]); await WaitForPullRequestComment( TestRepository.VmrTestRepoName, targetBranchName, + // here we test for this specific format because that's how we tag PRs after merging the forward flow $"- https://github.com/maestro-auth-test/maestro-test1/pull/{testPrNumber}", Octokit.ItemStateFilter.Closed); }