Skip to content

Commit 3a850ae

Browse files
committed
Remember a port found unowned for ten minutes
Every passing inline verification settles with the queue owner, and every verification the global switch declines retires, each a fresh connect to 127.0.0.1:3493. Nothing owning that port is the ordinary state of a machine with no tray, and on Windows a refused loopback connection is not free: the firewall's stealth mode, on by default, drops the reset a closed port would answer with, so the connect sits through the SYN retries and fails after two seconds rather than at once. A class of 188 passing inline tests took 382 seconds with nothing listening and under a second with a tray answering. ViewerClient now records, per port, when a connect found nobody, and for RecheckUnownedAfter (ten minutes) the telling sends - the bool TrySend and TrySendAsync, so settle, retire, move, delete and focus - return without connecting. The response-returning overloads take skipIfUnowned, off by default, so the tray, the viewer and InlineQueueClient, which share this file, keep connecting every time and notice an owner arriving. IsOwned always connects and corrects the memory, which is what lets the launch gate hand work to a viewer it has just started; the first inline and diff sends opt in because that gate probes for itself right after. SettleAppliedInline stays on the always-connect path: one send per human accept, and a viewer a later run started holds exactly the entry it settles. Only a failed connect marks a port unowned; an accepted connection that then times out does not. Ten minutes rather than the life of the process only for a long lived consumer that is not a test host, where a tray started later would otherwise never see its moves until a restart. A recheck buys nothing else: the sends the memory delays are ones a new owner did not need, and anything that has to reach one probes first. Same 188 tests after: 2.5 seconds with nothing listening, 0.5 with a tray. The inline docs asserted that a refused connection is immediate either way, which was the assumption at fault; corrected.
1 parent abda1dd commit 3a850ae

7 files changed

Lines changed: 403 additions & 16 deletions

File tree

