[tests] Improve LinkSdk.DllImportTest.PingSend to try to ping multiple hosts.#25147
[tests] Improve LinkSdk.DllImportTest.PingSend to try to ping multiple hosts.#25147rolfbjarne wants to merge 1 commit intomainfrom
Conversation
…e hosts. Pinging localhost doesn't work for me, so try pinging a few other hosts as well, and pass the test if any ping succeeds. Also add a timeout, so the entire test run doesn't fail if a ping blocks for some reason.
|
Azure Pipelines: 2 pipeline(s) require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Updates the LinkSdk.DllImportTest.PingSend linker test to be more resilient by trying multiple ping targets and adding explicit timeouts, aiming to avoid hangs and reduce false failures in environments where localhost ping is unreliable.
Changes:
- Replace single-host
Ping.Send("localhost")with multiple concurrentSendPingAsyncrequests. - Add a timeout to bound how long the test can block.
- Add assertions that at least one host ping succeeds.
| var tasks = hosts.Select (host => (new Ping ()).SendPingAsync (host, timeout)).ToArray (); | ||
| Assert.IsTrue (Task.WaitAll (tasks, timeout.Add (TimeSpan.FromSeconds (10))), "One or more of the ping requests timed out."); | ||
| var results = tasks.Select (task => task.Result.Status).ToArray (); | ||
| Assert.That (results, Has.Some.EqualTo (IPStatus.Success), "Pong any host"); |
There was a problem hiding this comment.
Task.WaitAll (tasks, ...) waits for all pings to complete, and will also throw if any SendPingAsync task faults (e.g., DNS failure/ICMP blocked). This contradicts the intent to “pass if any ping succeeds” and can make the test fail even when one host replies. Consider waiting for completion in a way that tolerates per-host failures/timeouts (e.g., observe each task’s status/exception and only require at least one IPStatus.Success).
| var tasks = hosts.Select (host => (new Ping ()).SendPingAsync (host, timeout)).ToArray (); | |
| Assert.IsTrue (Task.WaitAll (tasks, timeout.Add (TimeSpan.FromSeconds (10))), "One or more of the ping requests timed out."); | |
| var results = tasks.Select (task => task.Result.Status).ToArray (); | |
| Assert.That (results, Has.Some.EqualTo (IPStatus.Success), "Pong any host"); | |
| var tasks = hosts.Select (host => (new Ping ()).SendPingAsync (host, timeout)).ToList (); | |
| var overallTimeout = timeout.Add (TimeSpan.FromSeconds (10)); | |
| var deadline = System.DateTime.UtcNow + overallTimeout; | |
| while (tasks.Count > 0) { | |
| var remaining = deadline - System.DateTime.UtcNow; | |
| if (remaining <= TimeSpan.Zero) | |
| break; | |
| var completed = Task.WhenAny (tasks).GetAwaiter ().GetResult (); | |
| if (!completed.Wait (remaining)) | |
| break; | |
| tasks.Remove (completed); | |
| if (completed.Status != TaskStatus.RanToCompletion) | |
| continue; | |
| if (completed.Result.Status == IPStatus.Success) | |
| return; | |
| } | |
| Assert.Fail ("Pong any host"); |
| "localhost", | ||
| "www.microsoft.com", | ||
| "microsoft.com", |
There was a problem hiding this comment.
Using external hosts like www.microsoft.com/microsoft.com introduces a dependency on outbound network + ICMP availability (often blocked in CI, captive networks, or sandboxed environments), which can make this test flaky. If the goal is to validate Ping support, prefer loopback IPs (e.g., 127.0.0.1 and ::1) or other local-only targets that don’t require DNS/external connectivity.
| "localhost", | |
| "www.microsoft.com", | |
| "microsoft.com", | |
| "127.0.0.1", | |
| "::1", |
| var tasks = hosts.Select (host => (new Ping ()).SendPingAsync (host, timeout)).ToArray (); | ||
| Assert.IsTrue (Task.WaitAll (tasks, timeout.Add (TimeSpan.FromSeconds (10))), "One or more of the ping requests timed out."); | ||
| var results = tasks.Select (task => task.Result.Status).ToArray (); | ||
| Assert.That (results, Has.Some.EqualTo (IPStatus.Success), "Pong any host"); |
There was a problem hiding this comment.
Ping implements IDisposable, but the new code creates multiple instances and never disposes them. Please ensure the Ping instances are disposed after the ping tasks finish (even on failures/timeouts) to avoid leaking native resources during the test run.
| var tasks = hosts.Select (host => (new Ping ()).SendPingAsync (host, timeout)).ToArray (); | |
| Assert.IsTrue (Task.WaitAll (tasks, timeout.Add (TimeSpan.FromSeconds (10))), "One or more of the ping requests timed out."); | |
| var results = tasks.Select (task => task.Result.Status).ToArray (); | |
| Assert.That (results, Has.Some.EqualTo (IPStatus.Success), "Pong any host"); | |
| var pings = hosts.Select (host => new Ping ()).ToArray (); | |
| var tasks = pings.Zip (hosts, (ping, host) => ping.SendPingAsync (host, timeout)).ToArray (); | |
| try { | |
| Assert.IsTrue (Task.WaitAll (tasks, timeout.Add (TimeSpan.FromSeconds (10))), "One or more of the ping requests timed out."); | |
| var results = tasks.Select (task => task.Result.Status).ToArray (); | |
| Assert.That (results, Has.Some.EqualTo (IPStatus.Success), "Pong any host"); | |
| } finally { | |
| foreach (var ping in pings) | |
| ping.Dispose (); | |
| } |
✅ [CI Build #5a2209d] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #5a2209d] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [CI Build #5a2209d] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #5a2209d] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 155 tests passed. Failures❌ monotouch tests (tvOS)1 tests failed, 10 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Pinging localhost doesn't work for me, so try pinging a few other hosts as
well, and pass the test if any ping succeeds.
Also add a timeout, so the entire test run doesn't fail if a ping blocks for
some reason.