From f3c116ce9a43502dccd07cac53c36a4e397b66a6 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 23:03:57 +1000 Subject: [PATCH] Read Disabled when it is asked for rather than at type initialisation DiffRunner.Disabled captured DisabledChecker.IsDisable() in its initialiser, so whichever of the detectors reported first won for the life of the process. Setting BuildServerDetector.Detected or AiCliDetector.Detected after anything had touched DiffRunner did nothing at all - and a test host sets them after it has loaded, which is the only moment it can. BuildServerDetector.Detected is an AsyncLocal override on top of that, and a per-context value could never have reached a static captured once anyway. It is computed on read now, and an explicit set still pins it: consumers that assign Disabled keep the value they assigned, and nothing is read from the environment after that. --- src/DiffEngine.Tests/DisabledTests.cs | 38 +++++++++++++++++++++++++++ src/DiffEngine/DiffRunner.cs | 28 +++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 src/DiffEngine.Tests/DisabledTests.cs diff --git a/src/DiffEngine.Tests/DisabledTests.cs b/src/DiffEngine.Tests/DisabledTests.cs new file mode 100644 index 00000000..ed2f3382 --- /dev/null +++ b/src/DiffEngine.Tests/DisabledTests.cs @@ -0,0 +1,38 @@ +/// +/// was captured at type initialisation, so a build server or AI +/// CLI reported after that - which is when a test host reports one, having only just loaded - left +/// diff tools launching anyway. +/// +[NotInParallel] +public class DisabledTests +{ + [Test] + public async Task A_build_server_detected_after_first_use_still_disables() + { + // As the module initializer left it, and as any consumer that ever set it leaves it + DiffRunner.Disabled = false; + + DiffRunner.ResetDisabled(); + BuildServerDetector.Detected = true; + + await Assert.That(DiffRunner.Disabled).IsTrue(); + } + + [Test] + public async Task Setting_it_pins_it() + { + DiffRunner.ResetDisabled(); + BuildServerDetector.Detected = true; + + DiffRunner.Disabled = false; + + await Assert.That(DiffRunner.Disabled).IsFalse(); + } + + /// + /// Every other test in this assembly runs with it off, which the module initializer does once. + /// + [After(Test)] + public void Restore() => + DiffRunner.Disabled = false; +} diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs index eb492457..cef9327e 100644 --- a/src/DiffEngine/DiffRunner.cs +++ b/src/DiffEngine/DiffRunner.cs @@ -6,7 +6,33 @@ namespace DiffEngine; /// public static partial class DiffRunner { - public static bool Disabled { get; set; } = DisabledChecker.IsDisable(); + /// + /// Whether launching a diff tool is turned off, for this process or for this async context. + /// + /// Read rather than captured, so that the overrides feeding it - + /// and , both of which a test host sets after it has + /// loaded - are honoured whenever they are set. Captured once at type initialisation, they + /// were inert the moment anything had touched this class, and an AsyncLocal override could + /// never have reached a value read into a static anyway. + /// + /// + /// Setting it pins it, and nothing is read from the environment after that. + /// + /// + public static bool Disabled + { + get => disabled ?? DisabledChecker.IsDisable(); + set => disabled = value; + } + + static bool? disabled; + + /// + /// Forgets an explicit , so it is read from the environment again. For + /// tests, which is where anything sets it and then wants the detectors back. + /// + internal static void ResetDisabled() => + disabled = null; public static void MaxInstancesToLaunch(int value) => MaxInstance.SetForAppDomain(value);