rspec-wait icon indicating copy to clipboard operation
rspec-wait copied to clipboard

Do not use the same matcher instance on each wait iteration

Open martosaur opened this issue 7 years ago • 3 comments

Fixes: #20

Some notes on the rootcause and solution:

contain_exactly is one of those matchers that stores some intermediate computations internally. And hence we reuse this matcher object throughout the whole handle_matcher loop, it ignores the actual results and uses it's internal state computed on the first iteration.

The solution proposed is simply to clone initial matcher every iteration.

Please note that I have no idea is the solution is optimal, so any opinions welcome.

martosaur avatar Jun 04 '18 16:06 martosaur

I have a feeling this is awesome and needed, but:

  1. I don't really understand what's happening in the tests. Can they be simplified or at least commented?
  2. I'm also confused as to why cloning the matcher doesn't copy the matcher's internal state as well? Is it because we only ever use cloned matchers, so they're cloned from an empty state?
  3. How did you decide between clone and dup?

Thank you for this contribution! 👏

laserlemon avatar Jun 04 '18 16:06 laserlemon

Thank you very much for the immediate response.

  1. Yes they can! I completely forgot there is Enumerator – a better way to have a simple object returning some predefined values on each call. I've updated the tests code.
  2. Yes, exactly like this, we keep initial matcher as a sacred template and only use clones to do actual assertion.
  3. To be honest, it just felt like a safer choice, because clone seems a little more powerful and rspec-expectation class system looks somewhat complicated. Anyways dup works just as well. I can change it if you feel like this is a better choice.

martosaur avatar Jun 06 '18 08:06 martosaur

@laserlemon a kind reminder 😃

martosaur avatar Jul 11 '18 08:07 martosaur