Skip to content

Conversation

@j178
Copy link
Contributor

@j178 j178 commented Aug 19, 2020

Closes #1195

Should I add tests to cover these parameters in top-level API functions?

Copy link
Contributor

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

Nice, thanks!

tests

Sure! Perhaps a single test against httpx.request is enough here, but up to you!

@j178
Copy link
Contributor Author

j178 commented Aug 20, 2020

I find it a little difficult to test whether these parameters passed to top-level API functions are working as expected, do you have any ideas?

@florimondmanca
Copy link
Contributor

I see! Indeed I went through our proxy tests here and they're mostly about the proxy routing functionality - no existing test actually integrates with HTTPCore and a live proxy. Well no sweat then, I think it's alright (we certainly don't have tests for all options in the top level API anyway).

Copy link
Contributor

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

💯

Found a tiny typo

@j178 j178 merged commit 84ca201 into master Aug 20, 2020
@j178 j178 deleted the top-level-proxies branch August 20, 2020 07:55
@lovelydinosaur lovelydinosaur mentioned this pull request Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow proxies in httpx.request() high-level API

2 participants