test: Isolate test repos

* Replace ReferenceRepository.ResetRepo with creation of a new instance
* Add missing calls to Dispose
* Add EpilogueAttribute in order to cleanup after ConfigureJoinableTaskFactory
* Skip cleanup of test repos in CI environment
* Reorder methods of GitModuleTestHelper

Refs: gitextensions#12553, fixes gitextensions#12559

Author: logiclrd

tests/CommonTestUtils/CommonTestUtils.csproj

@@ -1,6 +1,6 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
-  <PropertyGroup>   
+  <PropertyGroup>
     <!-- TODO once all project migrated to SDK-style, remove this and move properties to Directory.Build.props -->
     <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
 
@@ -12,6 +12,10 @@
     <IsTestProject>false</IsTestProject>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(ContinuousIntegrationBuild)' == 'true'">
+    <DefineConstants>$(DefineConstants);CI_BUILD</DefineConstants>
+  </PropertyGroup>
+
   <ItemGroup>
     <PackageReference Include="LibGit2Sharp" />
     <PackageReference Include="Microsoft.CodeAnalysis.Workspaces.Common" />

tests/CommonTestUtils/Epilogue.cs

@@ -0,0 +1,82 @@
+using System.ComponentModel;
+
+namespace CommonTestUtils;
+
+/// <summary>
+/// This class supports TestCleanUpAttribute. See TestCleanUpAttribute.cs
+/// for technical details.
+/// </summary>
+public static class Epilogue
+{
+    private static readonly Lock _sync = new();
+    private static readonly OrderedDictionary<int, Action> _afterSuiteActions = [];
+    private static readonly List<Action> _afterTestActions = [];
+
+    /// <summary>
+    /// Registers an action to be executed after the completion of
+    /// the test suite. Wait actions are executed synchronously in
+    /// the sequence indicated by <paramref name="order"/>. Values
+    /// of <paramref name="order"/> must be unique.The wait action
+    /// registration is permanent. The <see cref="ExecuteAfterTestActions"/> method is
+    /// called by <see cref="EpilogueAttribute"/> in order to execute
+    /// the current plan of action.
+    /// </summary>
+    /// <param name="order">The position in sequence to run this action.</param>
+    /// <param name="action">The action to be called.</param>
+    public static void RegisterAfterSuiteAction(int order, Action action)
+    {
+        lock (_sync)
+        {
+            _afterSuiteActions.Add(order, action);
+        }
+    }
+
+    /// <summary>
+    /// Registers an action to be performed after the completion of the
+    /// current test, including the completion of any
+    /// <see cref="NUnit.Framework.ITestAction.AfterTest(NUnit.Framework.Interfaces.ITest)"/>
+    /// callbacks. The <see cref="ExecuteAfterSuiteActions"/> method is
+    /// called by <see cref="EpilogueAttribute"/> in order to execute
+    /// the current plan of action. Clean-up actions are called only once
+    /// and need to be registered each time they are needed.
+    /// </summary>
+    /// <param name="action">The clean-up action to run after the current test.</param>
+    public static void RegisterAfterTestAction(Action action)
+    {
+        lock (_sync)
+        {
+            _afterTestActions.Add(action);
+        }
+    }
+
+    /// <summary>
+    /// This method supports the <see cref="Epilogue"/> infrastructure.
+    /// It is called by <see cref="EpilogueAttribute"/>, which is an
+    /// implementation of NUnit <see cref="NUnit.Framework.ITestAction"/>,
+    /// after the end of the current test.
+    /// </summary>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void ExecuteAfterTestActions()
+    {
+        lock (_sync)
+        {
+            _afterTestActions.ForEach(action => action());
+            _afterTestActions.Clear();
+        }
+    }
+
+    /// <summary>
+    /// This method supports the <see cref="Epilogue"/> infrastructure.
+    /// It is called by <see cref="EpilogueAttribute"/>, which is an
+    /// implementation of NUnit <see cref="NUnit.Framework.ITestAction"/>,
+    /// after the end of the test suite.
+    /// </summary>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void ExecuteAfterSuiteActions()
+    {
+        lock (_sync)
+        {
+            _afterSuiteActions.Values.ForEach(action => action());
+        }
+    }
+}

tests/CommonTestUtils/EpilogueAttribute.cs

