-
-
Notifications
You must be signed in to change notification settings - Fork 390
Add microseconds specs for File.mtime, .atime and .ctime #386
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,21 @@ | |
File.ctime(@file).should be_kind_of(Time) | ||
end | ||
|
||
platform_is :linux do | ||
it "Returns the change time for the named file (the time at which directory information about the file was changed, not the file itself) with microseconds." do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small typo, specs descriptions should start with a lowercase letter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy-pasted from previous What can I do with some fixes if that PR was merged? New PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I fixed it along with a few other cases in c5de600 |
||
file = tmp('ctime') | ||
3.times do | ||
touch file | ||
@ctime = File.ctime(file) | ||
break if @ctime.usec > 0 | ||
rm_r file | ||
sleep 0.001 | ||
end | ||
rm_r file | ||
@ctime.usec.should > 0 | ||
end | ||
end | ||
|
||
it "accepts an object that has a #to_path method" do | ||
File.ctime(mock_to_path(@file)) | ||
end | ||
|
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.
The cost of doing this repeatedly is pretty low, so this might as well be 10 or 100. But I support the idea in general; this was my stumbling block as well when I started to think about a spec.
The alternative would be to create a temporary file and forcibly modify these times, but I don't know if that's even possible for atime.
Note also that some Linux systems disable atime (e.g. via mount params) for better filesystem speed. I don't know if that will ever affect specs, though.
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.
There is already only 1/1000 chance per iteration to get 0 (assuming millisecond granularity), so I think it's fine.
The second iteration will be even more unlikely to get 0 as the work in between will not take exactly one second.
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.
Increased to 100.
That's possible for
atime
andmtime
via FileUtils#touch.This alternative was used.
This comment added to code, if you don't mind.
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.
Oh, it's already merged… What can I do?
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.
I think it's OK as it is (that's why I merged it), but if you want to change please feel free to open another Pull Request.
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.
OK, thank you!