Conversation
There was a problem hiding this comment.
Code Review
This pull request adds a new integration test class, ITResumableUploadCallable, to verify the resumable upload pipeline against Showcase under various scenarios, including single-chunk, multi-chunk, zero-byte uploads, timeouts, and cancellations. The review feedback recommends awaiting the termination of the ScheduledExecutorService during teardown to prevent potential thread leaks.
| if (executor != null) { | ||
| executor.shutdownNow(); | ||
| } |
There was a problem hiding this comment.
The ScheduledExecutorService is shut down using shutdownNow(), but its termination is not awaited. To prevent thread leaks and ensure resources are fully released before subsequent tests run, please await the executor's termination. Additionally, ensure that teardown or close methods perform explicit null checks before invoking methods on lazily initialized resources to prevent NullPointerException.
if (executor != null) {
executor.shutdownNow();
executor.awaitTermination(TestClientInitializer.AWAIT_TERMINATION_SECONDS, TimeUnit.SECONDS);
}References
- When using lazily initialized resources (such as ExecutorService), ensure that teardown or close methods perform explicit null checks before invoking methods on them to prevent NullPointerException.
04bf956 to
3eb43da
Compare
3eb43da to
7476681
Compare
018d72d to
6362e8a
Compare
4af6505 to
0566d84
Compare
26f77ac to
3d62765
Compare
3d62765 to
7e0b084
Compare
|
|
27e12bd to
4ef352a
Compare
b93f790 to
782059e
Compare
7e6eec1 to
aff0da6
Compare
e05f31d to
6186690
Compare
6186690 to
d586d2f
Compare
d586d2f to
49c4acd
Compare
49c4acd to
c86ce04
Compare
c86ce04 to
df4636e
Compare
blakeli0
left a comment
There was a problem hiding this comment.
Really like the idea of testing against showcase with callables directly. Never did it before but we should definitely try it more often in the future. Especially with the help of AI, the cost of writing these tests are almost none.
df4636e to
9c56fdd
Compare
9c56fdd to
9277706
Compare
9277706 to
259914b
Compare
259914b to
858debf
Compare
858debf to
c7dcc24
Compare
c7dcc24 to
2d16cc6
Compare
Add integration tests against local GAPIC Showcase server for ResumableUploadCallable, validating: 1. Single-chunk upload happy path. 2. Multi-chunk upload with multiple intermediate chunks and final chunk. 3. 0-byte upload and exact-chunk uploads with separate 0-byte finalize request. 4. Future cancellation and timeout handling. 5. Convenience futureCall(request, stream) overload. 6. Per-request ApiCallContext extra header overrides. 7. Stub-level and per-request ResumableUploadCallSettings overrides. 8. Stub-level and per-request global timeout watchdog triggering DeadlineExceededException.
2d16cc6 to
3911e8a
Compare



For illustration only - this PR will not be merged. This is basically a unit test for the Callable except against the showcase fake rather than mocks. The proper IT for resumable uploads will operate on the client library level rather than the Callable.