Repository navigation
fix: honour combined FileShare.ReadWrite flags when sharing - #1091
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation and tests cover the intended combined sharing behavior.
Review effort: Lite
Findings: None
What changed in this PR
Fixes combined FileShare.ReadWrite handling when used with FileShare.Delete, with accompanying tests.
Changes:
- Corrects flag evaluation for combined sharing modes.
- Adds coverage for accepted and rejected sharing combinations.
| File | Summary |
|---|---|
Tests/Testably.Abstractions.Tests/FileSystem/File/ShareReadWriteWithDeleteTests.cs |
Adds sharing behavior tests. |
Source/Testably.Abstractions.Testing/Storage/FileHandle.cs |
Corrects combined FileShare handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
`FileHandle.CheckAccessWithShare` tests the `Read` and `Write` cases with `HasFlag`, but compared the `ReadWrite` case for equality. A file opened with `FileShare.ReadWrite | FileShare.Delete` therefore did not share read/write access, although it shares strictly more than `FileShare.ReadWrite` does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
73393f6 to
f55229b
Compare
|
No code change beyond the comment removal — correcting the verification note in the description. It quoted Debug numbers, and Debug leaves the real file system out of the test matrix: |
|
This is addressed in release v7.1.0. |
Fixes #1090.
Sibling of #1084 / #1085, one method along.
FileHandle.CheckAccessWithSharetests theReadandWritecases withHasFlagand theReadWritecase with equality, soFileShare.ReadWrite | FileShare.Delete— which shares strictly more thanFileShare.ReadWrite— did not grant read/write sharing.One line, plus tests that two
FileAccess.ReadWriteopeners succeed for bothFileShare.ReadWriteandFileShare.ReadWrite | FileShare.Delete, and that shares omittingWritestill throw on Windows.I found this while checking whether #1085 let some skipped tests in #1082 run again. It did for deletion; this was the second reason they were skipped.
Branches from
mainand is independent of #1082 / #1083.Run in Release, which is what puts the real file system into the test matrix:
Testably.Abstractions.Tests12130 on net8.0 and 12639 on net10.0, andTesting.Tests1308. All pass.MemoryMappedFiles.Testsfails onmainitself in the intermediate state between the10.4.0-pre.1release and the version bump in #1082, which is whatBuildScope.CoreOnlyskips — nothing to do with this change.Happy to rework it however you prefer.
🤖 Generated with Claude Code