Remove sleeps from formatter specs

[soulcutter]

I'm not sure stubbing is the best way to fix this, let me know if you have a better idea.

Also something is failing for me in command_line/order_spec.rb which can't possibly be related, can it?

[JonRowe]

I'm not sure how I feel about the stubs /cc @xaviershay

[xaviershay]

I'd like to investigate removing the explicit dependency on time before merging this, there might be a nicer way to do it.

[xaviershay]

(i.e. have a "clock" configuration / dependency inject or something)

[myronmarston]

We could also use something like timecop and do Time.travel(n) from within the examples to simulate the passage of time.

[JonRowe]

I like @myronmarston's idea here, simulate a long spec by shifting time forward within the spec.

[soulcutter]

RSpec uses RSpec::Core::Time.now… so it's trickier than just dropping in Timecop. I did try using Timecop, and Timecop itself generates some warnings which breaks tests.

[myronmarston]

Oh right...we use RSpec::Core::Time in order to not let timecop (and tools like it) affect the reports of how long specs took.

We could do similar stubbing on RSpec::Core::Time. @xaviershay's injection idea might work better, though.

[soulcutter]

The injection works… ok. TBH I'm not sure how much less brittle it is since it depends on a specific use pattern of the clock.

[myronmarston]

It'd be better to use class_double(RSpec::Core::Time). class_double to give the guarantees of verified doubles, and the constant (rather than string name) since the class is already loaded and using the constant will give a clear error if the class is renamed.

[xaviershay]

FWIW I'm generally not a fan of timecop (except where unavoidable). I've never regretted decoupling my code from time.

[soulcutter]

Uses class_double now, and simplified the mocking to only take place inside the example. Also squashed the commits together. I think this is as good as it's going to get.

[soulcutter]

It's interesting that this is apparently working since I missed the .now

[myronmarston]

Hmm, how is that working? I tried this in a debugger session:

(rdb:1) p RSpec::Core::Time + 0.5
*** NoMethodError Exception: undefined method `+' for RSpec::Core::Time:Class

[myronmarston]

You could just use Time.now + 0.5, right?

[myronmarston]

Also, why not use a much larger value such as 500? A half second isn't very long and if someone set a debugger breakpoint to look at what's going on they could cause the faster example to take longer than the 0.5 seconds of the slower example to fail. Since no actual time passes you might as well use more exaggerated values.

[soulcutter]

I used RSpec::Core::Time to be internally consistent with how the time gets calculated inside of Example. In the unlikely case of us changing how RSpec::Core::Time works, this spec might get re-examined because it uses it, which is probably a good thing.

I chose 0.5 so that I wouldn't have to change the specs expecting output like this:

expect(output.string).to match(/0(\.\d+)? seconds/)

[myronmarston]

Works for me.

[myronmarston]

LGTM. The rbx build just flaked out so I kicked it. Merge when green.

[JonRowe]

I've rebased my PR on top of this, thanks @soulcutter!