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: set Creation, LastAccess and LastWrite time for files #875

Merged
merged 16 commits into from
Sep 16, 2022
Merged

feat: set Creation, LastAccess and LastWrite time for files #875

merged 16 commits into from
Sep 16, 2022

Conversation

vbreuss
Copy link
Member

@vbreuss vbreuss commented Aug 10, 2022 •
edited
Loading

Implements #872
Adjust CreationTime, LastAccessTime and LastWriteTime when interacting with files in the MockFileSystem.

Provides a means to mock the used DateTime by calling e.g.

var fixedTime = new DateTime(2022, 01, 01);
var fileSystem = new MockFileSystem().MockTime(() => fixedTime);
// All times will now be set to the fixedTime.

Implementes the following logic in MockFile:
All these cases are covered by unit tests in MockFileAdjustTimesTest

  • When creating files
    CreationTime, LastAccessTime and LastWriteTime are set to the current time
  • When changing files (WriteAllText, WriteAllBytes, AppendAllText, OpenWrite, Open)
    LastAccessTime and LastWriteTime are set to the current time
    CreationTime is left unchanged
  • When reading files (ReadAllText, ReadAllLines, ReadAllBytes, OpenRead)
    LastAccessTime is set to the current time
    CreationTime and LastWriteTime are left unchanged
  • When setting attributes or ACLs (SetAttributes, SetAccessControl)
    LastAccessTime is set to the current time
    CreationTime and LastWriteTime are left unchanged
  • When moving files (Move)
    LastAccessTime is set to the current time
    CreationTime and LastWriteTime are left unchanged
  • When copying files (Copy)
    CreationTime and LastAccessTime of the copied file in the destination directory is set to the current time
    LastWriteTime and all times of the source file are left unchanged

@vbreuss vbreuss changed the title feat: Set Creation, LastAccess and LastWrite time for files feat: #872 Set Creation, LastAccess and LastWrite time for files Aug 10, 2022
@vbreuss vbreuss force-pushed the feature/872-set-creation-last-access-and-last-write-times branch 3 times, most recently from 7ecbd63 to 30df259 Compare August 11, 2022 18:52
@vbreuss
Copy link
Member Author

vbreuss commented Aug 11, 2022 •
edited
Loading

Please note, that I changed the Interface IMockFileDataAccessor.
This interface looks like it is intended for internal use, but as it is public this could be considered a breaking change...

I am not sure what I should do:
I could increase the version or use default interface methods from C# 8

@vbreuss vbreuss changed the title feat: #872 Set Creation, LastAccess and LastWrite time for files feat: Set Creation, LastAccess and LastWrite time for files Aug 16, 2022
@vbreuss
Copy link
Member Author

vbreuss commented Aug 17, 2022

If this is an accepted approach, I could also try to have a look at #839, which deals with the times of directories. This would probably be quite similar...

@fgreinacher
Copy link
Contributor

Sorry @vbreuss, I'm a bit busy with other things, will try to have a look by the end of this week!

fgreinacher
fgreinacher reviewed Aug 20, 2022
View reviewed changes
Copy link
Contributor

@fgreinacher fgreinacher left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@vbreuss Looks very good so far, I made some suggestions.

I'm leaving for summer vacation soon, feel free to finish the review with @siprbaum and ping me for merge!

src/System.IO.Abstractions.TestingHelpers/MockFileSystem.cs Outdated Show resolved Hide resolved
src/System.IO.Abstractions.TestingHelpers/MockFileSystem.cs Show resolved Hide resolved
src/System.IO.Abstractions.TestingHelpers/MockFileSystem.cs Outdated Show resolved Hide resolved
vbtig and others added 3 commits August 26, 2022 12:14
@vbtig @vbreuss
Adjust times in MockFileSystem
e60ff0b 
Also apply time in MockFileStream
Mark MockFile_AfterSetAccessControl_ShouldUpdateLastAccessTime as Windows-Only

Fix name of unit tests to match the actual expected behavior
@vbreuss
Avoid null for dateTimeProvider when deserializing
1b567e8
@vbreuss
Use HasFlag instead of bitwise flag comparison
99ebdab
@vbreuss vbreuss force-pushed the feature/872-set-creation-last-access-and-last-write-times branch from 510c290 to 99ebdab Compare August 26, 2022 10:21
@vbreuss vbreuss requested a review from siprbaum August 26, 2022 10:25
@vbreuss
Copy link
Member Author

vbreuss commented Aug 31, 2022

@siprbaum : I merged the latest changes, so that this branch is up to date.
All comments are either fixed and resolved, or I left a comment explaining, why I implemented it this way. Could you please have a look again at this pull request?

@vbreuss vbreuss mentioned this pull request Aug 31, 2022
Closed
@vbreuss
Copy link
Member Author

vbreuss commented Sep 2, 2022

@siprbaum : Thanks for the review!
I implemented the changes and answered your findings.
Can you please have a look again, and resolve the comments, that you agree with?

Regarding the user detectable behaviour change:
As we are now setting the times correctly, the behaviour changes when previously in the tests you would (incorrectly) rely upon unchanged times for the files.
However I am still unsure, if I should increment the version when considering my previous comment in this pull request, regarding the change in the interface IMockFileDataAccessor.
What do you think?

@vbreuss vbreuss mentioned this pull request Sep 2, 2022
Closed
@vbreuss
Copy link
Member Author

vbreuss commented Sep 10, 2022

@fgreinacher : I merged the latest main branch and answered or fixed all review comments.
Can you please have a look?

vbreuss and others added 3 commits September 13, 2022 13:27
@vbreuss @fgreinacher
Update src/System.IO.Abstractions.TestingHelpers/MockFileSystem.cs
de88c79 
Co-authored-by: Florian Greinacher <florian@greinacher.de>
@vbreuss @fgreinacher
Update src/System.IO.Abstractions.TestingHelpers/TimeAdjustments.cs
477d8ca 
Co-authored-by: Florian Greinacher <florian@greinacher.de>
@vbtig
Adjust DateTimeKind.Unspecified
f0bc24e
@xtopaz xtopaz mentioned this pull request Sep 16, 2022
Closed
@fgreinacher fgreinacher changed the title feat: Set Creation, LastAccess and LastWrite time for files feat: set Creation, LastAccess and LastWrite time for files Sep 16, 2022
@mergify mergify bot merged commit a53bd9a into TestableIO:main Sep 16, 2022
This was referenced Sep 16, 2022
Merged
Open
Open
This was referenced Sep 17, 2022
Merged
Merged
@github-actions
Copy link

This is addressed in release v17.2.3.

@github-actions github-actions bot added the state: released Issues that are released label Nov 18, 2022
@github-actions
Copy link

This is addressed in release v17.2.3.

This was referenced Jan 7, 2023
Closed
Closed
@vbreuss vbreuss mentioned this pull request Aug 22, 2023
Closed
@vbreuss vbreuss deleted the feature/872-set-creation-last-access-and-last-write-times branch March 28, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siprbaum siprbaum siprbaum requested changes

@xtopaz xtopaz xtopaz left review comments

@fgreinacher fgreinacher fgreinacher approved these changes

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.
5 participants
@vbreuss @fgreinacher @xtopaz @siprbaum @vbtig