‎claude.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,5 +321,6 @@ apart.
321321
- Tool discovery uses wildcard path matching (`WildcardFileFinder`) to find executables in common install locations
322322
- Tool order can be customized via `DiffEngine_ToolOrder` environment variable
323323
- `DisabledChecker` respects `DiffEngine_Disabled` env var
324+
- `ViewerClient` remembers a port found unowned for ten minutes (`RecheckUnownedAfter`), and the library's telling sends - settle, retire, move, delete, the first inline or diff send - skip the connect while that stands. A refused loopback connection costs two seconds on Windows (firewall stealth mode drops the reset), and a green run settles once per inline verification, which was six minutes for a class of 188 inline tests. Probes (`IsOwned`), the hosts and `InlineQueueClient` always connect and correct the memory; so does `SettleAppliedInline`, being one send per accept
324325
- `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
325326
- Tests use TUnit and Verify for snapshot testing

‎docs/inline.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ For the producing side — a test library with a failing inline snapshot:
7777

7878
* `DiffRunner.AddInlineAsync(patch)` queues a patch with whatever owns the port, launching the bundled viewer when nothing does. Returns `Queued`, `Disabled` (build servers, continuous testing and AI CLIs included), or `NoViewerFound` — the caller's cue to stage files and fall back to a text diff.
7979
* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry. That framework is the running process's, which makes this the test run's verb and only the test run's: a surface applying a patch of its own wants `SettleAppliedInline`, [below](#applying-a-patch-from-another-surface).
80+
* An absent owner is also remembered. A port found with nothing listening is taken as still unowned for ten minutes, and the sends that only tell the owner something — settle, retire, a move or delete to track, the first attempt to queue a patch — return without connecting while that stands. A refused loopback connection is not free on Windows: the firewall's stealth mode, on by default, drops the reset a closed port would answer with, so each refusal takes two seconds, and a green run settling once per inline verification was spending minutes on them. Anything that has to reach an owner probes for itself before launching a viewer, and that probe, like every listing, always connects and corrects the memory with what it finds.
8081
* `AddInlineAsync` stamps `patch.Framework` with the consuming project's target framework ("net9.0", "net48") unless the caller already set it, which is what lets the owner tell a re-run from another framework disagreeing. The value is the `$(TargetFramework)` the package's build targets stamp into the project's runtimeconfig, read back rather than asked of the process — in a hosted test run the entry assembly is the runner (testhost, ReSharperTestRunner), whose framework is not the project's — with the running runtime's version as the fallback for consumers without the targets. Callers may also set `patch.TestName`, which the viewer uses to group and label the queue; without it, items are labeled by call site.
8182
* Set `patch.OriginalExpression` from `CallerArgumentExpression` where the language supplies one, and `patch.OriginalValue` — the previous expected argument's value — where it does not. One of the two is what stops a patch rewriting the wrong call site when the file has moved since the run. `patch.MemberName` from `CallerMemberName` narrows it further, and is supported everywhere including F#.
8283
* Setting `DiffEngine_InlineViewer` to `false` reports `NoViewerFound` without probing, which is how a user opts into reviewing in their IDE instead of a window.
@@ -199,7 +200,7 @@ A review surface that is neither the viewer nor the tray — the ReSharper / Rid
199200

200201
`Accept` returns `Accepted` (the entry has gone), `Failed` (still pending and retryable — a locked file, or a conflicted entry that has to be resolved in the viewer), or `Unknown` (nobody owns the queue, or it holds nothing for that call site). It hands back the owner's own message, which is what tells an applied patch from a stale one that was dropped: the wire carries whether the verb was carried out rather than an apply status, so from a client those are the same observation, and only the words separate them.
201202

202-
Listing waits half a second, since an owner that cannot answer one in that time is wedged rather than slow, and a refused connection is immediate either way. Accepting waits fifteen seconds, because the owner applies through `InlineApplier` and that can sit on its cross process mutex for ten.
203+
Listing waits half a second, since an owner that cannot answer one in that time is wedged rather than slow. A refused connection is immediate on most machines, though not on Windows, where the firewall's stealth mode holds the connect for two seconds; a listing asks every time regardless, because a surface lists precisely to notice an owner arriving, and it is only the test run's fire and forget sends that remember an absent owner for ten minutes instead. Accepting waits fifteen seconds, because the owner applies through `InlineApplier` and that can sit on its cross process mutex for ten.
203204

204205

205206
## Applying a patch from another surface

‎docs/mdsource/inline.source.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ For the producing side — a test library with a failing inline snapshot:
7070

7171
* `DiffRunner.AddInlineAsync(patch)` queues a patch with whatever owns the port, launching the bundled viewer when nothing does. Returns `Queued`, `Disabled` (build servers, continuous testing and AI CLIs included), or `NoViewerFound` — the caller's cue to stage files and fall back to a text diff.
7272
* `DiffRunner.SettleInline(sourceFile, line)` drops the pending entry for a call site, for when a previously failing test passes. Unknown entries and an absent owner are no-ops, so call it freely. The settle carries the running framework, so a multi-targeted run only settles its own variant of a conflicted entry. That framework is the running process's, which makes this the test run's verb and only the test run's: a surface applying a patch of its own wants `SettleAppliedInline`, [below](#applying-a-patch-from-another-surface).
73+
* An absent owner is also remembered. A port found with nothing listening is taken as still unowned for ten minutes, and the sends that only tell the owner something — settle, retire, a move or delete to track, the first attempt to queue a patch — return without connecting while that stands. A refused loopback connection is not free on Windows: the firewall's stealth mode, on by default, drops the reset a closed port would answer with, so each refusal takes two seconds, and a green run settling once per inline verification was spending minutes on them. Anything that has to reach an owner probes for itself before launching a viewer, and that probe, like every listing, always connects and corrects the memory with what it finds.
7374
* `AddInlineAsync` stamps `patch.Framework` with the consuming project's target framework ("net9.0", "net48") unless the caller already set it, which is what lets the owner tell a re-run from another framework disagreeing. The value is the `$(TargetFramework)` the package's build targets stamp into the project's runtimeconfig, read back rather than asked of the process — in a hosted test run the entry assembly is the runner (testhost, ReSharperTestRunner), whose framework is not the project's — with the running runtime's version as the fallback for consumers without the targets. Callers may also set `patch.TestName`, which the viewer uses to group and label the queue; without it, items are labeled by call site.
7475
* Set `patch.OriginalExpression` from `CallerArgumentExpression` where the language supplies one, and `patch.OriginalValue` — the previous expected argument's value — where it does not. One of the two is what stops a patch rewriting the wrong call site when the file has moved since the run. `patch.MemberName` from `CallerMemberName` narrows it further, and is supported everywhere including F#.
7576
* Setting `DiffEngine_InlineViewer` to `false` reports `NoViewerFound` without probing, which is how a user opts into reviewing in their IDE instead of a window.
@@ -192,7 +193,7 @@ A review surface that is neither the viewer nor the tray — the ReSharper / Rid
192193

193194
`Accept` returns `Accepted` (the entry has gone), `Failed` (still pending and retryable — a locked file, or a conflicted entry that has to be resolved in the viewer), or `Unknown` (nobody owns the queue, or it holds nothing for that call site). It hands back the owner's own message, which is what tells an applied patch from a stale one that was dropped: the wire carries whether the verb was carried out rather than an apply status, so from a client those are the same observation, and only the words separate them.
194195

195-
Listing waits half a second, since an owner that cannot answer one in that time is wedged rather than slow, and a refused connection is immediate either way. Accepting waits fifteen seconds, because the owner applies through `InlineApplier` and that can sit on its cross process mutex for ten.
196+
Listing waits half a second, since an owner that cannot answer one in that time is wedged rather than slow. A refused connection is immediate on most machines, though not on Windows, where the firewall's stealth mode holds the connect for two seconds; a listing asks every time regardless, because a surface lists precisely to notice an owner arriving, and it is only the test run's fire and forget sends that remember an absent owner for ten minutes instead. Accepting waits fifteen seconds, because the owner applies through `InlineApplier` and that can sit on its cross process mutex for ten.
196197

197198

198199
## Applying a patch from another surface
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
/// <summary>
2+
/// The memory of a port nothing was listening on, which is what stops a green run paying for a
3+
/// refused connection once per inline verification.
4+
/// <para>
5+
/// A refused loopback connection is instant on most machines and two seconds on Windows with the
6+
/// firewall's stealth mode on, which is the default. Each test here meets one refusal at most, on
7+
/// an ephemeral port of its own, so it costs that once and never talks to the live viewer port.
8+
/// </para>
9+
/// </summary>
10+
[NotInParallel]
11+
public class ViewerClientUnownedTests
12+
{
13+
static readonly ViewerMessage settle = new(ViewerVerb.Settle, InlineKey.For("Tests.cs", 1));
14+
15+
// Read before any test can have changed it, so the restore below puts back the real default
16+
// rather than a copy of it kept here
17+
static readonly TimeSpan recheckUnownedAfter = ViewerClient.RecheckUnownedAfter;
18+
19+
[Before(Test)]
20+
public void Forget() =>
21+
ViewerClient.ForgetUnowned();
22+
23+
[After(Test)]
24+
public void Restore()
25+
{
26+
ViewerClient.RecheckUnownedAfter = recheckUnownedAfter;
27+
ViewerClient.ForgetUnowned();
28+
}
29+
30+
/// <summary>
31+
/// The first refusal is remembered, so the next telling send returns without connecting - and
32+
/// keeps returning that way once somebody is there, until something asks.
33+
/// </summary>
34+
[Test]
35+
public async Task ARefusalIsRemembered()
36+
{
37+
var port = FreePort();
38+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse();
39+
40+
using var owner = new Owner(port);
41+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse();
42+
await Assert.That(owner.Heard).IsEmpty();
43+
}
44+
45+
/// <summary>
46+
/// An ask always connects, and having found the owner, corrects the memory for the sends
47+
/// after it.
48+
/// </summary>
49+
[Test]
50+
public async Task AnAskCorrectsTheMemory()
51+
{
52+
var port = FreePort();
53+
ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true);
54+
using var owner = new Owner(port);
55+
56+
await Assert.That(ViewerClient.TrySend(settle, out _, port)).IsTrue();
57+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsTrue();
58+
await Assert.That(owner.Heard.Count).IsEqualTo(2);
59+
}
60+
61+
/// <summary>
62+
/// The probe is the ask that matters most. The launch gate runs it once a viewer is started,
63+
/// and the sends queued behind the gate have to reach the viewer it found.
64+
/// </summary>
65+
[Test]
66+
public async Task TheProbeCorrectsTheMemory()
67+
{
68+
var port = FreePort();
69+
ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true);
70+
using var owner = new Owner(port);
71+
72+
await Assert.That(ViewerClient.IsOwned(port)).IsTrue();
73+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsTrue();
74+
await Assert.That(owner.Heard.Count).IsEqualTo(1);
75+
}
76+
77+
/// <summary>
78+
/// A probe finding nobody is remembered too. The gate probes once per launch, and a viewer
79+
/// that never binds would otherwise leave every send in the run paying for the refusal.
80+
/// </summary>
81+
[Test]
82+
public async Task AProbeFindingNobodyIsRemembered()
83+
{
84+
var port = FreePort();
85+
await Assert.That(ViewerClient.IsOwned(port)).IsFalse();
86+
87+
using var owner = new Owner(port);
88+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse();
89+
await Assert.That(owner.Heard).IsEmpty();
90+
}
91+
92+
/// <summary>
93+
/// A tray started mid run is found once the memory has expired, without anything asking.
94+
/// </summary>
95+
[Test]
96+
public async Task TheMemoryExpires()
97+
{
98+
ViewerClient.RecheckUnownedAfter = TimeSpan.Zero;
99+
var port = FreePort();
100+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse();
101+
102+
using var owner = new Owner(port);
103+
await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsTrue();
104+
await Assert.That(owner.Heard.Count).IsEqualTo(1);
105+
}
106+
107+
/// <summary>
108+
/// Per port, or a test suite's dead ephemeral port would silence the sends to its live one.
109+
/// </summary>
110+
[Test]
111+
public async Task PortsAreRememberedApart()
112+
{
113+
var dead = FreePort();
114+
using var owner = new Owner();
115+
await Assert.That(ViewerClient.TrySend(settle, out _, dead, skipIfUnowned: true)).IsFalse();
116+
117+
await Assert.That(ViewerClient.TrySend(settle, out _, owner.Port, skipIfUnowned: true)).IsTrue();
118+
await Assert.That(owner.Heard.Count).IsEqualTo(1);
119+
}
120+
121+
[Test]
122+
public async Task TheAsyncSendRemembersToo()
123+
{
124+
var port = FreePort();
125+
await Assert.That(await ViewerClient.SendAsync(settle, Cancel.None, port, skipIfUnowned: true))
126+
.IsEqualTo(SendOutcome.NoOwner);
127+
128+
using var owner = new Owner(port);
129+
await Assert.That(await ViewerClient.SendAsync(settle, Cancel.None, port, skipIfUnowned: true))
130+
.IsEqualTo(SendOutcome.NoOwner);
131+
await Assert.That(owner.Heard).IsEmpty();
132+
133+
await Assert.That(ViewerClient.IsOwned(port)).IsTrue();
134+
await Assert.That(await ViewerClient.SendAsync(settle, Cancel.None, port, skipIfUnowned: true))
135+
.IsEqualTo(SendOutcome.Accepted);
136+
}
137+
138+
/// <summary>
139+
/// A caller that does not say is asking, and an ask is never answered from memory. This is
140+
/// what keeps the hosts and the IDE plugin, which share this client, noticing an owner arrive.
141+
/// </summary>
142+
[Test]
143+
public async Task AskingIsTheDefault()
144+
{
145+
var port = FreePort();
146+
ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true);
147+
using var owner = new Owner(port);
148+
149+
await Assert.That(await ViewerClient.SendAsync(settle, Cancel.None, port)).IsEqualTo(SendOutcome.Accepted);
150+
await Assert.That(ViewerClient.TrySend(settle, out _, port)).IsTrue();
151+
await Assert.That(owner.Heard.Count).IsEqualTo(2);
152+
}
153+
154+
/// <summary>
155+
/// A port that is free right now, found by binding and releasing it.
156+
/// </summary>
157+
static int FreePort()
158+
{
159+
if (!ViewerServer.TryBind(0, out var bound))
160+
{
161+
throw new("Could not bind an ephemeral port.");
162+
}
163+
164+
var port = bound.Port;
165+
bound.Dispose();
166+
return port;
167+
}
168+
169+
/// <summary>
170+
/// A queue owner on a port of the test's choosing, answering every verb and recording it.
171+
/// Zero binds an ephemeral port, for a test that only needs somebody to be there.
172+
/// </summary>
173+
sealed class Owner : IDisposable
174+
{
175+
readonly ViewerServer server;
176+
readonly CancelSource cancel = new();
177+
readonly Task listening;
178+
readonly Lock gate = new();
179+
readonly List<ViewerVerb> heard = [];
180+
181+
public Owner(int port = 0)
182+
{
183+
if (!ViewerServer.TryBind(port, out var bound))
184+
{
185+
throw new($"Could not bind port {port}.");
186+
}
187+
188+
server = bound;
189+
listening = server.Listen(
190+
_ =>
191+
{
192+
lock (gate)
193+
{
194+
heard.Add(_.Verb);
195+
}
196+
197+
return ViewerResponse.Success();
198+
},
199+
cancel.Token);
200+
}
201+
202+
public int Port => server.Port;
203+
204+
public IReadOnlyList<ViewerVerb> Heard
205+
{
206+
get
207+
{
208+
lock (gate)
209+
{
210+
return heard.ToList();
211+
}
212+
}
213+
}
214+
215+
public void Dispose()
216+
{
217+
cancel.Cancel();
218+
server.Dispose();
219+
try
220+
{
221+
listening.Wait(TimeSpan.FromSeconds(2));
222+
}
223+
catch (AggregateException)
224+
{
225+
// Cancellation unwinding through the listener; nothing to report
226+
}
227+
228+
cancel.Dispose();
229+
}
230+
}
231+
}

0 commit comments

Comments
 (0)