travisjeffery / timecop

A gem providing "time travel", "time freezing", and "time acceleration" capabilities, making it simple to test time-dependent code. It provides a unified method to mock Time.now, Date.today, and DateTime.now in a single call.
MIT License
3.36k stars 223 forks source link

Disable Process#clock_gettime mocking by default #426

Closed alexcwatt closed 2 months ago

alexcwatt commented 2 months ago

https://github.com/travisjeffery/timecop/issues/425

alexcwatt commented 2 months ago

@joshuacronemeyer This might be the simplest fix. I was also thinking it could be nice if the TimeStackItem new about whether or not we are mocking Process#clock_gettime so it could be an argument to Timecop.freeze and Timecop.travel in the future.

Just wanted to get something up quickly for feedback.

joshuacronemeyer commented 2 months ago

@alexcwatt code looks good. I think it's worth a mention in the README. Let me know if you'd like me to write something and add to this PR or if you got it. And thanks for the quick turnaround!

@olly and @nerdrew from the next release the process clock freezing will be opt-in. Feel free to weigh in on this.

olly commented 2 months ago

@joshuacronemeyer I'm happy with that proposal. I don't personally have any need to mock the Process clock, but to me it seems like needing to is a pretty niche use-case and so opting-in is appropriate.

alexcwatt commented 2 months ago

Glad it looks good! Feel free to take over the PR to update README. Thank you for flagging this issue; I definitely didn't anticipate it.

On Fri, Jun 14, 2024, at 12:54 PM, Oliver Legg wrote:

@joshuacronemeyer https://github.com/joshuacronemeyer I'm happy with that proposal. I don't personally have any need to mock the Process clock, but to me it seems like needing to is a pretty niche use-case and so opting-in is appropriate.

— Reply to this email directly, view it on GitHub https://github.com/travisjeffery/timecop/pull/426#issuecomment-2168421631, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYU6KAYFBNBRQ25WGUEXLZHMN5PAVCNFSM6AAAAABJJF3N7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRYGQZDCNRTGE. You are receiving this because you were mentioned.Message ID: @.***>

joshuacronemeyer commented 2 months ago

ok. i will shepherd this to a release this weekend. thanks all. @alexcwatt i actually wasn't surprised that something like this cropped up, even minor changes tend to bring folks out of the woodwork pretty fast.

joshuacronemeyer commented 2 months ago

closed this in favor of this https://github.com/travisjeffery/timecop/pull/427 because I don't know what i'm doing haha.

All i could figure out how to do was create my own branch and merge alex's branch into it and then make a PR from that.