@@ -0,0 +1,97 @@
+using NUnit.Framework;
+using NUnit.Framework.Interfaces;
+
+namespace CommonTestUtils;
+
+// Declare [assembly: TestCleanUp] in any test library that uses
+// GitModuleTestHelper or ReferenceRepository (which uses
+// GitModuleTestHelper internally). Otherwise, the temporary
+// repositories created in the temp folder for use in tests
+// won't get cleaned up.
+//
+// What this class does:
+//
+// Some tests create a physical Git repository on-disk as part of the
+// environment in which to perform the test. This needs to be cleaned
+// up. But, the test run's duration is measurably increased by doing
+// this clean-up synchronously. So, instead, the clean-up is run in a
+// background task. But, when the [ConfigureJoinableTaskFactory]
+// attribute is used, there is an AfterTest action that runs _after_
+// the test's TearDown is complete. The purpose of that AfterTest
+// action is to block until all tasks started within the test have
+// completed. If the TearDown kicks off clean-up of the repository,
+// it could delete the files out from under code that is still running.
+// This won't cause the test to fail, because it's a background task
+// fired up in the ambient ThreadHelper.JoinableTaskContext, but it
+// can cause unhandled exceptions to occur, resulting in a messagebox
+// appearing in the middle of the test run. And, this exception blocks
+// the worker task, and the AfterTest hook supplied by the
+// [ConfigureJoinableTaskFactory] joins on the worker task, so the test
+// run is blocked from continuing until the exception dialog is dealt
+// with.
+//
+// To resolve this issue, clean-up actions are deferred until after
+// the AfterTest hook from [ConfigureJoinableTestFactory] has finished
+// joining the threads. This is done by declaring this attribute before
+// the [ConfigureJoinableTestFactory]; this causes NUnit to process
+// this ITestAction for BeforeTest first and AfterTest last.
+//
+// ThreadCleanUp.BeforeTest
+//   ConfigureJoinableTestFactory.BeforeTest
+//     Test.SetUp
+//     Test
+//     Test.TearDown
+//   ConfigureJoinableTestFactory.AfterTest -> wait for detached tasks
+// ThreadCleanUp.AfterTest -> kick off clean-up tasks
+//
+// The clean-up tasks are thus kept from interfering with detached
+// operations started during the test.
+//
+// It is possible at the end of the test suite execution for a clean-
+// up action to still be in progress when the test process is ready
+// to exit. There is a theoretical possibility that a clean-up action
+// might fail to run or be interrupted. To avoid this possibility,
+// TestCleanUpAttribute also registers with NUnit for the Suite
+// target. This allows it to receive BeforeTest and AfterTest
+// notifications for the overall test suite, and the AfterTest
+// callback can be used to wait until all ongoing clean-up operations
+// are complete.
+//
+// ThreadCleanUp.BeforeTest (suite)
+//   ThreadCleanUp.BeforeTest
+//     ConfigureJoinableTestFactory.BeforeTest
+//       Test.SetUp
+//       Test
+//       Test.TearDown
+//     ConfigureJoinableTestFactory.AfterTest -> wait for detached tasks
+//   ThreadCleanUp.AfterTest -> kick off clean-up tasks
+//   ...
+//   ThreadCleanUp.BeforeTest
+//     ConfigureJoinableTestFactory.BeforeTest
+//       Test.SetUp
+//       Test
+//       Test.TearDown
+//     ConfigureJoinableTestFactory.AfterTest -> wait for detached tasks
+//   ThreadCleanUp.AfterTest -> kick off clean-up tasks
+// ThreadCleanUp.AfterTest (suite) -> wait for clean-up tasks before exit
+[AttributeUsage(AttributeTargets.Assembly)]
+public class EpilogueAttribute : Attribute, ITestAction
+{
+    public ActionTargets Targets => ActionTargets.Suite | ActionTargets.Test;
+
+    public void BeforeTest(ITest test)
+    {
+    }
+
+    public void AfterTest(ITest test)
+    {
+        if (test.IsSuite)
+        {
+            Epilogue.ExecuteAfterSuiteActions();
+        }
+        else
+        {
+            Epilogue.ExecuteAfterTestActions();
+        }
+    }
+}

tests/CommonTestUtils/GitModuleTestHelper.Cleanup.cs

@@ -0,0 +1,78 @@
+using GitUI;
+using Microsoft.VisualStudio.Threading;
+
+namespace CommonTestUtils;
+
+public partial class GitModuleTestHelper : IDisposable
+{
+#if CI_BUILD
+    static GitModuleTestHelper()
+    {
+        Console.WriteLine("GitModuleTestHelper: Disabling explicit clean-up for continuous integration test environment");
+    }
+
+    private void FileAndForgetCleanUp()
+    {
+    }
+
+    public static void WaitForCleanUpCompletion()
+    {
+    }
+#else
+    private static readonly TaskManager _cleanUpOperations = new(new JoinableTaskContext());
+
+    static GitModuleTestHelper()
+    {
+        Console.WriteLine("GitModuleTestHelper: Will perform clean-up in background tasks");
+
+        Epilogue.RegisterAfterSuiteAction(order: 2, WaitForCleanUpCompletion);
+    }
+
+    private static void CleanUp(string path)
+    {
+        try
+        {
+            // Directory.Delete seems to intermittently fail, so delete the files first before deleting folders
+            foreach (string file in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories))
+            {
+                if (File.GetAttributes(file).HasFlag(FileAttributes.ReparsePoint))
+                {
+                    continue;
+                }
+
+                File.SetAttributes(file, FileAttributes.Normal);
+                File.Delete(file);
+            }
+
+            // Delete tends to fail on the first try, so give it a few tries as a best effort.
+            // By this point, all files have been deleted anyway, so this is mainly about removing
+            // empty directories.
+            for (int tries = 0; tries < 10; ++tries)
+            {
+                try
+                {
+                    Directory.Delete(path, true);
+                    break;
+                }
+                catch
+                {
+                }
+            }
+        }
+        catch
+        {
+            // do nothing
+        }
+    }
+
+    private void FileAndForgetCleanUp()
+    {
+        Epilogue.RegisterAfterTestAction(() => { _cleanUpOperations.FileAndForget(() => CleanUp(TemporaryPath)); });
+    }
+
+    public static void WaitForCleanUpCompletion()
+    {
+        _cleanUpOperations.JoinPendingOperations();
+    }
+#endif
+}