Conversation
`MockFileSystem` threw for `File.OpenHandle`, every `IRandomAccess` member and the `SafeFileHandle` overloads of `IFile`, and `FileStream.New(handle)` used `handle.ToString()` as a path. Code that uses handles could not be tested against the mock. - `File.OpenHandle` returns a `SafeFileHandle` with a synthetic value that the mock resolves to the `MockFileData` it opened. The handle refers to that file, not its path, so it keeps working after the file is deleted. It takes a file share like a stream does, and releases it (and applies `FileOptions.DeleteOnClose`) once the handle is closed or collected. - `RandomAccess` reads and writes that file at an offset. Writes replace the contents, so an open `MockFileStream` sees them. - The `SafeFileHandle` overloads of `IFile` read and write the attributes, times and Unix mode of the open file. - `FileStream.New(handle, ...)` wraps the open file like `FileStream` does: no new share, no `FileMode` applied again, and disposing it closes the handle. - Arguments are validated in the runtime's order, and the outcomes were compared scenario by scenario against the real file system. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1
- The closed-handle sweep runs under the lock on the files. Before, a thread that closed a handle could skip it while another thread was sweeping, and still hit its own share lock or find its DeleteOnClose file. - `AllPaths`, `AllFiles` and `AllDirectories` sweep too, so a closed DeleteOnClose file no longer shows up in `Directory.GetFiles` or stops a non-recursive `Directory.Delete`. - A stream on a handle is limited by the handle's own access, and throws `ObjectDisposedException` once the handle is closed, as `FileStream` does. `bufferSize` is validated, `isAsync` must match the handle, and the overloads without it take it from the handle. - `Create` and `Truncate` on an existing empty file update the write time. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Handle registry concurrency, sharing, option handling, and closed-handle flushing have unresolved correctness issues.
Review effort: Balanced
Findings: 4
Open (4)
What changed in this PR
Adds mock support for file handles, RandomAccess, handle-backed streams, and handle-based metadata operations.
Changes:
- Adds synthetic handle registration and lifecycle management.
- Implements random-access and handle-backed stream operations.
- Adds broad behavioral test coverage and supporting exceptions/resources.
| File | Description |
|---|---|
MockRandomAccessTests.cs |
Tests random-access and handle-backed streams. |
MockFileOpenHandleTests.cs |
Tests handle opening, lifecycle, and metadata. |
Resources.resx |
Adds handle-related error messages. |
MockSafeFileHandles.cs |
Implements synthetic handle registry. |
MockRandomAccess.cs |
Implements random-access operations. |
MockFileSystem.cs |
Integrates handle cleanup. |
MockFileStreamFactory.cs |
Creates streams from mock handles. |
MockFileStream.cs |
Adds handle-backed stream behavior. |
MockFile.cs |
Implements handle APIs and metadata overloads. |
CommonExceptions.cs |
Adds exception factories. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // A stream on a handle writes to the file the handle holds open, wherever its path now points. | ||
| if (adoptedHandle != null || mockFileDataAccessor.FileExists(path)) | ||
| { | ||
| var mockFileData = mockFileDataAccessor.GetFile(path); | ||
| var mockFileData = adoptedHandle != null ? fileData : mockFileDataAccessor.GetFile(path); |
There was a problem hiding this comment.
Fixed in 67e514f. Flush, Flush(bool) and FlushAsync now throw ObjectDisposedException once the handle is closed, and disposing the stream still succeeds. I checked both against the real file system: FileStream behaves the same way.
| /// <summary> | ||
| /// The handles handed out by <see cref="MockFile.OpenHandle"/>. Not serialized, since they stand for open files. | ||
| /// </summary> | ||
| internal MockSafeFileHandles SafeFileHandles => safeFileHandles ??= new MockSafeFileHandles(this); |
There was a problem hiding this comment.
Fixed in 67e514f. The registry is now published with LazyInitializer.EnsureInitialized, so threads that make their first call at the same time share one registry. There's a test that opens 16 handles in parallel on a fresh file system, 20 times over, and then resolves each of them.
| FileOptions.DeleteOnClose | | ||
| FileOptions.SequentialScan | | ||
| FileOptions.Encrypted | | ||
| (FileOptions)0x20000000; // NoBuffering |
There was a problem hiding this comment.
Right, the real File.OpenHandle accepts BackupOrRestore (I checked on .NET 10). I added it to the mask in 67e514f, with a test. The mask is the only place the check happens, so this also covers the second location you mentioned.
There was a problem hiding this comment.
A correction to my reply above: the runtime accepts BackupOrRestore only from .NET 9. .NET 8 still rejects it, which I found when running the same check in the sibling library's tests, since those also run against the real file system. As of d69dd99 the mock accepts it on net9.0 and later and rejects it on net8.0 and earlier, with a test for each.
| } | ||
|
|
||
| var shareGuid = Guid.NewGuid(); | ||
| mockFileDataAccessor.FileHandles.AddHandle(path, shareGuid, access, share); |
There was a problem hiding this comment.
True, but this was already the case before this PR: MockFileStream registers its share in FileHandles the same way, and File.Delete/File.Move only look at MockFileData.AllowedFileShare. So a stream opened without FileShare.Delete doesn't block a delete today either. Making delete and move respect open shares would change how existing streams behave too, so I'd rather do that in a separate PR, and I'm happy to if you want it.
- `Flush`, `Flush(bool)` and `FlushAsync` on a stream whose handle has been closed throw `ObjectDisposedException`, as `FileStream` does; disposing it still succeeds. - The registry is published with `LazyInitializer`, so threads that open their first handle at the same time share one registry. - `OpenHandle` accepts `BackupOrRestore` (0x02000000), which the runtime's options check allows. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1
.NET 8's options check rejects `FileOptions` 0x02000000; .NET 9 and later accept it. The mock now does the same on each target. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1

