diff --git a/claude.md b/claude.md index d240860a..8c5d1bdf 100644 --- a/claude.md +++ b/claude.md @@ -306,4 +306,5 @@ apart. - Tool discovery uses wildcard path matching (`WildcardFileFinder`) to find executables in common install locations - Tool order can be customized via `DiffEngine_ToolOrder` environment variable - `DisabledChecker` respects `DiffEngine_Disabled` env var +- `TrayDisabledChecker` respects `DiffEngine_TrayDisabled` env var, behind `DiffRunner.TrayDisabled`. Separate from `Disabled` because tracking a pending move is separate from launching a tool: every exit of `InnerLaunch`, `Disabled` included, still calls `AddMove`. `PendingFiles.TrayAvailable` is the single gate - Tests use TUnit and Verify for snapshot testing diff --git a/docs/mdsource/tray.source.md b/docs/mdsource/tray.source.md index 9ac0b77d..92383d16 100644 --- a/docs/mdsource/tray.source.md +++ b/docs/mdsource/tray.source.md @@ -149,6 +149,19 @@ To limit impact on system resources, the [default max concurrent open tool insta Accept all open HotKey allows the current batch of open diffs to be accepted. +## Opting out of tracking + +Pending moves and deletes are sent to a running tray by the DiffEngine library inside the test process. That tracking is separate from launching a diff tool: every exit from `DiffRunner.Launch` adds the move, `DiffRunner.Disabled` included, so turning diff off does not turn it off. + +To opt a process out, set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code: + +``` +DiffRunner.TrayDisabled = true; +``` + +The case it exists for is a test suite that needs the launch to happen but does not want what it produces collected: for example a suite asserting on the files a snapshot library stages, where each run would otherwise leave the tray a pending entry pointing at a throwaway directory. A move with no tray falls through to whatever owns the inline queue, and goes nowhere when nothing does. + + ## Currently supported in * [ApprovalTests](https://github.com/approvals/ApprovalTests.Net) v5.4.0 and above diff --git a/docs/tray.md b/docs/tray.md index d34938d7..ea306850 100644 --- a/docs/tray.md +++ b/docs/tray.md @@ -156,6 +156,19 @@ To limit impact on system resources, the [default max concurrent open tool insta Accept all open HotKey allows the current batch of open diffs to be accepted. +## Opting out of tracking + +Pending moves and deletes are sent to a running tray by the DiffEngine library inside the test process. That tracking is separate from launching a diff tool: every exit from `DiffRunner.Launch` adds the move, `DiffRunner.Disabled` included, so turning diff off does not turn it off. + +To opt a process out, set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code: + +``` +DiffRunner.TrayDisabled = true; +``` + +The case it exists for is a test suite that needs the launch to happen but does not want what it produces collected: for example a suite asserting on the files a snapshot library stages, where each run would otherwise leave the tray a pending entry pointing at a throwaway directory. A move with no tray falls through to whatever owns the inline queue, and goes nowhere when nothing does. + + ## Currently supported in * [ApprovalTests](https://github.com/approvals/ApprovalTests.Net) v5.4.0 and above diff --git a/readme.md b/readme.md index 09bdd229..3f475c42 100644 --- a/readme.md +++ b/readme.md @@ -48,6 +48,7 @@ DiffEngine manages launching and cleanup of diff tools. It is designed to be use * [Programmatic usage](#programmatic-usage) * [Disable for a machine/process](#disable-for-a-machineprocess) * [Disable in code](#disable-in-code) + * [Disable the tray](#disable-the-tray) * [Icons](#icons) * [Tools](/docs/diff-tool.md) * [Tool Order](/docs/diff-tool.order.md) @@ -259,6 +260,19 @@ DiffRunner.Disabled = true; ``` +## Disable the tray + +Pending moves and deletes are sent to [DiffEngineTray](/docs/tray.md) when one is running. That tracking is separate from launching a diff tool, so disabling diff does not stop it. + +Set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code: + +``` +DiffRunner.TrayDisabled = true; +``` + +[More detail](/docs/tray.md#opting-out-of-tracking). + + ## Icons [Game](https://thenounproject.com/term/game/2956486/) designed by [Andrejs Kirma](https://thenounproject.com/andrejs/) from [The Noun Project](https://thenounproject.com). diff --git a/readme.source.md b/readme.source.md index 52413062..4fbb8b12 100644 --- a/readme.source.md +++ b/readme.source.md @@ -116,6 +116,19 @@ DiffRunner.Disabled = true; ``` +## Disable the tray + +Pending moves and deletes are sent to [DiffEngineTray](/docs/tray.md) when one is running. That tracking is separate from launching a diff tool, so disabling diff does not stop it. + +Set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code: + +``` +DiffRunner.TrayDisabled = true; +``` + +[More detail](/docs/tray.md#opting-out-of-tracking). + + ## Icons [Game](https://thenounproject.com/term/game/2956486/) designed by [Andrejs Kirma](https://thenounproject.com/andrejs/) from [The Noun Project](https://thenounproject.com). diff --git a/src/DiffEngine.Tests/TrayDisabledTests.cs b/src/DiffEngine.Tests/TrayDisabledTests.cs new file mode 100644 index 00000000..7eed8521 --- /dev/null +++ b/src/DiffEngine.Tests/TrayDisabledTests.cs @@ -0,0 +1,189 @@ +// DiffEngineTray is the obsolete public shim, but its IsRunning is still where the tray check +// lives, and this test has to move it. +#pragma warning disable CS0618 + +/// +/// : a process that wants a diff tool launched but does not +/// want the tray collecting what it produces. +/// +/// The case it exists for is a test suite driving a library that stages snapshots. Turning diff off +/// is not the same switch: in Verify it also turns off the inline staging such a suite exists to +/// test, and it does not stop the tracking anyway, since every exit of +/// DiffRunner.InnerLaunch - Disabled included - still adds the move. So a developer +/// box collected a pending move per snapshot per run, each pointing at a throwaway directory, and +/// each offering an accept that would write to it. +/// +/// +[NotInParallel] +public class TrayDisabledTests +{ + const string Variable = "DiffEngine_TrayDisabled"; + + [Test] + public async Task Read_from_the_environment_until_set() + { + DiffRunner.ResetTrayDisabled(); + + Environment.SetEnvironmentVariable(Variable, "true"); + await Assert.That(DiffRunner.TrayDisabled).IsTrue(); + + // Setting pins it, exactly as Disabled does, so a consumer that opts back in is not + // overruled by the machine it runs on. + DiffRunner.TrayDisabled = false; + await Assert.That(DiffRunner.TrayDisabled).IsFalse(); + } + + [Test] + public async Task A_disabled_tray_leaves_the_move_to_the_queue_owner() + { + await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue(); + using var server = bound!; + using var cancel = new CancelSource(); + + var heardByOwner = new ConcurrentBag(); + var listening = server.Listen( + _ => + { + heardByOwner.Add($"{_.Verb}:{_.Key}"); + return ViewerResponse.Success(); + }, + cancel.Token); + + using var tray = new PiperListener(); + + var previousPort = PiperClient.Port; + var previousViewerPort = Environment.GetEnvironmentVariable(ViewerClient.PortVariable); + var previousRunning = DiffEngineTray.IsRunning; + try + { + // A tray that is running and really would take it, so what follows is the switch + // rather than an absent tray. + PiperClient.Port = tray.Port; + DiffEngineTray.IsRunning = true; + Environment.SetEnvironmentVariable(ViewerClient.PortVariable, server.Port.ToString()); + + DiffRunner.TrayDisabled = false; + await PendingFiles.AddMoveAsync("taken.txt", "target.txt", null, null, false, null, cancel.Token); + + await tray.WaitFor(1); + await Assert.That(heardByOwner.Count).IsEqualTo(0); + + DiffRunner.TrayDisabled = true; + await PendingFiles.AddMoveAsync("skipped.txt", "target.txt", null, null, false, null, cancel.Token); + + // The owner took the second one, which is the fallback branch for no tray at all. + await Assert.That(heardByOwner.Count).IsEqualTo(1); + await Assert.That(heardByOwner).Contains(_ => _.StartsWith("Move:", StringComparison.Ordinal)); + + // And the tray still holds only the first. Asserted after the owner heard the second, + // because the piper decision is made before that send, so by here it has happened. + await Assert.That(tray.Payloads.Count).IsEqualTo(1); + await Assert.That(tray.Payloads).Contains(_ => _.Contains("taken.txt", StringComparison.Ordinal)); + } + finally + { + PiperClient.Port = previousPort; + DiffEngineTray.IsRunning = previousRunning; + Environment.SetEnvironmentVariable(ViewerClient.PortVariable, previousViewerPort); + await cancel.CancelAsync(); + try + { + // No token: the line above already cancelled it, so passing it here would return + // before the listener had unwound rather than waiting for it to. The timeout is + // what bounds the drain + // ReSharper disable once MethodSupportsCancellation + await listening.WaitAsync(TimeSpan.FromSeconds(5)); + } + catch (Exception exception) + when (exception is OperationCanceledException or TimeoutException) + { + } + } + } + + /// + /// Every other test in this assembly runs with the ambient value, which the module initializer + /// leaves alone. + /// + [After(Test)] + public void Restore() + { + Environment.SetEnvironmentVariable(Variable, null); + DiffRunner.ResetTrayDisabled(); + } + + /// + /// Stands in for the tray's PiperServer: the payload is written one way and never answered, so + /// accepting the connection and reading it to the end is the whole protocol from this side. + /// + sealed class PiperListener : IDisposable + { + readonly TcpListener listener; + readonly CancelSource cancellation = new(); + readonly Task loop; + + public PiperListener() + { + listener = new(IPAddress.Loopback, 0); + listener.Start(); + Port = ((IPEndPoint) listener.LocalEndpoint).Port; + loop = Task.Run(Accept); + } + + public int Port { get; } + + public ConcurrentBag Payloads { get; } = []; + + public async Task WaitFor(int count) + { + for (var attempt = 0; attempt < 250; attempt++) + { + if (Payloads.Count >= count) + { + return; + } + + await Task.Delay(20); + } + + throw new($"Only {Payloads.Count} payloads reached the tray, expected {count}."); + } + + async Task Accept() + { + while (!cancellation.IsCancellationRequested) + { + try + { + // No token: the cancellable overload is net6 and up, and this compiles for + // net48 too. Stop in Dispose is what breaks the accept, which lands in the + // catch below. + using var client = await listener.AcceptTcpClientAsync(); + using var stream = client.GetStream(); + using var reader = new StreamReader(stream); + Payloads.Add(await reader.ReadToEndAsync()); + } + catch (Exception exception) + when (exception is OperationCanceledException or ObjectDisposedException or SocketException) + { + return; + } + } + } + + public void Dispose() + { + cancellation.Cancel(); + listener.Stop(); + try + { + loop.Wait(TimeSpan.FromSeconds(2)); + } + catch (AggregateException) + { + } + + cancellation.Dispose(); + } + } +} diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs index de3fe608..d2eca433 100644 --- a/src/DiffEngine/DiffRunner.cs +++ b/src/DiffEngine/DiffRunner.cs @@ -34,6 +34,41 @@ public static bool Disabled internal static void ResetDisabled() => disabled = null; + /// + /// Whether pending moves and deletes are sent to DiffEngineTray. + /// + /// Independent of , because the two answer different questions and a + /// test suite driving a library that stages snapshots needs them apart. Disabling diff turns + /// off the launch, and in Verify it also turns off the inline staging that a suite testing + /// that staging exists to produce. This turns off only the tracking, so a machine with a tray + /// running does not collect a pending move per snapshot a test run happened to produce - + /// pointing at a throwaway directory, and offering an accept that would write to it. + /// + /// + /// A move with no tray falls through to the inline queue owner, and goes nowhere when nothing + /// owns it. Pair this with a DiffEngine_ViewerPort nothing is listening on to detach + /// from both, which is what DiffEngine.Tests does. + /// + /// + /// Read from DiffEngine_TrayDisabled until set, then pinned, exactly as + /// is. + /// + /// + public static bool TrayDisabled + { + get => trayDisabled ?? TrayDisabledChecker.IsDisabled(); + set => trayDisabled = value; + } + + static bool? trayDisabled; + + /// + /// Forgets an explicit , so it is read from the environment again. + /// For tests, which is where anything sets it and then wants the ambient value back. + /// + internal static void ResetTrayDisabled() => + trayDisabled = null; + public static void MaxInstancesToLaunch(int value) => MaxInstance.SetForAppDomain(value); diff --git a/src/DiffEngine/Tray/PendingFiles.cs b/src/DiffEngine/Tray/PendingFiles.cs index bc301167..76067c92 100644 --- a/src/DiffEngine/Tray/PendingFiles.cs +++ b/src/DiffEngine/Tray/PendingFiles.cs @@ -19,10 +19,11 @@ namespace DiffEngine; /// file to compare against and so no tool to open. /// /// -/// The tray check is , read once when that type initialises. -/// A tray started after the test process therefore never sees the piper port for the rest of that -/// process's life, and its moves and deletes arrive here instead — which a tray that owns the -/// queue answers, so they end up tracked either way. +/// The tray check is , whose first half is +/// , read once when that type initialises. A tray started +/// after the test process therefore never sees the piper port for the rest of that process's life, +/// and its moves and deletes arrive here instead — which a tray that owns the queue answers, so +/// they end up tracked either way. /// /// /// The mirror of that case is a tray that exits while a long lived host keeps running, and it is @@ -34,9 +35,22 @@ namespace DiffEngine; /// static class PendingFiles { + /// + /// Whether the tray is the surface for a pending file: one is running, and this process has + /// not opted out with . + /// + /// One property rather than the check at each of the six sends, so opting out cannot cover + /// some of them. Read per send, because the opt out is a setting a test moves and puts back + /// while is fixed for the life of the process. + /// + /// + static bool TrayAvailable => + DiffEngineTray.IsRunning && + !DiffRunner.TrayDisabled; + public static void AddDelete(string file) { - if (DiffEngineTray.IsRunning && + if (TrayAvailable && PiperClient.SendDelete(file)) { return; @@ -54,7 +68,7 @@ public static void AddDelete(string file) public static async Task AddDeleteAsync(string file, Cancel cancel) { - if (DiffEngineTray.IsRunning && + if (TrayAvailable && await PiperClient.SendDeleteAsync(file, cancel)) { return; @@ -92,7 +106,7 @@ public static LaunchResult AddDiff(ResolvedTool tool, string tempFile, string ta // No process, and the arguments and CanKill from the one place that answers that, because // the tray works out the same two values for itself when a move arrives without them. var (arguments, canKill) = RelaunchFor(tool, tempFile, targetFile); - if (DiffEngineTray.IsRunning && + if (TrayAvailable && PiperClient.SendMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null)) { ViewerClient.TrySend(new(ViewerVerb.Focus, TrackedKeys.ForMove(tempFile))); @@ -139,7 +153,7 @@ static LaunchResult Refused(string tempFile, string targetFile) => public static async Task AddDiffAsync(ResolvedTool tool, string tempFile, string targetFile, Cancel cancel) { var (arguments, canKill) = RelaunchFor(tool, tempFile, targetFile); - if (DiffEngineTray.IsRunning && + if (TrayAvailable && await PiperClient.SendMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null, cancel)) { await ViewerClient.TrySendAsync(new(ViewerVerb.Focus, TrackedKeys.ForMove(tempFile)), cancel); @@ -222,7 +236,7 @@ public static void AddMove( bool canKill, int? processId) { - if (DiffEngineTray.IsRunning && + if (TrayAvailable && PiperClient.SendMove(tempFile, targetFile, exe, arguments, canKill, processId)) { return; @@ -240,7 +254,7 @@ public static async Task AddMoveAsync( int? processId, Cancel cancel) { - if (DiffEngineTray.IsRunning && + if (TrayAvailable && await PiperClient.SendMoveAsync(tempFile, targetFile, exe, arguments, canKill, processId, cancel)) { return; diff --git a/src/DiffEngine/Tray/TrayDisabledChecker.cs b/src/DiffEngine/Tray/TrayDisabledChecker.cs new file mode 100644 index 00000000..fe150925 --- /dev/null +++ b/src/DiffEngine/Tray/TrayDisabledChecker.cs @@ -0,0 +1,8 @@ +static class TrayDisabledChecker +{ + public static bool IsDisabled() + { + var variable = Environment.GetEnvironmentVariable("DiffEngine_TrayDisabled"); + return string.Equals(variable, "true", StringComparison.OrdinalIgnoreCase); + } +} diff --git a/src/Directory.Build.props b/src/Directory.Build.props index 0b49ef19..c06bcd7b 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -1,7 +1,7 @@ - 20.0.0 + 20.1.0 1.0.0 Testing, Snapshot, Diff, Compare Launches diff tools based on file extensions. Designed to be consumed by snapshot testing libraries.