NoRedInk/rspec-retry

merge with rspec-repeat?

Opened this issue · 9 comments

@dwbutler @rstacruz

thoughts on merging with rspec-repeat? I think rspec-retry has better SEO / name recognition but rspec-repeat has a cleaner codebase. From that perspective: Would be happy to either

  • bring on @rstacruz as a maintainer and release rspec-repeat as the next big-version-number version of rspec-retry.
    or
  • deprecate rspec-retry and point people towards rspec-repeat in the readme.

My requirements would be

  • ensure feature parity (which might be done)
  • ensure adequate test coverage
  • ensure open issues are migrated to the appropriate repository

thoughts, you two?

i'm all for it, but have you also considered:

a) let them exist side by side?
b) deprecate rspec-repeat instead?
c) making rpsec-repeat a dependency of rspec-retry?

I think they're so similar that maybe deprecating one is better than keeping them side by side. I think your code base is better than ours (minus our much more comprehensive test suite). We could certainly use rspec-repeat as a dependency, however I think once we pull out the rspec-retry guts and replace them with rspec-retry, most of what we'd have is a test suite.

I'm happy with rspec-retry absorbing rspec-repeat in whatever way that probably means, even if it's replacing all of lib/.

I just request that you make repeating all of type: features still possible somehow — it's the primary use case of my team (and with people who's written to me about rspec-repeat) :)

@dwbutler thoughts on this? Then will move forward / give you access

@michaelglass I like @rstacruz's implementation of rspec-repeat. I would be in favor of giving him contributor access so he can merge in his changes in favor of mine.

The only difference is that rspec-repeat is opt-in - you have to set up an around hook in order to get any retry behavior. Whereas, rspec-retry automatically sets up the around hook and relies on configuration. We'll have to find a way to resolve the behavior difference, or document it.

The right approach might be to do a major version bump, move ahead with breaking changes, and document them.

@dwbutler thanks so much! @rstacruz I'm going to give you contributor access who's going to do the work WRT pulling rescue-retry guts?

would like to stay syntax-compatible (still with a major version bump) but I think we can have rspec-repeat do the heavy lifting

How about having both available — the repeat method, and the retry: 3 metadata?

That would be approximately:

config.around :each, :retry do |ex|
  RSpec::Repeat.repeat ex.metadata[:retry], ex.metadata
end
it 'eventually works', retry: 3, verbose: true do
  expect(rand(2)).to eq 0
end

also: gotta decide on method names at some point (retry as a method name is reserved)

yeah exactly