mperham / connection_pool

Generic connection pooling for Ruby
MIT License
1.63k stars 143 forks source link

Improve connection pool tests #49

Closed drbrain closed 10 years ago

drbrain commented 10 years ago

These methods in ConnectionPool were incompletely covered so here are some more tests.

Please double check @0e2f44f because it changes the implementation to not raise an exception when the user is doing things they shouldn't.

mperham commented 10 years ago

@drbrain I think the behavior should be an explicit raise rather than a return. WDYT?

drbrain commented 10 years ago

If you pool.checkout; pool.checkin; pool.checkin you get no exception without my change.

I picked return because the NoMethodError only occurs when you've never checked out a connection before and it was the smallest change to ConnectionPool I could make.

I don't mind if ConnectionPool raises an exception when you try to #checkin with nothing checked out, but that seemed too much for a testing-focused patch.

mperham commented 10 years ago

Fair enough, I'm very much a fan of blowing up loudly at any unexpected state, especially in multi-threaded infrastructure, so I'd be happy to get another PR which does this.

drbrain commented 10 years ago

I can create such a pull request, but it'll be easier after this one is merged so I can alter these tests.

mperham commented 10 years ago

Haha, oh did you want me to actually merge this??? Oops.