rspec-expectations
rspec-expectations copied to clipboard
rspec
Cache match results for compound failures
fixes #1331
When a compound matcher fails, the failure message checks both sides of the match to determine how to display the failure match. When multiple compound matchers are chained, that means the match for each side of the compound matcher may be called repeatedly for exponential growth.
To address this behavior, we can cache the match result for each side of the compound operator each time a match is executed. We need to reset the cache each time the match is executed because the matcher may be re-used for a different expectation.
Added RSpec context timing before the patch:
RSpec::Matchers::BuiltIn::Compound
when chaining many matchers together
with long chains of compound matchers
with a failing first matcher
generates a failure description quickly
with all failing matchers
generates a failure description quickly with or
generates a failure description quickly with and
with a failing last matcher
generates a failure description quickly
Finished in 2 minutes 47.7 seconds (files took 0.43342 seconds to load)
4 examples, 0 failures
Added RSpec context timing after the patch:
RSpec::Matchers::BuiltIn::Compound
when chaining many matchers together
with long chains of compound matchers
with a failing last matcher
generates a failure description quickly
with all failing matchers
generates a failure description quickly with and
generates a failure description quickly with or
with a failing first matcher
generates a failure description quickly
Finished in 0.05168 seconds (files took 0.42805 seconds to load)
4 examples, 0 failures
@pirj Style-wise, I was wondering about where the best place to reset state might be. I put the reset logic directly in the match method. I noticed that for a different issue, you add a blank reset method to match?. Should we incorporate that idea into this PR? or just concentrate on this PR.
All your suggestions seem good to me, so I'll go ahead and make them. Thanks for the look.
Looks great! Does it make sense add
timeout_if_not_debuggingin the same vein you've used in rspec/rspec-expectations#1333?
Where do helper functions go when they're used in multiple spec files? It seems that spec/support is all shared examples and matchers. There are a few helper functions in spec_helper.rb. That seems like where timeout_if_not_debugging should go, unless you have a different suggestion?
few helper functions in spec_helper.rb. That seems like where timeout_if_not_debugging should go
A perfect place for it 👍
This commit updates the timeout limit from 0.1s to 0.2s. Hopefully that's enough headroom for ruby 1.8.7.
