Skip to content

Commit 4e454ff

Browse files
authored
Harden CancelSendPingAsync tests (#116588)
Fixes #113831. Separate `SendPingAsync(hostname)` and `SendPingAsync(IPAddress)` test implementations: - `SendPingAsync(hostname)`: we didn't see issues with this variant in the CI. It effectively tests if the resolution of `www.microsoft.com` is cancelled as part of the `SendPingAsync` call. Keep the existing logic in a separate method. - `SendPingAsync(IPAddress)`: occasionally, pinging the valid IP likely resulted in replies before the cancellation could kick in on Helix Mac machines. Instead, let's ping an unreachable IP to avoid this. Since that can also sometimes result to an ICMP reply instead of a timeout, use the same trick as in #116500, repeating the test with other hosts if it happens.
1 parent ff7b6c5 commit 4e454ff

File tree

1 file changed

+52
-15
lines changed
  • src/libraries/System.Net.Ping/tests/FunctionalTests

1 file changed

+52
-15
lines changed

src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -687,24 +687,15 @@ public async Task SendPingAsyncWithAlreadyCanceledToken(bool useIPAddress)
687687
}
688688

689689
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
690-
[InlineData(false, false)]
691-
[InlineData(false, true)]
692-
[InlineData(true, false)]
693-
[InlineData(true, true)]
694-
[OuterLoop] // Depends on external host and assumption that successful ping takes long enough for cancellation to go through first
695-
public async Task CancelSendPingAsync(bool useIPAddress, bool useCancellationToken)
696-
{
697-
if (PlatformDetection.IsOSX && useIPAddress && !useCancellationToken)
698-
{
699-
throw new SkipTestException("[ActiveIssue(https://github.com/dotnet/runtime/issues/114782)]");
700-
}
701-
690+
[InlineData(false)]
691+
[InlineData(true)]
692+
[OuterLoop("Depends on external host.")]
693+
public async Task CancelSendPingAsync_HostName(bool useCancellationToken)
694+
{
702695
using CancellationTokenSource source = new();
703696

704697
using Ping ping = new();
705-
Task pingTask = useIPAddress
706-
? ping.SendPingAsync((await Dns.GetHostAddressesAsync(Test.Common.Configuration.Ping.PingHost))[0], TimeSpan.FromSeconds(5), cancellationToken: source.Token)
707-
: ping.SendPingAsync(Test.Common.Configuration.Ping.PingHost, TimeSpan.FromSeconds(5), cancellationToken: source.Token);
698+
Task pingTask = ping.SendPingAsync(Test.Common.Configuration.Ping.PingHost, TimeSpan.FromSeconds(5), cancellationToken: source.Token);
708699
if (useCancellationToken)
709700
{
710701
source.Cancel();
@@ -717,6 +708,52 @@ public async Task CancelSendPingAsync(bool useIPAddress, bool useCancellationTok
717708
Assert.True(pingTask.IsCanceled);
718709
}
719710

711+
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
712+
[InlineData(false)]
713+
[InlineData(true)]
714+
[OuterLoop("Depends on external host and runs long on Windows.")]
715+
public async Task CancelSendPingAsync_IPAddress(bool useCancellationToken)
716+
{
717+
if (await TestCore(TestSettings.UnreachableAddress)) return;
718+
if (await TestCore(TestSettings.UnreachableAddress2)) return;
719+
if (await TestCore(TestSettings.UnreachableAddress3)) return;
720+
721+
Assert.Fail("No OperationCanceledException has been thrown after attempting cancellation with various unreachable hosts.");
722+
723+
async Task<bool> TestCore(string unreachableIPString)
724+
{
725+
IPAddress address = IPAddress.Parse(unreachableIPString);
726+
using CancellationTokenSource source = new();
727+
728+
using Ping ping = new();
729+
Task<PingReply> pingTask = ping.SendPingAsync(address, TimeSpan.FromSeconds(5), cancellationToken: source.Token);
730+
if (useCancellationToken)
731+
{
732+
source.Cancel();
733+
}
734+
else
735+
{
736+
ping.SendAsyncCancel();
737+
}
738+
739+
try
740+
{
741+
PingReply reply = await pingTask;
742+
if (reply.Status == IPStatus.DestinationNetworkUnreachable)
743+
{
744+
_output.WriteLine($"We got a DestinationNetworkUnreachable reply before cancellation for {address}. Retry on a different address.");
745+
return false;
746+
}
747+
748+
Assert.Fail("No OperationCanceledException has been thrown.");
749+
}
750+
catch (OperationCanceledException) { }
751+
752+
Assert.True(pingTask.IsCanceled);
753+
return true;
754+
}
755+
}
756+
720757
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
721758
[OuterLoop] // Depends on external host and assumption that network respects and does not change TTL
722759
public async Task SendPingToExternalHostWithLowTtlTest()

0 commit comments

Comments
 (0)