FakeTimecop #14915
FakeTimecop #14915
Conversation
shared/test/police_box.rb
Outdated
| # | ||
| # Pass time in the test - using real or fake time, depending on how the | ||
| # time machine is configured. | ||
| # @param [Integer] seconds - How far to jump forward |
There was a problem hiding this comment.
This can (should) be a Float rather than an Integer? Note that you are passing in floats in your tests.
shared/test/police_box.rb
Outdated
| # | ||
| class PoliceBox | ||
| def initialize(in_real_time) | ||
| @time_circuits_on = !in_real_time |
There was a problem hiding this comment.
Can this (@time_circuits_on) be better named? Without looking, I have no idea whether this indicates we are using real time or fake time. Also, it smells that you are changing the name from in_real_time (negated) to @time_circuits_on.
Maybe just use_fake_time?
Aside, I'm not a fan of PoliceBox. The person next to you probably has no idea what this class does. Similarly for PoliceBox#in_box. Granted, I don't have better, so no change required here.
shared/test/police_box.rb
Outdated
| end | ||
|
|
||
| def pause_time | ||
| Timecop.freeze if @time_circuits_on |
There was a problem hiding this comment.
Note that I'd expect this to raise if time circuits are off, except we can't if we want tests to be "ambidextrous". Probably worth documenting this (also with resume_time).
shared/test/test_redis_table.rb
Outdated
| require 'helpers/null_pub_sub_api' | ||
| require 'helpers/redis_table' | ||
| require_relative './police_box' |
There was a problem hiding this comment.
Is the ./ necessary? Just curious.
For use in tests that sometimes want to time-travel (like most unit test runs) and sometimes don't (when verifying a behavior against real Redis.
|
Completely new approach, hopefully easier to grok for those unfamiliar with Doctor Who. PTAL. |
Deduplicates some test helpers from #14900 as requested by post-merge review. Lets tests swap in a fake
Timecopimplementation if they need real time to pass.Also cleans up some expected/actual mixup in assertions added in the last PR.