Initialize redis on connection pool #32
Conversation
There was a problem hiding this comment.
Great and expedient fix, thanks 🙏 I wonder though - do we need to duplicate all of our tests just to test the setup with two variants of the Redis argument? Can we be a bit terser and only test for that specific setup detail in a couple additional tests instead?
|
I only added one test atm. The rest of the throttling tests I simply wrapped in a |
|
Ah I see now! But should we create that much diff just for adding one context? Contexts imply that there will be multiple where different cases are covered, and here it is not the case. The reason I am being so picky about it is that Prorate is somewhat complex to read/understand. Any extra stuff we add in tests contributes to that complexity. |
spec/throttle_spec.rb
Outdated
|
|
||
| context 'with a connection pool' do | ||
| it 'throttles and raises an exception' do | ||
| pool = ConnectionPool.new(size: 3, timeout: 3) { Redis.new } |
There was a problem hiding this comment.
you can use the r you set on line 25 here.
|
👍 |
Fixes https://travis-ci.com/github/WeTransfer/spaceship/builds/176169747