Re-enable the WhenDiskIsFullTheErrorMessageContainsAllDetails test on Windows#125707
Re-enable the WhenDiskIsFullTheErrorMessageContainsAllDetails test on Windows#125707adamsitnik merged 3 commits intomainfrom
Conversation
Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR re-enables a Windows FileStream preallocation test and clarifies the rationale behind Windows preallocation failure handling in SafeFileHandle.
Changes:
- Removed a Windows-only
[ActiveIssue]suppression for the “disk full” preallocation error-message test. - Updated comments in Windows
SafeFileHandle.Preallocateto better describe whenSetFileInformationByHandlecan fail for oversized allocations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/FileStream/ctor_options.cs |
Removes Windows test suppression so the preallocation error-message test runs again on Windows. |
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs |
Refines explanatory comments for preallocation failure modes on Windows (no functional logic change in this diff). |
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
LGTM, but I am going to update the title to make it clear that it's not fixing the bug but just enabling a test.
There was a problem hiding this comment.
Pull request overview
Enables a previously skipped FileStream preallocation error-message test on Windows by updating Windows preallocation error handling to treat additional filesystem-limit failures as “disk full / file too large” conditions.
Changes:
- Unskips
WhenDiskIsFullTheErrorMessageContainsAllDetailson Windows. - Treats
ERROR_INVALID_PARAMETERfromSetFileInformationByHandle(FileAllocationInfo)as a “file too large” preallocation failure (delete created file + throwIOException), aligning behavior with the test’s expectations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/FileStream/ctor_options.cs | Removes the Windows ActiveIssue skip so the disk-full/file-too-large message test runs on Windows and Linux (still excluded on macOS). |
| src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs | Expands the set of Win32 errors that trigger the “preallocation failed” exception path to include ERROR_INVALID_PARAMETER, mapping it to “file too large”. |
You can also share your feedback on Copilot code review. Take the survey.
Description
On Windows,
SetFileInformationByHandlereturnsERROR_INVALID_PARAMETER(87) when the requested preallocation size exceeds the NTFS maximum file size for the volume's cluster size. ThePreallocatemethod only checked forERROR_DISK_FULLandERROR_FILE_TOO_LARGE, so oversized hints silently no-oped instead of throwingIOException.The fix (handling
ERROR_INVALID_PARAMETERinSafeFileHandle.Preallocate) was already applied; this PR re-enables the suppressed test and updates the stale comment:ctor_options.cs: Remove[ActiveIssue("…/92624", TestPlatforms.Windows)]fromWhenDiskIsFullTheErrorMessageContainsAllDetails— test now runs on Windows and validates that a 1 EiB preallocation throwsIOExceptioncontaining the path and size, and does not leave the file on disk.SafeFileHandle.Windows.cs: Update comment on the error-code guard to accurately describe thatERROR_INVALID_PARAMETERcovers NTFS rejecting allocation sizes that exceed the volume's per-cluster-size file-size limit.Changes
Original prompt
This section details on the original issue you should resolve
<issue_title>new FileStream with too-large preallocation hint does not throw on Windows</issue_title>
<issue_description>This test assumes creating a 1TB file will fail -- this is why the first run test fails here, because I have 1.5TB free. edit: turns out it fails anyway on Windows
Note the 2nd flavor of the test passes here -- perhaps because the first file wasn't deleted (?) but I would expect dispose to have happened between them and cleaned up the test directory. edit: it's because the finalizer hadn't run before dispose.
It should try to create a 100TB file or something, and we can meet back here in 2028. 😄 </issue_description>
Comments on the Issue (you are @copilot in this section)
@ Tagging subscribers to this area: @dotnet/area-system-io See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.Issue Details
This test assumes creating a 1TB file will fail -- this is why the first run test fails here, because I have 1.5TB free.
Note the 2nd flavor of the test passes here -- perhaps because the first file wasn't deleted (?) but I would expect dispose to have happened between them and cleaned up the test directory.
It should try to create a 100TB file or something, and we can meet back here in 2028. 😄
area-System.IO💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.