diff --git a/src/DiffEngine.Tests/HeldPort.cs b/src/DiffEngine.Tests/HeldPort.cs new file mode 100644 index 00000000..088f4772 --- /dev/null +++ b/src/DiffEngine.Tests/HeldPort.cs @@ -0,0 +1,85 @@ +/// +/// A port bound and not listening, for a test that needs one nobody is on. Bound rather than found +/// free and let go, so that nothing else is handed it while the test is using it: a test beside +/// this one binding port zero could otherwise be given the same port, and be told whatever this +/// test had the listener table say about it (), or be found +/// listening there by a test asking whether anybody is. +/// +sealed class HeldPort : IDisposable +{ + readonly Socket socket; + + public HeldPort() : + this(IPAddress.Loopback, 0) + { + } + + HeldPort(IPAddress address, int port) + { + socket = new(address.AddressFamily, SocketType.Stream, ProtocolType.Tcp); + try + { + socket.Bind(new IPEndPoint(address, port)); + } + catch + { + socket.Dispose(); + throw; + } + + Port = ((IPEndPoint) socket.LocalEndPoint!).Port; + } + + /// + /// A listener of the test's own on , asked about by + /// while it listens and by once it has + /// stopped, with the port held again in between. The tests beside one bind port zero, and + /// Windows hands a port straight back out: asked about while free, a port was found listened + /// on by one of them within milliseconds. Where one had it first, this starts again on another. + /// + public static async Task AfterListening(IPAddress address, Func listening, Func stopped) + { + for (var attempt = 0; attempt < 10; attempt++) + { + var listener = new TcpListener(address, 0); + listener.Start(); + var port = ((IPEndPoint) listener.LocalEndpoint).Port; + try + { + await listening(port); + } + finally + { + listener.Stop(); + } + + using var held = TryAgain(address, port); + if (held is null) + { + continue; + } + + await stopped(port); + return; + } + + throw new("Every port was handed to something else before it could be held again."); + } + + static HeldPort? TryAgain(IPAddress address, int port) + { + try + { + return new(address, port); + } + catch (SocketException) + { + return null; + } + } + + public int Port { get; } + + public void Dispose() => + socket.Dispose(); +} diff --git a/src/DiffEngine.Tests/ListenerTableTests.cs b/src/DiffEngine.Tests/ListenerTableTests.cs index 3f357a66..0bfb531a 100644 --- a/src/DiffEngine.Tests/ListenerTableTests.cs +++ b/src/DiffEngine.Tests/ListenerTableTests.cs @@ -15,26 +15,53 @@ public class ListenerTableTests /// running is asked of it there. /// [Test] - public async Task AListenerIsListedOnEveryPlatform() + public Task AListenerIsListedOnEveryPlatform() => + HeldPort.AfterListening( + IPAddress.Loopback, + async _ => + { + await Assert.That(ListenerTable.IsListed(_)).IsTrue(); + await Assert.That(ListenerTable.Enumerated(_)).IsTrue(); + }, + async _ => await Assert.That(ListenerTable.IsListed(_)).IsFalse()); + + /// + /// Answers that overlap and end in either order leave the table answering for both ports, + /// which is what a lookup each test wrapped around the one before did not: the second test to + /// finish put back the first one's. + /// + [Test] + public async Task AnswersThatOverlapLeaveNothingBehind() { - var listener = new TcpListener(IPAddress.Loopback, 0); - listener.Start(); - var port = ((IPEndPoint) listener.LocalEndpoint).Port; - bool listed; - bool enumerated; - try - { - listed = ListenerTable.IsListed(port); - enumerated = ListenerTable.Enumerated(port); - } - finally - { - listener.Stop(); - } + using var first = new HeldPort(); + using var second = new HeldPort(); - await Assert.That(listed).IsTrue(); - await Assert.That(enumerated).IsTrue(); - await Assert.That(ListenerTable.IsListed(port)).IsFalse(); + var firstAnswer = ListenerTable.Answer(first.Port, () => true); + var secondAnswer = ListenerTable.Answer(second.Port, () => true); + await Assert.That(ListenerTable.IsListed(first.Port)).IsTrue(); + await Assert.That(ListenerTable.IsListed(second.Port)).IsTrue(); + + firstAnswer.Dispose(); + await Assert.That(ListenerTable.IsListed(first.Port)).IsFalse(); + await Assert.That(ListenerTable.IsListed(second.Port)).IsTrue(); + + secondAnswer.Dispose(); + await Assert.That(ListenerTable.IsListed(second.Port)).IsFalse(); + } + + /// + /// One answer a port, so two tests that believe they hold the same port find out, rather than + /// one quietly answering for the other. + /// + [Test] + public async Task APortIsAnsweredForOnce() + { + using var held = new HeldPort(); + using var answer = ListenerTable.Answer(held.Port, () => true); + + await Assert.That(() => ListenerTable.Answer(held.Port, () => false)) + .Throws(); + await Assert.That(ListenerTable.IsListed(held.Port)).IsTrue(); } [Test] @@ -96,19 +123,10 @@ public async Task AListenerSaysWhoseItIs() IPAddress[] addresses = Socket.OSSupportsIPv6 ? [IPAddress.Loopback, IPAddress.IPv6Loopback] : [IPAddress.Loopback]; foreach (var address in addresses) { - var listener = new TcpListener(address, 0); - listener.Start(); - var port = ((IPEndPoint) listener.LocalEndpoint).Port; - try - { - await Assert.That(ListenerTable.Owner(port)).IsEqualTo(current.Id); - } - finally - { - listener.Stop(); - } - - await Assert.That(ListenerTable.Owner(port)).IsNull(); + await HeldPort.AfterListening( + address, + async _ => await Assert.That(ListenerTable.Owner(_)).IsEqualTo(current.Id), + async _ => await Assert.That(ListenerTable.Owner(_)).IsNull()); } } diff --git a/src/DiffEngine.Tests/TrayDetectorTests.cs b/src/DiffEngine.Tests/TrayDetectorTests.cs index 24a2b17a..451825d3 100644 --- a/src/DiffEngine.Tests/TrayDetectorTests.cs +++ b/src/DiffEngine.Tests/TrayDetectorTests.cs @@ -7,24 +7,11 @@ public class TrayDetectorTests { [Test] [RunOn(TUnit.Core.Enums.OS.Linux | TUnit.Core.Enums.OS.MacOs)] - public async Task AListenerOnThePiperPortIsATray() - { - var listener = new TcpListener(IPAddress.Loopback, 0); - listener.Start(); - var port = ((IPEndPoint) listener.LocalEndpoint).Port; - bool running; - try - { - running = TrayDetector.IsRunning(port); - } - finally - { - listener.Stop(); - } - - await Assert.That(running).IsTrue(); - await Assert.That(TrayDetector.IsRunning(port)).IsFalse(); - } + public Task AListenerOnThePiperPortIsATray() => + HeldPort.AfterListening( + IPAddress.Loopback, + async _ => await Assert.That(TrayDetector.IsRunning(_)).IsTrue(), + async _ => await Assert.That(TrayDetector.IsRunning(_)).IsFalse()); /// /// No tray, where the piper's send would have taken it as one and let the connect decide: @@ -38,20 +25,18 @@ public async Task ATableThatCannotBeReadIsNoTray() var listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); var port = ((IPEndPoint) listener.LocalEndpoint).Port; - var previous = ListenerTable.Lookup; - // Every other port goes through untouched, since the tests beside this one are asking - // about theirs at the same time - ListenerTable.Lookup = _ => _ == port ? throw new("The table could not be read.") : previous(_); bool running; bool held; try { - running = TrayDetector.IsRunning(port); - held = ListenerTable.IsHeld(port); + using (ListenerTable.Answer(port, () => throw new("The table could not be read."))) + { + running = TrayDetector.IsRunning(port); + held = ListenerTable.IsHeld(port); + } } finally { - ListenerTable.Lookup = previous; listener.Stop(); } diff --git a/src/DiffEngine.Tests/ViewerClientUnownedTests.cs b/src/DiffEngine.Tests/ViewerClientUnownedTests.cs index 537d23d4..a99bf5ea 100644 --- a/src/DiffEngine.Tests/ViewerClientUnownedTests.cs +++ b/src/DiffEngine.Tests/ViewerClientUnownedTests.cs @@ -168,13 +168,16 @@ public async Task AnOwnerStartedLaterIsFoundByTheTellingSends() public async Task APortAConnectFoundEmptyIsNotAskedAboutAgainSoSoon() { ViewerClient.RecheckUnlistedAfter = TimeSpan.Zero; - var port = FreePort(); + var held = new HeldPort(); + var port = held.Port; using var unreadable = new Lookup(port) { Unreadable = true }; await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse(); + // Let go only to be listened on, so the port is held again a moment later + held.Dispose(); using var owner = new Owner(port); await Assert.That(ViewerClient.TrySend(settle, out _, port, skipIfUnowned: true)).IsFalse(); await Assert.That(owner.Heard).IsEmpty(); @@ -303,7 +306,8 @@ public async Task APortNobodyHoldsIsNotProbed() public async Task ATableThatCannotBeReadLeavesTheConnectToAnswer() { using var owner = new Owner(); - var dead = FreePort(); + using var held = new HeldPort(); + var dead = held.Port; using var unreadable = new Lookup(owner.Port, dead) { Unreadable = true @@ -457,19 +461,17 @@ await Assert.That(await ViewerClient.SendAsync(settle, Cancel.None, 0, skipIfUno /// Stands in front of the listener table for the ports a test names, counting how often each /// was asked about and, when told to, failing the way a table that cannot be read does, or /// saying nobody listens there whoever does. Every other port goes through untouched, since - /// the tests beside this one are asking about theirs at the same time. + /// the tests beside this one are asking about theirs at the same time. Each port named is one + /// the test holds ( or an ), or port zero, which + /// nothing is ever handed. /// sealed class Lookup : IDisposable { - readonly Func previous = ListenerTable.Lookup; - readonly int[] ports; + readonly IDisposable[] answers; int asked; - public Lookup(params int[] ports) - { - this.ports = ports; - ListenerTable.Lookup = Answer; - } + public Lookup(params int[] ports) => + answers = ports.Select(_ => ListenerTable.Answer(_, () => Answer(_))).ToArray(); public bool Unreadable { get; init; } @@ -479,11 +481,6 @@ public Lookup(params int[] ports) bool Answer(int port) { - if (!ports.Contains(port)) - { - return previous(port); - } - Interlocked.Increment(ref asked); if (Unreadable) { @@ -495,11 +492,16 @@ bool Answer(int port) return false; } - return previous(port); + return ListenerTable.Listed(port); } - public void Dispose() => - ListenerTable.Lookup = previous; + public void Dispose() + { + foreach (var answer in answers) + { + answer.Dispose(); + } + } } /// diff --git a/src/DiffEngine.Tests/ViewerProtocolTests.cs b/src/DiffEngine.Tests/ViewerProtocolTests.cs index 3167c67f..6f6b27b9 100644 --- a/src/DiffEngine.Tests/ViewerProtocolTests.cs +++ b/src/DiffEngine.Tests/ViewerProtocolTests.cs @@ -1455,9 +1455,8 @@ await Assert.That(await ViewerClient.SendAsync(new(ViewerVerb.List), default, po [NotInParallel] public async Task AConnectGivenUpOnIsObserved() { - ViewerServer.TryBind(0, out var server); - var port = server!.Port; - server.Dispose(); + using var held = new HeldPort(); + var port = held.Port; ViewerClient.ForgetUnowned(); var unobserved = new List(); @@ -1469,8 +1468,7 @@ void Record(object? sender, UnobservedTaskExceptionEventArgs args) } } - var lookup = ListenerTable.Lookup; - ListenerTable.Lookup = _ => _ == port || lookup(_); + using var listed = ListenerTable.Answer(port, () => true); TaskScheduler.UnobservedTaskException += Record; try { @@ -1491,7 +1489,6 @@ void Record(object? sender, UnobservedTaskExceptionEventArgs args) finally { TaskScheduler.UnobservedTaskException -= Record; - ListenerTable.Lookup = lookup; ViewerClient.ForgetUnowned(); } diff --git a/src/DiffEngine/Protocol/ListenerTable.cs b/src/DiffEngine/Protocol/ListenerTable.cs index dc722323..bb7728c7 100644 --- a/src/DiffEngine/Protocol/ListenerTable.cs +++ b/src/DiffEngine/Protocol/ListenerTable.cs @@ -65,14 +65,48 @@ public static bool IsListed(int port) } /// - /// What and ask. Replaced by the tests, for the two things about the table - /// that cannot be arranged or seen from outside: that it could not be read, and that it was - /// not asked at all. Given the port, so a test can answer for its own and pass every other - /// through: the tests beside it are asking about theirs at the same time. + /// What and ask: a test's answer for the port, + /// where one stands (), and otherwise the table. /// - internal static Func Lookup { get; set; } = Listed; + static bool Lookup(int port) => + answers.TryGetValue(port, out var answer) ? answer() : Listed(port); - static bool Listed(int port) + static readonly ConcurrentDictionary> answers = new(); + + /// + /// Answers for in place of the table until disposed. For the tests, and + /// the two things about the table that cannot be arranged or seen from outside: that it could + /// not be read, which is an answer that throws, and that it was not asked at all, which is one + /// that counts. is the table's own answer, for one that passes through. + /// + /// One answer a port, kept apart from every other port's, since the tests beside one are asking + /// about theirs at the same time. It was a lookup a test replaced with one that wrapped the + /// lookup before it, and two tests that overlapped put each other's back: the one left + /// answered for its port for the rest of the run. + /// + /// + /// Only for a port the test holds, listening or bound, for as long as the answer stands. A free + /// port can be handed to a test that binds port zero, which is then told this answer about its + /// own listener. + /// + /// + internal static IDisposable Answer(int port, Func answer) + { + if (!answers.TryAdd(port, answer)) + { + throw new InvalidOperationException($"Port {port} is already answered for."); + } + + return new Answered(port, answer); + } + + sealed class Answered(int port, Func answer) : IDisposable + { + public void Dispose() => + ((ICollection>>) answers).Remove(new(port, answer)); + } + + internal static bool Listed(int port) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Listeners(port) is { } held) diff --git a/src/DiffEngineTray.Core.Tests/TrackerSnapshotTest.cs b/src/DiffEngineTray.Core.Tests/TrackerSnapshotTest.cs index 1dd4959f..de79b808 100644 --- a/src/DiffEngineTray.Core.Tests/TrackerSnapshotTest.cs +++ b/src/DiffEngineTray.Core.Tests/TrackerSnapshotTest.cs @@ -225,29 +225,20 @@ public async Task TheOwnerIsLookedForInTheListenerTable() { using var viewer = new FakeViewer("Sample.cs:1"); var port = ViewerClient.Port; - var previous = ListenerTable.Lookup; var asked = 0; - ListenerTable.Lookup = _ => - { - if (_ == port) + using var counted = ListenerTable.Answer( + port, + () => { asked++; - } + return ListenerTable.Listed(port); + }); - return previous(_); - }; - try - { - var answered = new RemoteInlineHost().TryList(out var pending); + var answered = new RemoteInlineHost().TryList(out var pending); - await Assert.That(answered).IsTrue(); - await Assert.That(pending.Select(_ => _.Name)).IsEquivalentTo(["Sample.cs:1"]); - await Assert.That(asked).IsGreaterThan(0); - } - finally - { - ListenerTable.Lookup = previous; - } + await Assert.That(answered).IsTrue(); + await Assert.That(pending.Select(_ => _.Name)).IsEquivalentTo(["Sample.cs:1"]); + await Assert.That(asked).IsGreaterThan(0); } /// @@ -258,21 +249,13 @@ public async Task TheOwnerIsLookedForInTheListenerTable() public async Task NobodyInTheListenerTableIsNoOwner() { using var viewer = new FakeViewer("Sample.cs:1"); - var port = ViewerClient.Port; - var previous = ListenerTable.Lookup; - ListenerTable.Lookup = _ => _ != port && previous(_); - try - { - var answered = new RemoteInlineHost().TryList(out var pending); + using var unlisted = ListenerTable.Answer(ViewerClient.Port, () => false); - await Assert.That(answered).IsTrue(); - await Assert.That(pending).IsEmpty(); - await Assert.That(viewer.Verbs).IsEmpty(); - } - finally - { - ListenerTable.Lookup = previous; - } + var answered = new RemoteInlineHost().TryList(out var pending); + + await Assert.That(answered).IsTrue(); + await Assert.That(pending).IsEmpty(); + await Assert.That(viewer.Verbs).IsEmpty(); } ///