Fix E2E code flow tests (fetching refs, updating version files) (#4190)

dotnet · Nov 28, 2024 · 4dd2d58 · 4dd2d58
1 parent 5ba7d22
commit 4dd2d58
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 33 deletions.
diff --git a/docs/DevGuide.md b/docs/DevGuide.md
@@ -17,6 +17,7 @@
       ('https://github.com/maestro-auth-test/maestro-test', 289474),
       ('https://github.com/maestro-auth-test/maestro-test2', 289474),
       ('https://github.com/maestro-auth-test/maestro-test3', 289474),
+      ('https://github.com/maestro-auth-test/maestro-test-vmr', 289474),
       ('https://github.com/maestro-auth-test/arcade', 289474),
       ('https://github.com/maestro-auth-test/dnceng-vmr', 289474);
   ```

diff --git a/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/ILocalGitClient.cs
@@ -140,6 +140,14 @@ Task CommitAsync(
     /// <returns>List of currently modified staged files</returns>
     Task<string[]> GetStagedFiles(string repoPath);
 
+    /// <summary>
+    ///     Determines if a given path is a git repository.
+    /// </summary>
+    /// <param name="repoPath">Path to a git repository</param>
+    /// <param name="gitRef">Git reference to check for</param>
+    /// <returns>True if the path is a git repository, false otherwise</returns>
+    Task<bool> GitRefExists(string repoPath, string gitRef, CancellationToken cancellationToken = default);
+
     /// <summary>
     /// Fetches from all remotes.
     /// </summary>

diff --git a/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs b/src/Microsoft.DotNet.Darc/DarcLib/LocalGitClient.cs
@@ -210,7 +210,6 @@ public async Task<GitObjectType> GetObjectTypeAsync(string repoPath, string obje
         };
 
         var result = await _processManager.ExecuteGit(repoPath, args);
-        result.ThrowIfFailed($"Failed to find object {objectSha} in {repoPath}");
 
         return result.StandardOutput.Trim() switch
         {
@@ -443,6 +442,22 @@ public async Task<string> BlameLineAsync(string repoPath, string relativeFilePat
         return result.StandardOutput.Trim().Split(' ').First();
     }
 
+    public async Task<bool> GitRefExists(string repoPath, string gitRef, CancellationToken cancellationToken = default)
+    {
+        // If the ref is a SHA or local branch/tag, we can check it directly via git cat-file -t
+        var objectType = await GetObjectTypeAsync(repoPath, gitRef);
+        if (objectType != GitObjectType.Unknown)
+        {
+            return true;
+        }
+
+        // If it's a remote branch that has been fetched git cat-file -t won't work,
+        // because we would have to query for [remote name]/gitRef
+        var result = await RunGitCommandAsync(repoPath, ["branch", "-a", "--list", "*/" + gitRef], cancellationToken);
+        result.ThrowIfFailed($"Failed to verify if git ref '{gitRef}' exists in {repoPath}");
+        return result.StandardOutput.Contains(gitRef);
+    }
+
     public async Task<bool> HasWorkingTreeChangesAsync(string repoPath)
     {
         var result = await _processManager.ExecuteGit(repoPath, ["diff", "--exit-code"]);

diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/CloneManager.cs
@@ -76,28 +76,17 @@ protected async Task<ILocalGitRepo> PrepareCloneInternalAsync(
             // Path should be returned the same for all invocations
             // We checkout a default ref
             path = await PrepareCloneInternal(remoteUri, dirName, cancellationToken);
-            var missingCommit = false;
 
             // Verify that all requested commits are available
-            foreach (string commit in refsToVerify.ToArray())
+            foreach (string gitRef in refsToVerify.ToArray())
             {
-                try
+                if (await _localGitRepo.GitRefExists(path, gitRef, cancellationToken))
                 {
-                    var objectType = await _localGitRepo.GetObjectTypeAsync(path, commit);
-                    if (objectType == GitObjectType.Commit)
-                    {
-                        refsToVerify.Remove(commit);
-                    }
-                }
-                catch
-                {
-                    // Ref not found yet, let's try another remote
-                    missingCommit = true;
-                    break;
+                    refsToVerify.Remove(gitRef);
                 }
             }
 
-            if (!missingCommit)
+            if (!refsToVerify.Any())
             {
                 _logger.LogDebug("All requested refs ({refs}) found in {repo}", string.Join(", ", requestedRefs), path);
                 break;

diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrDependencyTracker.cs
@@ -45,7 +45,6 @@ public class VmrDependencyTracker : IVmrDependencyTracker
 {
     private readonly AllVersionsPropsFile _repoVersions;
     private readonly ISourceManifest _sourceManifest;
-    private readonly LocalPath _allVersionsFilePath;
     private readonly IVmrInfo _vmrInfo;
     private readonly IFileSystem _fileSystem;
     private readonly ISourceMappingParser _sourceMappingParser;
@@ -63,7 +62,6 @@ public VmrDependencyTracker(
         ISourceManifest sourceManifest)
     {
         _vmrInfo = vmrInfo;
-        _allVersionsFilePath = vmrInfo.VmrPath / VmrInfo.GitInfoSourcesDir / AllVersionsPropsFile.FileName;
         _sourceManifest = sourceManifest;
         _repoVersions = new AllVersionsPropsFile(sourceManifest.Repositories);
         _fileSystem = fileSystem;
@@ -94,7 +92,7 @@ public async Task InitializeSourceMappings(string? sourceMappingsPath = null)
     public void UpdateDependencyVersion(VmrDependencyUpdate update)
     {
         _repoVersions.UpdateVersion(update.Mapping.Name, update.TargetRevision, update.TargetVersion);
-        _repoVersions.SerializeToXml(_allVersionsFilePath);
+        _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath);
 
         _sourceManifest.UpdateVersion(update.Mapping.Name, update.RemoteUri, update.TargetRevision, update.TargetVersion);
         _fileSystem.WriteToFile(_vmrInfo.SourceManifestPath, _sourceManifest.ToJson());
@@ -126,7 +124,7 @@ public bool RemoveRepositoryVersion(string repo)
 
         if (_repoVersions.DeleteVersion(repo))
         {
-            _repoVersions.SerializeToXml(_allVersionsFilePath);
+            _repoVersions.SerializeToXml(_vmrInfo.AllVersionsFilePath);
             hasChanges = true;
         }
 

diff --git a/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs b/src/Microsoft.DotNet.Darc/DarcLib/VirtualMonoRepo/VmrInfo.cs
@@ -59,6 +59,11 @@ public interface IVmrInfo
     /// Gets a full path leading to sources belonging to a given repo
     /// </summary>
     NativePath GetRepoSourcesPath(string mappingName);
+
+    /// <summary>
+    /// Path to the AllRepoVersions.props
+    /// </summary>
+    NativePath AllVersionsFilePath { get; }
 }
 
 public class VmrInfo : IVmrInfo
@@ -137,4 +142,6 @@ public VmrInfo(NativePath vmrPath, NativePath tmpPath)
     public static UnixPath GetRelativeRepoSourcesPath(string mappingName) => RelativeSourcesDir / mappingName;
 
     public NativePath SourceManifestPath { get; private set; }
+
+    public NativePath AllVersionsFilePath => VmrPath / GitInfoSourcesDir / AllVersionsPropsFile.FileName;
 }
diff --git a/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs b/src/ProductConstructionService/ProductConstructionService.WorkItems/ReminderManager.cs
@@ -50,7 +50,7 @@ public async Task UnsetReminderAsync(bool isCodeFlow)
         {
             await client.DeleteWorkItemAsync(receipt.MessageId, receipt.PopReceipt);
         }
-        catch (RequestFailedException e) when (e.Message.Contains("The specified message does not exist") || e.Message.Contains("did not match the pop receipt"))
+        catch (RequestFailedException e) when (e.ErrorCode == "MessageNotFound" || e.ErrorCode == "PopReceiptMismatch")
         {
             // The message was already deleted, so we can ignore this exception.
         }

diff --git a/src/ProductConstructionService/Readme.md b/src/ProductConstructionService/Readme.md
@@ -17,6 +17,7 @@
       ('https://github.com/maestro-auth-test/maestro-test', 289474),
       ('https://github.com/maestro-auth-test/maestro-test2', 289474),
       ('https://github.com/maestro-auth-test/maestro-test3', 289474),
+      ('https://github.com/maestro-auth-test/maestro-test-vmr', 289474),
       ('https://github.com/maestro-auth-test/arcade', 289474),
       ('https://github.com/maestro-auth-test/dnceng-vmr', 289474);
   ```

diff --git a/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs b/test/Microsoft.DotNet.Darc.VirtualMonoRepo.E2E.Tests/VmrTwoWayCodeflowTest.cs
@@ -124,8 +124,8 @@ repo                   VMR
           │ x─┘ 5.       7. x   │   
           │ │               │   │   
         6.O◄┘               └──►O 8.
-          │   9.                │   
-          │    ◄────────────────┤   
+          │                     │   
+          |────────────────────►O 9.
           │                     │   
      */
     [Test]
@@ -178,7 +178,9 @@ await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName,
         // While the VMR accepted the content from the repo but it will get overriden by the VMR content again
         var hadUpdates = await CallDarcForwardflow(Constants.ProductRepoName, ProductRepoPath, branch: forwardBranchName);
         hadUpdates.ShouldHaveUpdates();
-        await GitOperations.MergePrBranch(VmrPath, forwardBranchName);
+        await GitOperations.VerifyMergeConflict(VmrPath, forwardBranchName,
+            mergeTheirs: true,
+            expectedConflictingFile: VmrInfo.SourcesDir / Constants.ProductRepoName / _productRepoFileName);
 
         // Both VMR and repo need to have the version from the VMR as it flowed to the repo and back
         CheckFileContents(_productRepoFilePath, "New content from the VMR #2");

diff --git a/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs b/test/Microsoft.DotNet.DarcLib.Tests/VirtualMonoRepo/RepositoryCloneManagerTests.cs
@@ -276,14 +276,21 @@ public async Task CommitIsNotFound()
 
         var remotes = configuration.Values.Select(x => x.RemoteUri).ToArray();
 
-        var action = async() => await _manager.PrepareCloneAsync(mapping, remotes, new[] { "sha1", "sha2", "sha4" }, "main", default);
+        var searchedRefs = new[] { "sha1", "sha2", "sha4" };
+        var action = async() => await _manager.PrepareCloneAsync(mapping, remotes, searchedRefs, "main", default);
         await action.Should().ThrowAsync<Exception>("because sha4 is not present anywhere");
 
         foreach (var pair in configuration)
         {
             _localGitRepo
                 .Verify(x => x.AddRemoteIfMissingAsync(clonePath, pair.Value.RemoteUri, It.IsAny<CancellationToken>()), Times.Once);
         }
+
+        foreach (var sha in searchedRefs)
+        {
+            _localGitRepo
+                .Verify(x => x.GitRefExists(clonePath, sha, It.IsAny<CancellationToken>()), Times.AtLeastOnce);
+        }
     }
 
     /// <summary>
@@ -314,15 +321,11 @@ private void SetupLazyFetching(string clonePath, Dictionary<string, RemoteState>
                 .Returns(Task.CompletedTask);
 
             _localGitRepo
-                .Setup(x => x.GetObjectTypeAsync(clonePath, It.IsAny<string>()))
-                .Callback((string _, string sha) =>
+                .Setup(x => x.GitRefExists(clonePath, It.IsAny<string>(), It.IsAny<CancellationToken>()))
+                .ReturnsAsync((string _, string sha, CancellationToken __) =>
                 {
-                    if (!configuration.Any(p => p.Value.CommitsContained.Contains(sha) && p.Value.IsCloned))
-                    {
-                        throw new Exception($"Could not find {sha}");
-                    }
-                })
-                .ReturnsAsync(GitObjectType.Commit);
+                    return configuration.Any(p => p.Value.CommitsContained.Contains(sha) && p.Value.IsCloned);
+                });
         }
     }