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

Remove external HTTP request from hyper crate tests #4631

Open
goatgoose opened this issue Jun 26, 2024 · 2 comments · May be fixed by #4838
Open

Remove external HTTP request from hyper crate tests #4631

goatgoose opened this issue Jun 26, 2024 · 2 comments · May be fixed by #4838

Comments

@goatgoose
Copy link
Contributor

Problem:

To avoid complicating the initial hyper crate PR with a localhost testing framework, a test was added that makes an HTTP request to an external source. This introduces potential flakiness, since requests to external sources aren't reliable.

Solution:

Remove the external HTTP request after localhost testing can be performed.

@goatgoose goatgoose changed the title Remove external HTTP request from hyper crate Remove external HTTP request from hyper crate tests Jun 26, 2024
@jmayclin
Copy link
Contributor

jmayclin commented Jul 1, 2024

I know that the external HTTP request stuff is a bit tedious, but it is a really nice integration test to have. Maybe if it's a matter of trading off, we could deprecate the "well-known endpoints" integration test and move all of that to a similar integration test relying on s2n-tls-hyper?

@goatgoose
Copy link
Contributor Author

The purpose of the current external HTTP request is just a sanity check to make sure the hyper crate works at all. I think it does make sense to remove this test after localhost tests are added. However, I do agree that it makes sense to have real network tests, so I opened a separate issue for this: #4837

@goatgoose goatgoose linked a pull request Oct 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants