feat: Add support for event-driven behaviours in MockFileSystem. #1313
Conversation
Added unit tests including various event handling scenarios, such as tracking file operations, validating timestamps, monitoring access patterns, simulating controlled failures, and profiling performance metrics.
| public enum FileOperation | ||
| { | ||
| /// <summary> | ||
| /// File or directory creation operation. |
There was a problem hiding this comment.
It would help to have a list of operations on the file system in the XML-Comment, that trigger the corresponding FileOperations.
| /// </summary> | ||
| [NonSerialized] | ||
| #endif | ||
| private readonly List<Subscription> subscriptions = []; |
There was a problem hiding this comment.
You could use a ConcurrentDictionary (or another thread-safe collection), to avoid using the LockObject...
| /// Tracks the current version of active subscriptions, allowing for the detection of changes | ||
| /// such as additions or removals of subscription handlers. | ||
| /// </summary> | ||
| private volatile int subscriptionVersion; |
There was a problem hiding this comment.
I don't understand why you need the subscriptionVersion, as you only ever increment it, but never read it. Or did I overlook something?
| } | ||
|
|
||
| var args = new FileSystemOperationEventArgs(path, operation, resourceType, phase); | ||
| Subscription[] currentSubscriptions; |
There was a problem hiding this comment.
For thread-safe collections you should be able to directly iterate over the subscriptions and can avoid introducing a temporary copy of the collection.
| /// <summary> | ||
| /// Gets a value indicating whether this subscription has been disposed. | ||
| /// </summary> | ||
| public bool IsDisposed |
There was a problem hiding this comment.
As you remove the Subscription from the collection, it seems unnecessary to provide a flag and check for it.
What are your reasons for doing so?
|
|
||
| using (fileSystem.Events.Subscribe(args => Interlocked.Increment(ref eventCount))) | ||
| { | ||
| const int operationCount = 100; |
There was a problem hiding this comment.
100 parallel tasks seems a bit excessive for a unit test. Wouldn't e.g. 10 be sufficient?
| { | ||
| var fileSystem = new MockFileSystem(new MockFileSystemOptions { EnableEvents = true }); | ||
| var eventCount = 0; | ||
| var exceptions = new List<Exception>(); |
There was a problem hiding this comment.
If you use a thread-safe collection, you can avoid locking on it (see line 588).
| public void Events_SubscriptionDisposal_DuringEventFiring_ShouldNotFail() | ||
| { | ||
| var fileSystem = new MockFileSystem(new MockFileSystemOptions { EnableEvents = true }); | ||
| var exceptions = new List<Exception>(); |
There was a problem hiding this comment.
If you use a thread-safe collection, you can avoid locking on it (see line 620).
| public void Events_ConcurrentSubscriptionAndUnsubscription_ShouldBeThreadSafe() | ||
| { | ||
| var fileSystem = new MockFileSystem(new MockFileSystemOptions { EnableEvents = true }); | ||
| var exceptions = new List<Exception>(); |
There was a problem hiding this comment.
If you use a thread-safe collection, you can avoid locking on it (see line 694 and 714).
| [Test] | ||
| public void Events_FileSystemOperationEventArgs_ShouldNotBeSerializable() | ||
| { |
There was a problem hiding this comment.
We don't need a test to verify, that a class is NOT serializable.
Added unit tests including various event handling scenarios, such as tracking file operations, validating timestamps, monitoring access patterns, simulating controlled failures, and profiling performance metrics.
Potential resolution for #805