Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix flaky FileWatcherTests.DeleteSubfolder test - #54801

#54801
Closed
MichaelSimons wants to merge 4 commits into
maindotnet/sdk:mainfrom
michaelsimons/fix-flaky-delete-subfolder-testdotnet/sdk:michaelsimons/fix-flaky-delete-subfolder-testCopy head branch name to clipboard
Closed

Fix flaky FileWatcherTests.DeleteSubfolder test#54801
MichaelSimons wants to merge 4 commits into
maindotnet/sdk:mainfrom
michaelsimons/fix-flaky-delete-subfolder-testdotnet/sdk:michaelsimons/fix-flaky-delete-subfolder-testCopy head branch name to clipboard

Conversation

@MichaelSimons

Copy link
Copy Markdown
Member

Summary

Fixes #49233

The DeleteSubfolder test was intermittently failing because the test harness (TestOperation) used a count-based completion condition that didn't account for extra filesystem events emitted by the OS.

Impact

Per the Known Build Error tracker on the issue, this flaky test is hitting frequently:

24-Hour Hit Count 7-Day Hit Count 1-Month Count
5 26 40

These failures cause unnecessary CI noise and re-runs across PRs in the repo.

Root Cause

TestOperation signaled completion when it received any N unique events (where N = expectedChanges.Length). On Linux with polling, Directory.Delete(subdir, recursive: true) can emit additional events — such as (subdir, Update) or (subdir, Delete) — beyond the expected file-level delete events. These extra events filled the count quota before all expected events arrived, causing the final SequenceEqual assertion to fail with a mismatch like:

Expected: foo1/Delete, foo2/Delete, foo3/Delete
Actual:   subdir/Update, subdir/Delete, foo1/Delete, foo2/Delete

Fix

  1. Completion condition: Changed from filesChanged.Count == expectedChanges.Length to expectedSet.IsSubsetOf(filesChanged) — the test now waits until all expected events have been observed, regardless of how many extra events the OS generates.

  2. Assertion: Changed from strict sequence equality to verifying all expected changes are present in the collected set. Extra OS-level events are tolerated since they don't indicate incorrect behavior.

This makes the test resilient to platform-specific filesystem event noise while still verifying the core watcher behavior.

The TestOperation method previously signaled completion when it received
any N unique events (where N = expectedChanges.Length). On Linux with
polling, deleting a subdirectory can emit extra events like (subdir, Update)
or (subdir, Delete) in addition to the expected file-level delete events.
These spurious events filled the count quota before all expected events
arrived, causing the assertion to fail.

Fix: Change completion condition to wait until all expected events are
present in the collected set (subset check), and change the assertion to
verify all expected events were observed while tolerating extra OS events.

Fixes #49233

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 16:34
@MichaelSimons
MichaelSimons requested review from a team and tmat as code owners June 16, 2026 16:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Reduces flakiness in FileWatcherTests.DeleteSubfolder by changing the test harness to wait until all expected filesystem events have been observed, rather than completing after receiving an arbitrary number of unique events (which can be skewed by extra OS-generated noise).

Changes:

  • Track expected changes as a set and complete the operation only when the expected set is a subset of observed changes.
  • Update test verification to assert that all expected changes were observed (tolerating additional events) and improve diagnostic output.

Comment thread test/dotnet-watch.Tests/FileWatcher/FileWatcherTests.cs
Comment thread test/dotnet-watch.Tests/FileWatcher/FileWatcherTests.cs
MichaelSimons and others added 2 commits June 16, 2026 11:56
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
await (Debugger.IsAttached ? task : task.TimeoutAfter(DefaultTimeout));

AssertEx.SequenceEqual(expectedChanges, filesChanged.OrderBy(x => x.Path));
// Verify all expected changes were observed (extra events from the OS are acceptable)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, extra events can cause different watch behavior, so they are generally not acceptable.

Are the events non-deterministic on Linux?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can modify the logic so that a test can list specific optional events that may occur.

@MichaelSimons
MichaelSimons deleted the michaelsimons/fix-flaky-delete-subfolder-test branch June 30, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Microsoft.DotNet.Watch.UnitTests.FileWatcherTests.DeleteSubfolder has flaky test assertion

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.