-
Notifications
You must be signed in to change notification settings - Fork 158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ready: Make integration tests great again #657
Conversation
add more checking to RiakClusterFixtureTest:testOperationQueue()
…per converting null string to BinaryValue in BinIndexQuery.java
…ava-client into expand-coverage-api
Ready: Fix Null Pointer exception in #656.
@@ -371,9 +371,9 @@ public void await() throws InterruptedException | |||
} | |||
|
|||
@Override | |||
public void await(long timeout, TimeUnit unit) throws InterruptedException | |||
public boolean await(long timeout, TimeUnit unit) throws InterruptedException | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this cause backwards incompatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. If they don't assign/use the return value then the behavior is exactly the same. If they want to use it though, they can change their code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to http://stackoverflow.com/questions/3589946/retrofitting-void-methods-to-return-its-argument-to-facilitate-fluency-breaking/3589948#3589948, this is a source-compatible, but binary-incompatible change.
👍 Tests pass. Code looks good. Might want to keep in mind when it comes to release if there are breaking changes in this PR. |
@alexmoore what is this reminder about: Remainder fix ITestCoveragePlan: fetchAllDataByUsingCoverageContext -> get null pointer exception. ? |
@srgg Oh, I meant the remainder (the rest of) the commits were for that fix. |
@christophermancini Yeah, including a return type where one didn't before shouldn't break anything in this case, we're just passing on a little bit of extra information that has no effect on the computation. |
👍 LGTM |
Fix for #656 (CLIENTS-960).
c4d8383 Fixes ITestListKeysOperation.testSingleFetch -> fails on key comparison.
414748e Fixes ITestFullBucketRead -> All tests can throw sibling found exceptions on multi-node test setups.
c52e990 + 61f474e Fixes RiakClusterFixtureTest.testOperationQueue -> times out.
Remainder fix
ITestCoveragePlan: fetchAllDataByUsingCoverageContext -> get null pointer exception
.