Tests: the ManagedConf crash hook and the Lite time settings are no longer shared between parallel test classes (#4773, #4776)

Darling/Darling.Tests/ManagedConfCrashHookCollection.cs

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// Serializes the test classes that set <c>ManagedConfMigrationSteps.FailBetweenSteps</c> (#4773), the same
+/// reason <c>PgFileSettingsStaticsCollection</c> exists for its capability statics. The hook is one static
+/// field shared by the whole test process, and any <c>WriteTwoSteps</c> call that runs while it is set throws the
+/// simulated crash. Two classes set it to throw, and run in parallel one class's crash landed inside the other
+/// class's test: <c>ManagedConfMigrationRunnerTests.RunStepA_Mismatch_RestoresAndListsTheKey</c> failed with
+/// the steps class's "simulated crash between steps". <see cref="CollectionDefinitionAttribute.DisableParallelization"/>
+/// also keeps every other class off the hook while these run, including a class that reaches
+/// <c>WriteTwoSteps</c> only through <c>DarlingManagedPostgres</c>.
+/// </summary>
+[CollectionDefinition("managed-conf-crash-hook", DisableParallelization = true)]
+public sealed class ManagedConfCrashHookCollection
+{
+}

Darling/Darling.Tests/ManagedConfCrashHookCollectionTests.cs

@@ -0,0 +1,123 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Text.RegularExpressions;
+using Xunit;
+
+namespace Darling.Tests;
+
+/// <summary>
+/// #4773: <c>ManagedConfMigrationSteps.FailBetweenSteps</c> is one static field for the whole test process, and
+/// two test classes set it to throw. If they run at the same time, one class's simulated crash lands inside the
+/// other class's <c>WriteTwoSteps</c> call and fails a test that never asked for it. Both classes carry
+/// <c>[Collection("managed-conf-crash-hook")]</c>, a collection that also runs alone, so nothing else can meet
+/// the hook. These two tests fail when that stops being true, whether by removing the attribute, by dropping
+/// <c>DisableParallelization</c>, or by adding a third class that sets the hook without joining the collection.
+/// </summary>
+public sealed class ManagedConfCrashHookCollectionTests
+{
+    private const string CollectionName = "managed-conf-crash-hook";
+
+    /// <summary>
+    /// An assignment of anything but <c>null</c> to the hook: the assignment that arms a crash. <c>(?!=)</c> keeps
+    /// out a comparison (<c>== null</c>). The last lookahead also refuses whitespace, so the run of spaces before
+    /// the value cannot give characters back and let <c>null</c> look like something else: it only passes once
+    /// it has seen past all of them.
+    /// </summary>
+    private static readonly Regex ArmsTheHook = new(
+        @"ManagedConfMigrationSteps\s*\.\s*FailBetweenSteps\s*=(?!=)\s*(?!\s|null\b)",
+        RegexOptions.CultureInvariant);
+
+    [Fact]
+    public void TheClassesThatArmTheHook_ShareOneCollection_ThatRunsAlone()
+    {
+        foreach (var type in new[] { typeof(ManagedConfMigrationStepsTests), typeof(ManagedConfMigrationRunnerTests) })
+        {
+            var collection = type.GetCustomAttribute<CollectionAttribute>();
+            Assert.True(
+                collection is not null && collection.Name == CollectionName,
+                $"{type.Name} sets ManagedConfMigrationSteps.FailBetweenSteps, a static shared by the whole test "
+                + $"process, so it must carry [Collection(\"{CollectionName}\")]; found "
+                + $"{(collection is null ? "no [Collection] attribute" : $"[Collection(\"{collection.Name}\")]")}.");
+        }
+
+        var definition = typeof(ManagedConfCrashHookCollection).GetCustomAttribute<CollectionDefinitionAttribute>();
+        Assert.NotNull(definition);
+        Assert.Equal(CollectionName, definition.Name);
+        Assert.True(
+            definition.DisableParallelization,
+            $"The \"{CollectionName}\" collection must disable parallelization: without it the two classes are "
+            + "serialized against each other only, and any other class that reaches WriteTwoSteps can still meet "
+            + "a crash that was armed for someone else.");
+    }
+
+    [Fact]
+    public void EveryTestFileThatArmsTheHook_CarriesTheCollection()
+    {
+        var thisFile = ThisFile();
+        var offenders = new List<string>();
+        var filesThatArmTheHook = 0;
+
+        foreach (var path in Directory.EnumerateFiles(Path.GetDirectoryName(thisFile)!, "*.cs", SearchOption.TopDirectoryOnly))
+        {
+            if (string.Equals(Path.GetFileName(path), Path.GetFileName(thisFile), StringComparison.OrdinalIgnoreCase))
+            {
+                continue;
+            }
+
+            var text = File.ReadAllText(path);
+            if (!ArmsTheHook.IsMatch(text))
+            {
+                continue;
+            }
+
+            filesThatArmTheHook++;
+            if (!text.Contains($"[Collection(\"{CollectionName}\")]", StringComparison.Ordinal))
+            {
+                offenders.Add(Path.GetFileName(path));
+            }
+        }
+
+        Assert.True(
+            filesThatArmTheHook >= 2,
+            $"Expected the scan to find at least the two known classes that arm the hook, found {filesThatArmTheHook}; "
+            + "the scan has stopped seeing the test sources.");
+        Assert.True(
+            offenders.Count == 0,
+            "These test files arm ManagedConfMigrationSteps.FailBetweenSteps without "
+            + $"[Collection(\"{CollectionName}\")], so they can crash another class's WriteTwoSteps call: "
+            + string.Join(", ", offenders));
+    }
+
+    /// <summary>
+    /// The scan's pattern, run on strings. A test class that only clears the hook (<c>= null</c>, however it is
+    /// spaced) or compares it (<c>== null</c>) cannot crash another class, so it must not count as arming the
+    /// hook, or the scan would tell that class to join the collection. An assignment of a lambda arms it,
+    /// whatever the lambda starts with.
+    /// </summary>
+    [Theory]
+    [InlineData(" = null;", false)]
+    [InlineData("=null;", false)]
+    [InlineData(" =  null;", false)]
+    [InlineData(" =\r\n            null;", false)]
+    [InlineData(" == null", false)]
+    [InlineData(" = () => throw new IOException()", true)]
+    [InlineData(" = _ => throw new IOException()", true)]
+    [InlineData(" =\r\n            () => throw new IOException()", true)]
+    public void ArmsTheHook_MatchesAnAssignmentThatArms_NotOneThatClearsOrComparesTheHook(string afterTheName, bool expected)
+    {
+        Assert.Equal(expected, ArmsTheHook.IsMatch("ManagedConfMigrationSteps.FailBetweenSteps" + afterTheName));
+    }
+
+    private static string ThisFile([CallerFilePath] string path = "") => path;
+}

Darling/Darling.Tests/ManagedConfMigrationRunnerTests.cs

@@ -19,7 +19,11 @@ namespace Darling.Tests;
 /// <summary>
 /// <see cref="ManagedConfMigrationRunner"/> (#4336): <c>RunStepA</c> and <c>ResumePending</c> over
 /// fake snapshot delegates, no database.
+///
+/// <para>One test here sets <c>ManagedConfMigrationSteps.FailBetweenSteps</c>, a static shared by the whole test
+/// process, so this class shares a non-parallel collection with the other class that sets it (#4773).</para>
 /// </summary>
+[Collection("managed-conf-crash-hook")]
 public sealed class ManagedConfMigrationRunnerTests : IDisposable
 {
     private readonly string _dataDir;

Darling/Darling.Tests/ManagedConfMigrationStepsTests.cs

@@ -16,7 +16,11 @@ namespace Darling.Tests;
 /// <summary>
 /// <see cref="ManagedConfMigrationSteps"/> (#4336): the backup, the two-step atomic write, and the
 /// verified stamp. Pure file I/O in throwaway temp directories, no database.
+///
+/// <para>One test here sets <c>ManagedConfMigrationSteps.FailBetweenSteps</c>, a static shared by the whole test
+/// process, so this class shares a non-parallel collection with the other class that sets it (#4773).</para>
 /// </summary>
+[Collection("managed-conf-crash-hook")]
 public sealed class ManagedConfMigrationStepsTests : IDisposable
 {
     private readonly string _dataDir;

Lite.Tests/QueryWindowTruncationTests.cs

@@ -33,7 +33,14 @@ namespace PerformanceMonitorLite.Tests;
 /// Own <see cref="DuckDbInitializer"/> per test (not <c>SharedDuckDbFixture</c>) because the archive
 /// tests need control of the database's archive directory, to COPY hot rows out to parquet exactly like
 /// <c>ArchiveViewDedupTests</c> does.
+///
+/// <para>One test formats the same time twice with <c>ServerTimeHelper.FormatServerTime</c> (once inside
+/// <c>ServerTab.SetWindowTruncatedBanner</c>, once in its assertion), and that reads two settings shared by the
+/// whole test process, <c>UtcOffsetMinutes</c> and <c>CurrentDisplayMode</c>. This class joins the
+/// <c>server-time-helper</c> collection so a class that changes either setting cannot run between the two
+/// calls (#4776).</para>
 /// </summary>
+[Collection("server-time-helper")]
 public sealed class QueryWindowTruncationTests : IDisposable
 {
     private readonly int ServerId;

Lite.Tests/ServerTimeHelperCollectionTests.cs

@@ -0,0 +1,112 @@
+/*
+ * Copyright (c) 2026 Erik Darling, Darling Data LLC
+ *
+ * This file is part of the SQL Server Performance Monitor.
+ *
+ * Licensed under the MIT License. See LICENSE file in the project root for full license information.
+ */
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Text.RegularExpressions;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+/// <summary>
+/// #4776: <c>ServerTimeHelper.UtcOffsetMinutes</c> and <c>ServerTimeHelper.CurrentDisplayMode</c> are settings shared
+/// by the whole test process. Classes that write them carry <c>[Collection("server-time-helper")]</c> so xUnit runs
+/// them one at a time. A class that only READS them (through <c>FormatServerTime</c>, <c>ToServerTime</c> and the
+/// like) needs the same collection: if it formats one time twice, a writer running between the two calls makes the
+/// two texts differ. <c>QueryWindowTruncationTests</c> did exactly that and failed by a 4 hour difference.
+/// </summary>
+public sealed class ServerTimeHelperCollectionTests
+{
+    private const string CollectionName = "server-time-helper";
+
+    /// <summary>
+    /// The members of <c>ServerTimeHelper</c> that read or write one of the two shared settings. The conversions
+    /// (<c>ConvertForDisplay</c>, <c>DisplayTimeToServerTime</c>) and <c>GetTimezoneLabel</c> read the offset, in
+    /// the modes that use it. The pattern cannot tell the two <c>DisplayTimeToServerTime</c> overloads apart: the
+    /// one that takes the offset as an argument reads no setting, so a file that calls only that one goes in
+    /// <c>NamesWithoutReading</c>.
+    /// </summary>
+    private static readonly Regex ReadsTheSettings = new(
+        @"ServerTimeHelper\s*\.\s*(UtcOffsetMinutes|CurrentDisplayMode|ToServerTime|ConvertForDisplay|DisplayTimeToServerTime|GetTimezoneLabel|FormatServerTime|FormatServerClock)\b",
+        RegexOptions.CultureInvariant);
+
+    private static readonly Regex Comments = new(
+        @"//[^\r\n]*|/\*.*?\*/",
+        RegexOptions.CultureInvariant | RegexOptions.Singleline);
+
+    /// <summary>
+    /// Test files that name a setting in code text without reading it. Each entry says why; add one only with a
+    /// reason, because an entry switches the scan off for that file.
+    /// </summary>
+    private static readonly Dictionary<string, string> NamesWithoutReading = new(StringComparer.OrdinalIgnoreCase)
+    {
+        ["ComparisonWindowUtcTests.cs"] =
+            "names ServerTimeHelper.UtcOffsetMinutes inside a string it searches the product source for",
+    };
+
+    [Fact]
+    public void QueryWindowTruncationTests_IsInTheServerTimeHelperCollection()
+    {
+        var collection = typeof(QueryWindowTruncationTests).GetCustomAttribute<CollectionAttribute>();
+
+        Assert.True(
+            collection is not null && collection.Name == CollectionName,
+            "QueryWindowTruncationTests formats one time twice with ServerTimeHelper.FormatServerTime, which reads "
+            + "settings shared by the whole test process, so it must carry "
+            + $"[Collection(\"{CollectionName}\")]; found "
+            + $"{(collection is null ? "no [Collection] attribute" : $"[Collection(\"{collection.Name}\")]")}.");
+    }
+
+    [Fact]
+    public void EveryTestFileThatReadsTheSettings_CarriesTheCollection()
+    {
+        var thisFile = ThisFile();
+        var testsDirectory = Path.GetDirectoryName(thisFile)!;
+        var offenders = new List<string>();
+        var filesThatRead = 0;
+
+        foreach (var path in Directory.EnumerateFiles(testsDirectory, "*.cs", SearchOption.AllDirectories))
+        {
+            var relative = Path.GetRelativePath(testsDirectory, path);
+            var segments = relative.Split(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+            if (Array.Exists(segments, s => s is "bin" or "obj")
+                || string.Equals(Path.GetFullPath(path), Path.GetFullPath(thisFile), StringComparison.OrdinalIgnoreCase)
+                || NamesWithoutReading.ContainsKey(Path.GetFileName(path)))
+            {
+                continue;
+            }
+
+            var code = Comments.Replace(File.ReadAllText(path), string.Empty);
+            if (!ReadsTheSettings.IsMatch(code))
+            {
+                continue;
+            }
+
+            filesThatRead++;
+            if (!code.Contains($"[Collection(\"{CollectionName}\")]", StringComparison.Ordinal))
+            {
+                offenders.Add(relative);
+            }
+        }
+
+        Assert.True(
+            filesThatRead >= 10,
+            $"Expected the scan to find the many classes that use ServerTimeHelper's shared settings, found {filesThatRead}; "
+            + "the scan has stopped seeing the test sources.");
+        Assert.True(
+            offenders.Count == 0,
+            "These test files use ServerTimeHelper's shared settings without "
+            + $"[Collection(\"{CollectionName}\")], so a class that changes them can run between two of their reads: "
+            + string.Join(", ", offenders));
+    }
+
+    private static string ThisFile([CallerFilePath] string path = "") => path;
+}