Issue #510: use monotonic tick count #512
Conversation
| class << self | ||
| def now | ||
| if defined?(Process::CLOCK_MONOTONIC) | ||
| Process.clock_gettime(Process::CLOCK_MONOTONIC) |
There was a problem hiding this comment.
I can tell you that on every Ruby implementation that I'm aware of, Process::CLOCK_MONOTONIC will be defined.
If you insist on this, you should at least put the if statement outside the method definition rather than checking it every time.
There was a problem hiding this comment.
Since I can never see every Ruby implementation, I feel best going with the documentation.
I moved the if outside. That was tricky to get the tests to still pass, but I think I got it working, by using load to keep reloading the file, and especially one final time in a before(:all) block, to put it back like it started. 0ea7595
| @@ -132,15 +132,15 @@ def status_for_time(time) | |||
| it 'still does not process events because it is paused' do | |||
| # pretend we were woken up at 0.6 seconds since start | |||
| allow(config).to receive(:sleep). | |||
| with(anything) { |*_args| state[:time] += 2.0 } | |||
| with(anything) { |*_args| state[:monotonic_time] += 2.0 } | |||
There was a problem hiding this comment.
Is it necessary to rename this field?
There was a problem hiding this comment.
I found it helpful to keep clear that that was not the Time.now stub value but rather the MonotonicTime.now stub value.
It looked to me like it was just used in this test, so I didn't see a downside to clarifying the name.
Did I get that wrong?
There was a problem hiding this comment.
I'm not sure I was just concerned it was part of a public interface.
There was a problem hiding this comment.
My read is that it's strictly internal to this test. The only reason it's in a hash is so it can be mutated as the test runs, to mimic time advancing. (An @ variable in the test would work that way too.)
ColinDKelley
force-pushed
the
issue-510/use-monotonic-tick-count
branch
from
0ea7595
to
20f0585
Compare
Addressed issue #510:
Listen::MonotonicTimemodule withnowmethod. including tests.mtime) as it was, usingTime.now.timestampand_timestampmethods.