[tests] Fix flaky NSPasteboardTests.DetectMetadataTests on macOS - #25450
Conversation
The DetectMetadata tests were flaky because the macOS pasteboard subsystem may not have fully analyzed content metadata by the time DetectMetadata is called immediately after SetStringForType. The callback would be invoked but return an empty dictionary. Fix by: - Adding retry logic (up to 5 attempts with 500ms delays) for the '#2' detection case where we expect results after writing content - Increasing WaitOne timeouts from 1s to 10s for CI under load Fixes #25446 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses flakiness in the macOS NSPasteboard metadata detection tests by accommodating asynchronous pasteboard metadata analysis, which can return empty results immediately after writing content (as seen in issue #25446).
Changes:
- Increased
WaitOnetimeouts forDetectMetadatacallbacks from 1s to 10s to reduce CI timing sensitivity. - Added retry logic (up to 5 attempts with 500ms delays) for the “#2”
DetectMetadatacase after writing a file URL to the pasteboard.
Comments suppressed due to low confidence (2)
tests/monotouch-test/AppKit/NSPasteboard.cs:434
- The retry loop only checks
detected.Countand doesn’t handle a non-nullerrorper attempt. If an attempt returns an error, the loop may continue and overwrite it, potentially hiding a real problem. Suggest failing immediately (or at least assertingerroris null) before deciding to retry.
for (int attempt = 0; attempt < 5; attempt++) {
evt.Reset ();
detected = null;
error = null;
pasteboard.DetectMetadata (hashSet, callback);
Assert.That (evt.WaitOne (TimeSpan.FromSeconds (10)), "SomewhatStronglyTyped DetectMetadata #2 wait");
Assert.That (detected, Is.Not.Null, "SomewhatStronglyTyped DetectMetadata #2 patterns");
if ((int) detected.Count > 0)
break;
Thread.Sleep (500);
tests/monotouch-test/AppKit/NSPasteboard.cs:498
- The retry loop ignores
erroruntil after the loop, so an intermediateerrorcan be overwritten by a later attempt and the test may pass unexpectedly. It would be safer to asserterroris null on each attempt (and avoid retrying when an actual error is returned).
for (int attempt = 0; attempt < 5; attempt++) {
evt.Reset ();
detected = null;
error = null;
pasteboard.DetectMetadata (hashSet, callback);
Assert.That (evt.WaitOne (TimeSpan.FromSeconds (10)), "StronglyTyped DetectMetadata #2 wait");
Assert.That (detected, Is.Not.Null, "StronglyTyped DetectMetadata #2 patterns");
if ((int) detected.Count > 0)
break;
Thread.Sleep (500);
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #54e7534] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ 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 |
✅ [PR Build #54e7534] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #54e7534] Build passed (Build macOS tests) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #54e7534] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 175 tests passed 🎉 Tests counts✅ 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 |
The DetectMetadata tests were flaky because the macOS pasteboard subsystem may not have fully analyzed content metadata by the time DetectMetadata is called immediately after SetStringForType. The callback would be invoked but return an empty dictionary.
Fix by:
Fixes #25446