Skip to content

Commit 972016d

Browse files
authored
Add DiffRunner.TrayDisabled (#854)
Pending moves and deletes go to a running tray from inside the test process, and nothing turned that off on its own. Disabling diff is not the same switch: it does not stop the tracking either, since every exit of InnerLaunch adds the move, Disabled included; and in Verify it turns off the inline staging that a suite testing that staging exists to produce. So a suite that has to leave diff on collected a pending move per staged snapshot per run on any developer box with a tray, each pointing at a throwaway directory and each offering an accept that would write to it. DiffEngine.Tests already cut both routes for itself, but half of that - DiffEngineTray.IsRunning - is internal, so no consumer could. TrayDisabled is the public half, read from DiffEngine_TrayDisabled until set and pinned after, as Disabled is. PendingFiles now gates all six sends through one TrayAvailable, so opting out cannot cover only some of them.
1 parent 816156d commit 972016d

10 files changed

Lines changed: 311 additions & 11 deletions

File tree

claude.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,4 +306,5 @@ apart.
306306
- Tool discovery uses wildcard path matching (`WildcardFileFinder`) to find executables in common install locations
307307
- Tool order can be customized via `DiffEngine_ToolOrder` environment variable
308308
- `DisabledChecker` respects `DiffEngine_Disabled` env var
309+
- `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
309310
- Tests use TUnit and Verify for snapshot testing

docs/mdsource/tray.source.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,19 @@ To limit impact on system resources, the [default max concurrent open tool insta
149149
Accept all open HotKey allows the current batch of open diffs to be accepted.
150150

151151

152+
## Opting out of tracking
153+
154+
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.
155+
156+
To opt a process out, set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code:
157+
158+
```
159+
DiffRunner.TrayDisabled = true;
160+
```
161+
162+
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.
163+
164+
152165
## Currently supported in
153166

154167
* [ApprovalTests](https://github.com/approvals/ApprovalTests.Net) v5.4.0 and above

docs/tray.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,19 @@ To limit impact on system resources, the [default max concurrent open tool insta
156156
Accept all open HotKey allows the current batch of open diffs to be accepted.
157157

158158

159+
## Opting out of tracking
160+
161+
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.
162+
163+
To opt a process out, set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code:
164+
165+
```
166+
DiffRunner.TrayDisabled = true;
167+
```
168+
169+
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.
170+
171+
159172
## Currently supported in
160173

161174
* [ApprovalTests](https://github.com/approvals/ApprovalTests.Net) v5.4.0 and above

readme.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ DiffEngine manages launching and cleanup of diff tools. It is designed to be use
4848
* [Programmatic usage](#programmatic-usage)
4949
* [Disable for a machine/process](#disable-for-a-machineprocess)
5050
* [Disable in code](#disable-in-code)
51+
* [Disable the tray](#disable-the-tray)
5152
* [Icons](#icons)<!-- endToc -->
5253
* [Tools](/docs/diff-tool.md)<!-- include: doc-index. path: /docs/mdsource/doc-index.include.md -->
5354
* [Tool Order](/docs/diff-tool.order.md)
@@ -259,6 +260,19 @@ DiffRunner.Disabled = true;
259260
```
260261

261262

263+
## Disable the tray
264+
265+
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.
266+
267+
Set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code:
268+
269+
```
270+
DiffRunner.TrayDisabled = true;
271+
```
272+
273+
[More detail](/docs/tray.md#opting-out-of-tracking).
274+
275+
262276
## Icons
263277

264278
[Game](https://thenounproject.com/term/game/2956486/) designed by [Andrejs Kirma](https://thenounproject.com/andrejs/) from [The Noun Project](https://thenounproject.com).

readme.source.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,19 @@ DiffRunner.Disabled = true;
116116
```
117117

118118

119+
## Disable the tray
120+
121+
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.
122+
123+
Set an environment variable `DiffEngine_TrayDisabled` with the value `true`, or in code:
124+
125+
```
126+
DiffRunner.TrayDisabled = true;
127+
```
128+
129+
[More detail](/docs/tray.md#opting-out-of-tracking).
130+
131+
119132
## Icons
120133

121134
[Game](https://thenounproject.com/term/game/2956486/) designed by [Andrejs Kirma](https://thenounproject.com/andrejs/) from [The Noun Project](https://thenounproject.com).
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
// DiffEngineTray is the obsolete public shim, but its IsRunning is still where the tray check
2+
// lives, and this test has to move it.
3+
#pragma warning disable CS0618
4+
5+
/// <summary>
6+
/// <see cref="DiffRunner.TrayDisabled" />: a process that wants a diff tool launched but does not
7+
/// want the tray collecting what it produces.
8+
/// <para>
9+
/// The case it exists for is a test suite driving a library that stages snapshots. Turning diff off
10+
/// is not the same switch: in Verify it also turns off the inline staging such a suite exists to
11+
/// test, and it does not stop the tracking anyway, since every exit of
12+
/// <c>DiffRunner.InnerLaunch</c> - <c>Disabled</c> included - still adds the move. So a developer
13+
/// box collected a pending move per snapshot per run, each pointing at a throwaway directory, and
14+
/// each offering an accept that would write to it.
15+
/// </para>
16+
/// </summary>
17+
[NotInParallel]
18+
public class TrayDisabledTests
19+
{
20+
const string Variable = "DiffEngine_TrayDisabled";
21+
22+
[Test]
23+
public async Task Read_from_the_environment_until_set()
24+
{
25+
DiffRunner.ResetTrayDisabled();
26+
27+
Environment.SetEnvironmentVariable(Variable, "true");
28+
await Assert.That(DiffRunner.TrayDisabled).IsTrue();
29+
30+
// Setting pins it, exactly as Disabled does, so a consumer that opts back in is not
31+
// overruled by the machine it runs on.
32+
DiffRunner.TrayDisabled = false;
33+
await Assert.That(DiffRunner.TrayDisabled).IsFalse();
34+
}
35+
36+
[Test]
37+
public async Task A_disabled_tray_leaves_the_move_to_the_queue_owner()
38+
{
39+
await Assert.That(ViewerServer.TryBind(0, out var bound)).IsTrue();
40+
using var server = bound!;
41+
using var cancel = new CancelSource();
42+
43+
var heardByOwner = new ConcurrentBag<string>();
44+
var listening = server.Listen(
45+
_ =>
46+
{
47+
heardByOwner.Add($"{_.Verb}:{_.Key}");
48+
return ViewerResponse.Success();
49+
},
50+
cancel.Token);
51+
52+
using var tray = new PiperListener();
53+
54+
var previousPort = PiperClient.Port;
55+
var previousViewerPort = Environment.GetEnvironmentVariable(ViewerClient.PortVariable);
56+
var previousRunning = DiffEngineTray.IsRunning;
57+
try
58+
{
59+
// A tray that is running and really would take it, so what follows is the switch
60+
// rather than an absent tray.
61+
PiperClient.Port = tray.Port;
62+
DiffEngineTray.IsRunning = true;
63+
Environment.SetEnvironmentVariable(ViewerClient.PortVariable, server.Port.ToString());
64+
65+
DiffRunner.TrayDisabled = false;
66+
await PendingFiles.AddMoveAsync("taken.txt", "target.txt", null, null, false, null, cancel.Token);
67+
68+
await tray.WaitFor(1);
69+
await Assert.That(heardByOwner.Count).IsEqualTo(0);
70+
71+
DiffRunner.TrayDisabled = true;
72+
await PendingFiles.AddMoveAsync("skipped.txt", "target.txt", null, null, false, null, cancel.Token);
73+
74+
// The owner took the second one, which is the fallback branch for no tray at all.
75+
await Assert.That(heardByOwner.Count).IsEqualTo(1);
76+
await Assert.That(heardByOwner).Contains(_ => _.StartsWith("Move:", StringComparison.Ordinal));
77+
78+
// And the tray still holds only the first. Asserted after the owner heard the second,
79+
// because the piper decision is made before that send, so by here it has happened.
80+
await Assert.That(tray.Payloads.Count).IsEqualTo(1);
81+
await Assert.That(tray.Payloads).Contains(_ => _.Contains("taken.txt", StringComparison.Ordinal));
82+
}
83+
finally
84+
{
85+
PiperClient.Port = previousPort;
86+
DiffEngineTray.IsRunning = previousRunning;
87+
Environment.SetEnvironmentVariable(ViewerClient.PortVariable, previousViewerPort);
88+
await cancel.CancelAsync();
89+
try
90+
{
91+
// No token: the line above already cancelled it, so passing it here would return
92+
// before the listener had unwound rather than waiting for it to. The timeout is
93+
// what bounds the drain
94+
// ReSharper disable once MethodSupportsCancellation
95+
await listening.WaitAsync(TimeSpan.FromSeconds(5));
96+
}
97+
catch (Exception exception)
98+
when (exception is OperationCanceledException or TimeoutException)
99+
{
100+
}
101+
}
102+
}
103+
104+
/// <summary>
105+
/// Every other test in this assembly runs with the ambient value, which the module initializer
106+
/// leaves alone.
107+
/// </summary>
108+
[After(Test)]
109+
public void Restore()
110+
{
111+
Environment.SetEnvironmentVariable(Variable, null);
112+
DiffRunner.ResetTrayDisabled();
113+
}
114+
115+
/// <summary>
116+
/// Stands in for the tray's PiperServer: the payload is written one way and never answered, so
117+
/// accepting the connection and reading it to the end is the whole protocol from this side.
118+
/// </summary>
119+
sealed class PiperListener : IDisposable
120+
{
121+
readonly TcpListener listener;
122+
readonly CancelSource cancellation = new();
123+
readonly Task loop;
124+
125+
public PiperListener()
126+
{
127+
listener = new(IPAddress.Loopback, 0);
128+
listener.Start();
129+
Port = ((IPEndPoint) listener.LocalEndpoint).Port;
130+
loop = Task.Run(Accept);
131+
}
132+
133+
public int Port { get; }
134+
135+
public ConcurrentBag<string> Payloads { get; } = [];
136+
137+
public async Task WaitFor(int count)
138+
{
139+
for (var attempt = 0; attempt < 250; attempt++)
140+
{
141+
if (Payloads.Count >= count)
142+
{
143+
return;
144+
}
145+
146+
await Task.Delay(20);
147+
}
148+
149+
throw new($"Only {Payloads.Count} payloads reached the tray, expected {count}.");
150+
}
151+
152+
async Task Accept()
153+
{
154+
while (!cancellation.IsCancellationRequested)
155+
{
156+
try
157+
{
158+
// No token: the cancellable overload is net6 and up, and this compiles for
159+
// net48 too. Stop in Dispose is what breaks the accept, which lands in the
160+
// catch below.
161+
using var client = await listener.AcceptTcpClientAsync();
162+
using var stream = client.GetStream();
163+
using var reader = new StreamReader(stream);
164+
Payloads.Add(await reader.ReadToEndAsync());
165+
}
166+
catch (Exception exception)
167+
when (exception is OperationCanceledException or ObjectDisposedException or SocketException)
168+
{
169+
return;
170+
}
171+
}
172+
}
173+
174+
public void Dispose()
175+
{
176+
cancellation.Cancel();
177+
listener.Stop();
178+
try
179+
{
180+
loop.Wait(TimeSpan.FromSeconds(2));
181+
}
182+
catch (AggregateException)
183+
{
184+
}
185+
186+
cancellation.Dispose();
187+
}
188+
}
189+
}

src/DiffEngine/DiffRunner.cs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,41 @@ public static bool Disabled
3434
internal static void ResetDisabled() =>
3535
disabled = null;
3636

37+
/// <summary>
38+
/// Whether pending moves and deletes are sent to DiffEngineTray.
39+
/// <para>
40+
/// Independent of <see cref="Disabled" />, because the two answer different questions and a
41+
/// test suite driving a library that stages snapshots needs them apart. Disabling diff turns
42+
/// off the launch, and in Verify it also turns off the inline staging that a suite testing
43+
/// that staging exists to produce. This turns off only the tracking, so a machine with a tray
44+
/// running does not collect a pending move per snapshot a test run happened to produce -
45+
/// pointing at a throwaway directory, and offering an accept that would write to it.
46+
/// </para>
47+
/// <para>
48+
/// A move with no tray falls through to the inline queue owner, and goes nowhere when nothing
49+
/// owns it. Pair this with a <c>DiffEngine_ViewerPort</c> nothing is listening on to detach
50+
/// from both, which is what <c>DiffEngine.Tests</c> does.
51+
/// </para>
52+
/// <para>
53+
/// Read from <c>DiffEngine_TrayDisabled</c> until set, then pinned, exactly as
54+
/// <see cref="Disabled" /> is.
55+
/// </para>
56+
/// </summary>
57+
public static bool TrayDisabled
58+
{
59+
get => trayDisabled ?? TrayDisabledChecker.IsDisabled();
60+
set => trayDisabled = value;
61+
}
62+
63+
static bool? trayDisabled;
64+
65+
/// <summary>
66+
/// Forgets an explicit <see cref="TrayDisabled" />, so it is read from the environment again.
67+
/// For tests, which is where anything sets it and then wants the ambient value back.
68+
/// </summary>
69+
internal static void ResetTrayDisabled() =>
70+
trayDisabled = null;
71+
3772
public static void MaxInstancesToLaunch(int value) =>
3873
MaxInstance.SetForAppDomain(value);
3974

0 commit comments

Comments
 (0)