TST: Use pytest-localserver instead of making network connections #53828
+468
−577
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mroeschke
added
Testing
rhshadrach
reviewed
Jun 26, 2023
Comment on lines
615
to
616
| It is highly discouraged to add a test that connects to the internet due to flakiness of network connections and | ||
| lack of ownership of the server that is being connected to. If network connectivity is absolutely required, use the | ||
| ``tm.network`` decorator. | ||
| lack of ownership of the server that is being connected to. If network connectivity is absolutely required, mock |
There was a problem hiding this comment.
With this, are these still highly discouraged? Seems like all the reasons to discourage them no longer apply.
There was a problem hiding this comment.
Good point I'll clarify the wording here, "highly discouraged" is probably too strong if they can be mocked properly
|
This broke the env on ARM Mac. pytest local server requires aiosmtpd which isn't available on Mac Edit: Does anyone have bandwidth to look into it? Otherwise I'd revert this for now |
|
I am OK with revert unless @mroeschke has an objection |
1 task
|
Could we just remove pytest localserver from environment.yaml? This plugin helps save a few minutes from CI runtime and having the network rests run locally probably isn't a huge priority |
Daquisu
pushed a commit
to Daquisu/pandas
that referenced
this pull request
Jul 8, 2023
…ndas-dev#53828) * Use pytest-localserver instead of making network connections * Fix test, remove network function * remove network from init * Ignore disutils from datareader, s3so * specify encoding * Specify encoding * Clarify contributing doc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we have a lot of CI jobs with unit test that make network connections, it is better if the network connections could be made locally if possible. This PR refactors all tests that make network connections to use
pytest-localserverinstead