Skip to content

Make test file watchers more resilient#131304

Open
rosebyte wants to merge 2 commits into
dotnet:mainfrom
rosebyte:fix/126726-onchange-relative-path-flaky
Open

Make test file watchers more resilient#131304
rosebyte wants to merge 2 commits into
dotnet:mainfrom
rosebyte:fix/126726-onchange-relative-path-flaky

Conversation

@rosebyte

Copy link
Copy Markdown
Member

Fixes #126726

@azure-pipelines

Copy link
Copy Markdown
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.

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

Improves resiliency of Microsoft.Extensions.Configuration functional tests that validate reload-on-change behavior, aiming to reduce flakiness in file watcher–driven tests (notably on Linux/containerized/stress environments).

Changes:

  • Replace one-shot file writes + WaitForChange polling with a new WaitForReload helper that re-applies file writes on each retry until the expected reload is observed.
  • Update the relative-path change-token test to use a TaskCompletionSource signal and dispose the ChangeToken.OnChange subscription.
  • Re-enable several previously [ActiveIssue]-skipped reload tests by making their wait logic more tolerant of missed/delayed notifications.

Copilot AI review requested due to automatic review settings July 24, 2026 14:22

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +1040 to +1044
var reloadedSignal = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
using IDisposable subscription = ChangeToken.OnChange(config.GetReloadToken, () => reloadedSignal.TrySetResult(true));

// A relative path is watched relative to the default file provider root (AppContext.BaseDirectory),
// so write to that exact location instead of the raw relative path; otherwise the test would depend
@svick

svick commented Jul 24, 2026

Copy link
Copy Markdown
Member

I'm not convinced writing to the file repeatedly is necessary here. PhysicalFilesWatcher shouldn't be losing any notifications and its tests are no longer flaky (AFAIK), even though they don't do anything similar. What I would do is to increase timeout from the ~20 s it's now to the 30 s used by PFW tests.

Other notes:

  • The change to explicitly use AppContext.BaseDirectory looks good (though this shouldn't be the cause of flakiness).
  • It could make sense to ensure different runs of tests don't interfere with each other by using unique file names (in cases where unique directory names through DisposableFileSystem don't make sense).
  • There are other tests that have [ActiveIssue] against the closed Using FileWatcher on non-windows has a very large impact on startup time #42036 that should probably be reactivated.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-filesystem
See info in area-owners.md if you want to be subscribed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test Failure: Microsoft.Extensions.Configuration.Test.ConfigurationTests.OnChangeGetFiredForRelativeLinuxPath

4 participants