Merge Job into Call. CallTest = AsyncApiTest + SyncApiTest #764
Conversation
|
CallTest now has a lot of duplication.. we can either pick through it now, or later. |
| */ | ||
| public Response execute(Request request) throws IOException { | ||
| public Call newCall(Request request) { |
There was a problem hiding this comment.
I like this much better.
|
cc @jhump |
| } | ||
|
|
||
| try { | ||
| call.execute(callback); |
|
LGTM |
|
OK! updated to make blocking calls cancelable with tests. PTAL |
Merge Job into Call. CallTest = AsyncApiTest + SyncApiTest
| } | ||
| }); | ||
|
|
||
| Thread.sleep(100); // wait for it to go in flight. |
There was a problem hiding this comment.
To avoid test flakiness (and potentially make this faster since often it won't even take 100 millis to do), I prefer to use latches to coordinate the main test thread and the async action.
There was a problem hiding this comment.
I don't think we can access a good spot to place such a latch. Here, we are looking at ensuring the http engine is initialized to avoid a quick exit. Without making a state loop on visibility of a protected field, I think a 100ms is next best, and as you mentioned, significantly long enough to avoid being flakey.
There was a problem hiding this comment.
Ok. I was thinking it could be in the above submitted task, but I now see why that doesn't suffice.
One call to rule them all.