search

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

Add `Process.clock_gettime` support #419

Closed alexcwatt closed 3 months ago

alexcwatt commented 4 months ago

I am interested in being able to use Timecop with Process.clock_gettime. I saw https://github.com/travisjeffery/timecop/issues/220 (the request for this) and https://github.com/travisjeffery/timecop/pull/258 (the PR that implemented it).

This code is nearly the same as https://github.com/travisjeffery/timecop/pull/258; I have only made minor changes.

alexcwatt commented 4 months ago

@lavoiesl more great feedback - thank you!

joshuacronemeyer commented 3 months ago

Thanks @lavoiesl and @alexcwatt for making/workshopping this PR (and all the folks over the years that moved the original issues and PRs forward). I am going to have to play around with this a little bit, but don't see why this won't get merged in.

One thought I had was that it would be nice if this new was mentioned in the README. Could you take a crack at that?

alexcwatt commented 3 months ago

@joshuacronemeyer I made a small change to the README. Let me know what you think. I'm happy to elaborate more but it seems the most important thing is to mention that Process.clock_gettime is now supported.

Also let me know if you need me to change anything else, like squashing to a single commit; I kept the commit history originally so reviewers could easily see what I was changing between iterations.

alexcwatt commented 3 months ago

@joshuacronemeyer Thank you for the detailed review! I obviously missed some things. I think this version addresses your comments. Please let me know what you think when you have another look. I meant to address this feedback sooner but couldn't get to it last weekend.

I've preserved commit history so you can see what changed, but when this is ready to be merged, we can squash everything.

nerdrew commented 2 months ago

Can we make this stub opt-in? It seems to break various low-level use cases when it's enabled. e.g. https://github.com/travisjeffery/timecop/issues/425 and the fluentd PR above...