MockFileSystemthrows forFile.OpenHandleand for every member ofRandomAccesssince #1542, and theSafeFileHandleoverloads ofIFilethrowNotImplementedException.FileStream.New(handle)does not throw, but passeshandle.ToString()on as a path, so it fails withFileNotFoundException. Code that uses handles therefore cannot be tested against the mock. This adds that support, along the lines of what Testably.Abstractions shipped in 7.1.0, adapted to this mock's model.How it works
SafeFileHandleis sealed and wraps an operating system handle, so the mock cannot create one a real system call would accept.File.OpenHandlehands out a handle with a synthetic value instead (well above any real handle, unique across instances) and an internal registry onMockFileSystemremembers which file it stands for. The handle refers to theMockFileDatait opened rather than to its path, so it keeps working after the file is deleted, as it does on a real file system.File.OpenHandleopens likeMockFileStreamdoes: missing file or directory,CreateNewon an existing file, a directory at the path, a read-only file opened for writing. It takes a file share throughFileHandles, so it conflicts with streams exactly as a stream would. Arguments are validated in the runtime's order (FileStreamHelpers.ValidateArguments), so a call with several invalid arguments reports the same one.GetFile,FileExistsand theAll*enumerations release closed handles before answering: their file share goes, andFileOptions.DeleteOnCloseis applied if the same file is still at that path. The sweep runs under the lock on the files, so a thread never misses a handle it closed itself. It costs nothing while no handle is open. The registry holds handles weakly, so a handle that is dropped without being disposed is released once it has been collected, as a real one is by its finalizer.RandomAccessreads and writes that file at an offset. Writes replace the contents rather than changing them in place, so an openMockFileStreamsees them, and they update the file times.GetLengthworks through a write-only handle,FlushToDiskvalidates the handle, andSetLengththrough a read-only handle throwsIOExceptionon Unix (EINVAL) andUnauthorizedAccessExceptionon Windows.SafeFileHandleoverloads ofIFileread and write the attributes, times and Unix mode of the open file.FileStream.New(handle, ...)wraps the file the handle holds open, asFileStreamdoes. It does not open the path again, takes no share of its own, does not apply the handle'sFileModea second time, and closes the handle when it is disposed. It is limited by the handle's own access. It throwsObjectDisposedExceptiononce the handle is closed. It validatesbufferSize, and rejects anisAsyncthat differs from the handle's. Targets withoutFEATURE_RANDOM_ACCESSkeep the previous behaviour.FileStream.Newwith a handle that did not come from the mock now throwsArgumentException. Before, it opened a file named afterhandle.ToString().MockFileSystem, or from the real file system, throwsArgumentException. An accessor that is not aMockFileSystemstill gets theNotSupportedException(with an updated message).There is no public API change: the registry is internal and the members already existed.
Validated against the real file system
Besides the new unit tests, I ran 69 scenarios through both
new FileSystem()andnew MockFileSystem()and compared the outcome of each: the return value, or the exception type and parameter name. They cover argument validation, open modes, reads, writes, gather/scatter, lengths, access checks, closed and null handles, cancellation, file shares,DeleteOnClose(including through directory enumeration), handles on deleted files, streams built on handles and the handle overloads. I also ran a two-thread stress test of the handle lifetime (200,000 iterations, no misses). On macOS 65 scenarios match. The other 4 are listed below.Known limitations
File.Movein this mock copies theMockFileData, so a handle does not follow a file that is moved while it is open.Array.MaxLengththrowsIOException. For an offset that overflows, the runtime throwsArgumentOutOfRangeExceptioninstead.MockFileStreamrefreshes from the shared contents on reads, not on writes. A stream that writes after aRandomAccess.Writeto the same file can therefore overwrite it, as two streams on one file already can in this mock.SafeFileHandle.IsAsyncasks the operating system, so it isfalsefor a mock handle even withFileOptions.Asynchronous. The mock's ownisAsyncchecks use the options the handle was opened with.FileShare.Readblocks a writer, just as aMockFileStreamdoes, while on Unix onlyFileShare.Noneis enforced.Found along the way, not changed here
MockFile.Open(path, mode, access, share)ignoresshareand always opens withFileShare.Read, seeMockFile.OpenInternal. The new tests useFileStream.New, which passes the share on. Happy to fix that separately.Verification
In Release on net8.0, net9.0 and net10.0 (macOS):
TestingHelpers.Tests: all pass (77 new).🤖 Generated with Claude Code
https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1