Closed andrewcox closed 11 years ago
@electrum: I did also try forcing guava 14 and checking out old version of ThriftClientManager to run against the test I added to verify they do fail before the fix. Thanks for pointing this out!
Looks good, but @dain should take a look
Other than the one comment on the test assertion, looks good
Added checks to ensure the proper cause of the ExecutionException, rebased and pushed
Another change here:
Before we saw the old guava 13 code was masking the test failure even w/o my fix, I thought it was because async connect was actually happening too fast (instantly), so the future callback was happening on the same thread (which would result in the future getting thrown and caught by the test instead of swallowed on another thread). So I added a default 10ms delay in channel creation to those tests, so that the connection future will callback on the netty thread as per what will almost always happen when you are connecting to a remote server.