Repository navigation
Conversation
…ename On macOS, fseventsd can occasionally replay directory/file creation events after the native watcher starts, racing with assertions that expect no changes to have been reported yet. Add a short delay before starting the watcher to reduce (not eliminate) the chance of this, and treat the known race as a skip rather than a failure if it still occurs for one of the newly created entries. Fixes dotnet#135283 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b2aac025-5dc1-420e-bfdf-19dd4b2f2f6f
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @dotnet/area-extensions-filesystem |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
SkipTestException requires [ConditionalFact]; under [Fact], the race still fails the test.
1 open finding
What changed in this PR
Mitigates a macOS FSEvents race affecting a PhysicalFileProvider rename test.
Changes:
- Adds a macOS-only delay before watcher registration.
- Dynamically skips when pre-rename tokens already fired.
| File | Description |
|---|---|
PhysicalFileProviderTests.cs |
Adds macOS race mitigation and skip handling. |
🧠 Review effort: Balanced
| if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX) && changed) | ||
| { | ||
| // Mark the test as inconclusive/skipped. | ||
| throw new SkipTestException($"Known macOS FSEvents race caused the {tokenDescription} token to change before the rename."); |
This was referenced Oct 7, 2026
Closed
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Mitigates the flaky
Microsoft.Extensions.FileProviders.PhysicalFileProviderTests.TokensFiredForNewDirectoryContentsOnRenametest on macOS.Root cause
The test creates a real directory/subdirectory/file structure on disk, then starts a real
FileSystemWatcher-backedPhysicalFilesWatcher/PhysicalFileProviderwatch, and asserts the corresponding change tokens have not fired yet (before the test's simulated rename). On macOS, the native watcher is backed byfseventsd, a separate daemon that the process subscribes to asynchronously. As documented by a .NET team member investigating a related issue (#30415 (comment)):So even though the watcher requests
kFSEventStreamEventIdSinceNow, there is no documented guarantee it excludes events for changes made shortly before the stream starts. This occasionally causes the "new directory/subdirectory/file should not have changed yet" assertions to fail.This is architecturally specific to macOS: Windows (
ReadDirectoryChangesW) and Linux (inotify) are handle/descriptor-based with no separate daemon replaying historical events.Why not fix it in
FileSystemWatcher/PhysicalFilesWatcherinsteadkFSEventStreamEventIdSinceNow,0.0flatency,NoDefer), so there isn't more headroom at that layer.PhysicalFilesWatcher-side fix (e.g. snapshotting expected state at watch-registration time and suppressing events that predate it) could close this deterministically, but it's a meaningfully more invasive change for a bug class that, in production, results in at most a spurious/redundant change notification — not a missed one or a correctness issue. That cost/benefit doesn't justify it here, so this PR only addresses the test.Fix
SkipTestException) instead of a hard failure, since it does not indicate a product regression.Fixes #135283
Note
This PR description and change were drafted with AI (GitHub Copilot) assistance.