Fix flaky SucceedsWithWarningOnLockedFile test#54040
Draft
MichaelSimons wants to merge 1 commit intomainfrom
Draft
Fix flaky SucceedsWithWarningOnLockedFile test#54040MichaelSimons wants to merge 1 commit intomainfrom
MichaelSimons wants to merge 1 commit intomainfrom
Conversation
The test was intermittently failing on Windows CI (windows.amd64.vs2026.pre.scout.open) with Warnings.Count == 0, meaning the expected IOException from the file lock was never thrown and no warning was logged. Root cause: The test created a locked file using a relative path (SucceedsWithWarningOnLockedFile.dll) in the process working directory. The task resolves file paths via TaskEnvironment.GetAbsolutePath(), which combines the path with the ProjectDirectory. When xUnit runs test collections in parallel (4+ threads), the working directory can differ from what the TaskEnvironment captured, causing File.Exists() to check the wrong location and skip the file entirely — resulting in zero warnings. Fix: - Use TestAssetsManager.CreateTestDirectory() to create an isolated temp directory with an absolute path, eliminating any dependency on the process working directory. - Add a lock-verification probe that attempts to open the locked file before running the task. If the probe succeeds (lock not enforced), the test fails with a clear diagnostic rather than silently producing a misleading Warnings.Count == 0 failure. - Extend the test class from SdkTest to use standard test infrastructure. Fixes #53870 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
@MichaelSimons the TaskEnvironment should have an API that is intended to be used with parallel execution, like in test runners: https://github.com/dotnet/msbuild/blob/main/src/Framework/TaskEnvironment.cs#L49 Could you see if this API provides a pathway towards solving this issue as well? It's what the MSBuild team intended to be the user-facing solution for test runners. |
This was referenced Apr 22, 2026
Open
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
Fixes the intermittent
SucceedsWithWarningOnLockedFiletest failure tracked by #53870.This test has been failing ~12 times per month on Windows CI (
windows.amd64.vs2026.pre.scout.open), with the assertionWarnings.Count == 0— meaning the expectedIOExceptionfrom the file lock was never thrown and no warning was logged by the task.Root Cause Analysis
The test created a locked file using a relative path (
SucceedsWithWarningOnLockedFile.dll) resolved against the process working directory. TheGetDependsOnNETStandardtask resolves reference paths viaTaskEnvironment.GetAbsolutePath(), which combines the path withProjectDirectory(captured fromDirectory.GetCurrentDirectory()at construction time).When xUnit runs test collections in parallel (4+ threads sharing the same process), the working directory state can become unreliable. If the absolute path resolved by the task differs from where the test actually created the locked file,
File.Exists()returnsfalseand the task skips the file entirely — producing zero warnings instead of the expectedIOException-triggered warning.Additionally, on Windows CI machines, antimalware mini-filter drivers (e.g., Windows Defender) can interfere with file locking on newly created
.dllfiles, potentially allowing a second file handle to open despiteFileShare.None.Changes
Use absolute paths in an isolated temp directory: Replace the relative-path file creation with
TestAssetsManager.CreateTestDirectory(), which creates a unique temp directory with an absolute path. This eliminates any dependency on the process working directory.Add lock-verification probe: Before running the task, a probe attempts to open the locked file with
FileAccess.Read. If the probe succeeds (meaning the exclusive lock is not being enforced), the test fails immediately with a clear diagnostic message rather than proceeding to a misleadingWarnings.Count == 0failure.Extend from SdkTest: The test class now extends
SdkTestto use standard test infrastructure (TestAssetsManager).Fixes #53870