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

fix flaky test #2513

Merged
merged 1 commit into from
Aug 14, 2023
Merged

fix flaky test #2513

merged 1 commit into from
Aug 14, 2023

Conversation

szuecs
Copy link
Member

@szuecs szuecs commented Aug 11, 2023

fix: #2435 stdlib http client can sometimes create more than one request, so use RoundTrip and transport instead

…oundTrip and transport to do the request

Signed-off-by: Sandor Szücs <[email protected]>
@szuecs
Copy link
Member Author

szuecs commented Aug 11, 2023

Works on my machine :)

]% go test ./filters/tee -run TestTeeEndToEndBody2TeeRoutesAndClosing -count 100
ok github.com/zalando/skipper/filters/tee 0.508s

@szuecs
Copy link
Member Author

szuecs commented Aug 11, 2023

#2443 fixes the other flaky test (admission control test)

@RomanZavodskikh
Copy link
Contributor

👍

Comment on lines +259 to +261
rt := net.NewTransport(net.Options{})
defer rt.Close()
rsp, err := rt.RoundTrip(req)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use proxy.Client() which is https://pkg.go.dev/net/http/httptest#Server.Client

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I misunderstood the problem this is fixing.

http client can sometimes create more than one request

Maybe its worth expanding when this can happen and how it is related to the flaky test this fixes.

@AlexanderYastrebov
Copy link
Member

👍

@szuecs szuecs merged commit c0bbaae into master Aug 14, 2023
@szuecs szuecs deleted the fix/flaky-test-tee-close branch August 14, 2023 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flaky TestTeeEndToEndBody2TeeRoutesAndClosing
3 participants