Skip to content
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

Create test to ensure HttpLoggingInterceptor doesn't consume bytes #5251

Closed
manusa opened this issue Jun 15, 2023 · 4 comments · Fixed by #5532
Closed

Create test to ensure HttpLoggingInterceptor doesn't consume bytes #5251

manusa opened this issue Jun 15, 2023 · 4 comments · Fixed by #5532

Comments

@manusa
Copy link
Member

manusa commented Jun 15, 2023

Description

Relates to #5250

In #5146 we removed by mistake the copy instruction for the consumed ByteBuffer.
This has basically reintroduced the problems (eclipse-jkube/jkube#2000, eclipse-jkube/jkube#1950) that were originally addressed by our HttpLoggingInterceptor feature.

We need to add a specific test to ensure that this is not a problem again.

@stale
Copy link

stale bot commented Sep 13, 2023

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

@stale stale bot added the status/stale label Sep 13, 2023
@rohanKanojia rohanKanojia self-assigned this Oct 19, 2023
@rohanKanojia rohanKanojia moved this from Planned to In Progress in Eclipse JKube Oct 19, 2023
rohanKanojia added a commit to rohanKanojia/kubernetes-client that referenced this issue Oct 19, 2023
…ptor to not consume response bytes (fabric8io#5251)

Add a unit test in AbstractHttpLoggingInterceptor to verify response
bytes are not consumed after they've been processed by
HttpLoggingInterceptor.

I've verified that test fails when I revert fabric8io#5250
in httpclient-okhttp and httpclient-vertx modules

Signed-off-by: Rohan Kumar <[email protected]>
@rohanKanojia
Copy link
Member

rohanKanojia commented Oct 19, 2023

We need to add a specific test to ensure that this is not a problem again.

I can reproduce the original issue using a unit test in AbstractHttpLoggingInterceptorTest, by test do you mean a unit test or an E2E test (in kubernetes-itests)?

@manusa
Copy link
Member Author

manusa commented Oct 19, 2023

A black-boxed unit test that ensures the behavior (i.e. avoid mocking and verify calls to things like BufferUtil::copy)

rohanKanojia added a commit to rohanKanojia/kubernetes-client that referenced this issue Oct 19, 2023
…ptor to not consume response bytes (fabric8io#5251)

Add a unit test in AbstractHttpLoggingInterceptor to verify response
bytes are not consumed after they've been processed by
HttpLoggingInterceptor.

I've verified that test fails when I revert fabric8io#5250
in httpclient-okhttp and httpclient-vertx modules

Signed-off-by: Rohan Kumar <[email protected]>
@rohanKanojia
Copy link
Member

I see only logger being mocked in AbstractHttpLoggingInterceptorTest. Could you please check if #5532 matches your expectations?

@manusa manusa moved this from In Progress to Review in Eclipse JKube Oct 25, 2023
@manusa manusa added this to the 6.10.0 milestone Nov 9, 2023
manusa pushed a commit that referenced this issue Nov 9, 2023
…ptor to not consume response bytes (#5251)

Add a unit test in AbstractHttpLoggingInterceptor to verify response
bytes are not consumed after they've been processed by
HttpLoggingInterceptor.

I've verified that test fails when I revert #5250
in httpclient-okhttp and httpclient-vertx modules

Signed-off-by: Rohan Kumar <[email protected]>
@github-project-automation github-project-automation bot moved this from Review to Done in Eclipse JKube Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment