Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: adjust timestamps when accessing and modifying files #861

Merged
merged 3 commits into from
Jul 4, 2022
Merged

feat: adjust timestamps when accessing and modifying files #861

merged 3 commits into from
Jul 4, 2022

Conversation

xtopaz
Copy link
Contributor

@xtopaz xtopaz commented Jun 29, 2022

For tests with MockFiles we would like to have the same behaviour as on a real
file system regarding the timestamps on the copied file.

@xtopaz
Add adjusted timestamps on copied MockFiles
327f4a7 
This sets the timestamps for copied MockFile instances so that the CreationTime
and LastAccessTime get the timestamp of the copy operation and the LastWriteTime
is inherited from the source as before.
@xtopaz xtopaz changed the title Add adjusted timestamps on copied MockFiles feat: Add adjusted timestamps on copied MockFiles Jun 29, 2022
@fgreinacher fgreinacher mentioned this pull request Jun 30, 2022
Closed
@fgreinacher
Copy link
Contributor

This looks very similar to #860

@xtopaz
Fix timestamps in MockFileStream
920527b 
This fixes setting of the appropriate timestamps in the MockFileData
instances, when accessing them via the MockFileStream.

This contains the implementation intended in TestableIO#860.
@xtopaz
Copy link
Contributor Author

xtopaz commented Jul 3, 2022

Unfortunately after my last commit 920527b an issue cropped up in the static code analysis which claims that "'existingContents' is null on at least one execution path." in src/System.IO.Abstractions.TestingHelpers/MockFileStream.cs#L44.
@fgreinacher, since this is obviously a false positive, could you please set it to ignore?

siprbaum
siprbaum reviewed Jul 3, 2022
View reviewed changes
Copy link
Contributor

@siprbaum siprbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments, mainly clarifications needed.

src/System.IO.Abstractions.TestingHelpers/MockFileStream.cs Outdated Show resolved Hide resolved
src/System.IO.Abstractions.TestingHelpers/MockFileStream.cs Show resolved Hide resolved
@xtopaz
Fix timestamp update when file is opened
d5f5d1e 
This removes the premature update of the LastAccessTime timestamp when a
MockFileStream is opened.
@fgreinacher
Copy link
Contributor

Unfortunately after my last commit 920527b an issue cropped up in the static code analysis which claims that "'existingContents' is null on at least one execution path." in src/System.IO.Abstractions.TestingHelpers/MockFileStream.cs#L44. @fgreinacher, since this is obviously a false positive, could you please set it to ignore?

Sure, done! Thanks for having a look at this!

@fgreinacher fgreinacher changed the title feat: Add adjusted timestamps on copied MockFiles feat: adjust timestamps when modifying files Jul 4, 2022
@fgreinacher fgreinacher changed the title feat: adjust timestamps when modifying files feat: adjust timestamps when accessing and modifying files Jul 4, 2022
@fgreinacher
Copy link
Contributor

Thanks a lot for your contribution @xtopaz.

Also thanks @siprbaum for helping with the review!

@fgreinacher fgreinacher merged commit 25e44d7 into TestableIO:main Jul 4, 2022
@renovate renovate bot mentioned this pull request Jul 4, 2022
Merged
1 task
This was referenced Jul 5, 2022
Merged
Merged
This was referenced Jul 5, 2022
Merged
Merged
@github-actions
Copy link

This is addressed in release v17.0.21.

@github-actions github-actions bot added the state: released Issues that are released label Jul 15, 2022
@fgreinacher fgreinacher mentioned this pull request Aug 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fgreinacher fgreinacher fgreinacher left review comments

@siprbaum siprbaum siprbaum left review comments

Assignees
No one assigned
Labels
state: released Issues that are released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xtopaz @fgreinacher @siprbaum