-
Notifications
You must be signed in to change notification settings - Fork 254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: improve cache invalidation in MockDirectoryInfo #862
fix: improve cache invalidation in MockDirectoryInfo #862
Conversation
I'm not going to add the conventional commit prefix as I prefer the repository maintainers to squash-merge my PR and apply the appropriate convention. This is not out of laziness, but rather out of respect: The maintainers will be more familiar with how they wish to deliver my changes. |
tests/System.IO.Abstractions.TestingHelpers.Tests/MockDirectoryInfoTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @rcdailey! We have a failing test on non-Windows that needs adaptation, otherwise good to go!
When invoking the `Create()` and `Delete()` methods on `MockDirectoryInfo`, the `Exists` property should intrinsically invoke `Refresh()` the next time it is called. This is necessary to prevent returning stale state about the directory after those operations. Additional test cases have been added for `MoveTo()` as well, to ensure it also not returning stale state. For context, this change was motivated by PR TestableIO#828 where similar issues were solved in `MockFileInfo`.
ae2e5b3
to
18d37a8
Compare
@fgreinacher Let me know if there's anything else I need to do. Thank you once again for taking the time to review my PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work on this!
When invoking the `Create()` and `Delete()` methods on `MockDirectoryInfo`, the `Exists` property should intrinsically invoke `Refresh()` the next time it is called. This is necessary to prevent returning stale state about the directory after those operations. Additional test cases have been added for `MoveTo()` as well, to ensure it also not returning stale state. For context, this change was motivated by PR #828 where similar issues were solved in `MockFileInfo`. Co-authored-by: Florian Greinacher <florian@greinacher.de>
When invoking the
Create()
andDelete()
methods onMockDirectoryInfo
, theExists
property should intrinsically invokeRefresh()
the next time it is called. This is necessary to preventreturning stale state about the directory after those operations.
Additional test cases have been added for
MoveTo()
as well, to ensureit also not returning stale state.
For context, this change was motivated by PR #828 where similar issues
were solved in
MockFileInfo
.