HDDS-16100. Parameterize TestChunkInputStream - #11117
Conversation
| .setStreamBufferMaxSize(MAX_FLUSH_SIZE) | ||
| .applyTo(conf); | ||
|
|
||
| enableShortCircuitRead(dir, conf); |
There was a problem hiding this comment.
Could we keep short-circuit setup opt-in for TestChunkInputStream and TestLocalChunkInputStream? Enabling it in the shared InputStreamTests setup also changes TestKeyInputStream and TestStreamBlockInputStream to use LocalChunkInputStream when the native library is available, so their existing gRPC coverage is lost.
There was a problem hiding this comment.
Thanks, updated.
| testCloseReleasesBuffers(bucket); | ||
| } | ||
|
|
||
| assertEquals(useShortCircuitRead, localChunkInputStreamLog.getOutput() |
There was a problem hiding this comment.
nit: Could we use AssertJ contains / doesNotContain here? This would make a failure show which log content selected the wrong client path.
There was a problem hiding this comment.
We would have to mirror the statements for the two cases, so I prefer assertEquals here.
|
@adoroszlai thanks for the patch! |
|
@ChenSammi please take a look |
szetszwo
left a comment
szetszwo
left a comment
There was a problem hiding this comment.
+1 the change looks good.
3 participants
What changes were proposed in this pull request?
TestLocalChunkInputStreamextendInpuStreamTestsinstead ofTestChunkInputStreamTestChunkInputStreamto also test with short-circuit read enabledtestFallbackToGrpcinTestLocalChunkInputStreamTestStreamBlockInputStreamhttps://issues.apache.org/jira/browse/HDDS-16100
How was this patch tested?
Without native lib:
With native lib:
https://github.com/adoroszlai/ozone/actions/runs/32855529592