From ffe3cbab40ba47fed9eb29178e729e2a4ea4d47f Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Tue, 10 Jun 2025 22:22:33 +0200 Subject: [PATCH 1/2] harden Ping_TimedOut_* tests --- .../tests/FunctionalTests/PingTest.cs | 81 +++++++++++-------- .../tests/FunctionalTests/TestSettings.cs | 3 + 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs index 49a95c858a9af8..2d4d74c51d18e4 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs @@ -745,53 +745,64 @@ public async Task SendPingToExternalHostWithLowTtlTest() Assert.NotEqual(IPAddress.Any, pingReply.Address); } - [Fact] - [OuterLoop] - public void Ping_TimedOut_Sync_Success() + private async Task Ping_TimedOut_Core(Func> sendPing) { - var sender = new Ping(); - PingReply reply = sender.Send(TestSettings.UnreachableAddress); + Ping sender = new Ping(); + PingReply reply = await sendPing(sender, TestSettings.UnreachableAddress); + if (reply.Status == IPStatus.DestinationNetworkUnreachable) + { + // The network has dropped the packet towards UnreachableAddress. Repeat the PING attempt on another address. + reply = await sendPing(sender, TestSettings.UnreachableAddress2); + } + + if (reply.Status == IPStatus.DestinationNetworkUnreachable) + { + // Do another attempt. + reply = await sendPing(sender, TestSettings.UnreachableAddress3); + } + Assert.Equal(IPStatus.TimedOut, reply.Status); } [Fact] [OuterLoop] - public async Task Ping_TimedOut_EAP_Success() - { - var sender = new Ping(); - sender.PingCompleted += (s, e) => - { - var tcs = (TaskCompletionSource)e.UserState; + public Task Ping_TimedOut_Sync_Success() + => Ping_TimedOut_Core((sender, address) => Task.Run(() => sender.Send(address))); - if (e.Cancelled) - { - tcs.TrySetCanceled(); - } - else if (e.Error != null) - { - tcs.TrySetException(e.Error); - } - else + [Fact] + [OuterLoop] + public Task Ping_TimedOut_EAP_Success() + => Ping_TimedOut_Core(async (sender, address) => + { + static void PingCompleted(object sender, PingCompletedEventArgs e) { - tcs.TrySetResult(e.Reply); - } - }; + var tcs = (TaskCompletionSource)e.UserState; - var tcs = new TaskCompletionSource(); - sender.SendAsync(TestSettings.UnreachableAddress, tcs); - - PingReply reply = await tcs.Task; - Assert.Equal(IPStatus.TimedOut, reply.Status); - } + if (e.Cancelled) + { + tcs.TrySetCanceled(); + } + else if (e.Error != null) + { + tcs.TrySetException(e.Error); + } + else + { + tcs.TrySetResult(e.Reply); + } + } + sender.PingCompleted += PingCompleted; + var tcs = new TaskCompletionSource(); + sender.SendAsync(address, tcs); + PingReply reply = await tcs.Task; + sender.PingCompleted -= PingCompleted; + return reply; + }); [Fact] [OuterLoop] - public async Task Ping_TimedOut_TAP_Success() - { - var sender = new Ping(); - PingReply reply = await sender.SendPingAsync(TestSettings.UnreachableAddress); - Assert.Equal(IPStatus.TimedOut, reply.Status); - } + public Task Ping_TimedOut_TAP_Success() + => Ping_TimedOut_Core((sender, address) => sender.SendPingAsync(address)); private static bool IsRemoteExecutorSupportedAndPrivilegedProcess => RemoteExecutor.IsSupported && PlatformDetection.IsPrivilegedProcess; diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs index 0e48bb4777b2e1..78b2be986651e6 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/TestSettings.cs @@ -11,6 +11,9 @@ internal static class TestSettings { public static readonly string LocalHost = "localhost"; public static readonly string UnreachableAddress = "192.0.2.0"; // TEST-NET-1 + public static readonly string UnreachableAddress2 = "100.64.0.1"; // CGNAT block + public static readonly string UnreachableAddress3 = "10.255.255.1"; // High address in the private 10.0.0.0/8 range. Likely unused and unrouted. + public const int PingTimeout = 10 * 1000; public const string PayloadAsString = "'Post hoc ergo propter hoc'. 'After it, therefore because of it'. It means one thing follows the other, therefore it was caused by the other. But it's not always true. In fact it's hardly ever true."; From 6f8adf250ef506cba004847fbed7b4717746ae0e Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 11 Jun 2025 13:57:20 +0200 Subject: [PATCH 2/2] improve comments --- .../System.Net.Ping/tests/FunctionalTests/PingTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs index 2d4d74c51d18e4..323829662c315d 100644 --- a/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs +++ b/src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs @@ -751,13 +751,13 @@ private async Task Ping_TimedOut_Core(Func> sendPi PingReply reply = await sendPing(sender, TestSettings.UnreachableAddress); if (reply.Status == IPStatus.DestinationNetworkUnreachable) { - // The network has dropped the packet towards UnreachableAddress. Repeat the PING attempt on another address. + // A network middleware has dropped the packed and replied with DestinationNetworkUnreachable. Repeat the PING attempt on another address. reply = await sendPing(sender, TestSettings.UnreachableAddress2); } if (reply.Status == IPStatus.DestinationNetworkUnreachable) { - // Do another attempt. + // Do yet another attempt. reply = await sendPing(sender, TestSettings.UnreachableAddress3); }