Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

PoC:pytest-socket like testing#4317

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
Bibo-Joshi wants to merge4 commits intooverhaul-testsfrompytest-socket

Conversation

Bibo-Joshi
Copy link
Member

spin-off from#4271.

Initial fidings & thoughts:

  • pytest-socket doesn't seem to play well with our asyncio-loop-settings
  • evaluating fixtures that make requests from withinWithoutRequest tests is currently failing
  • at first glance, there doesn't seem to be a large number of tests that make requests even though we don't want them to. It might be worth hiding the logic behind a env-var so that the additional logic doesn't slow down local testing (but reports wrong networking usage in the CI)

@Bibo-JoshiBibo-Joshi added the ⚙️ testsaffected functionality: tests labelJun 21, 2024
@Bibo-JoshiBibo-Joshi marked this pull request as draftJune 21, 2024 21:05
@Bibo-JoshiBibo-Joshi changed the base branch frommaster tooverhaul-testsJune 22, 2024 09:28
@Bibo-Joshi
Copy link
MemberAuthor

Further insights:

  • If the flag for "internet connection allowed" is a singleton, i.e. globally unique, it's not really possible to have the tests run in parallel via pytest-xdist while still allowingsome tests to make http requests. If you want to avoid race conditions here by using locks, you basically go back to sequential execution (at least for everything that uses botHTTPXRequest).
  • If we want to set the flag on a per-test/per-fixture basis, we'd have to use uniqueHTTPXRequest/httpx.AsyncClient/bot/socket objects for each testand figure out a way how to set the flag via markers or similar methods that don't require manual interaction in the tests

These insights are quite a bummer in terms of incorporating the allow-internet-flag into our standard testing suite. Still, I can see a few options:

  • provide different fixturesoffline_bot &online_bot that can be used. This requires rewriting all tests to use those new fixtures. It also means that it will harder (or impossible?) to check if a*WithoutRequest class uses theonline_bot or vice versa and deny that.
  • Instead of aiming to fully incorporate this into our standard test suite, we could try to simply run a sanity check every week or so to detect if any test is using the internet even though I should not. For that, we could cut off the internet connection for almost everything and run only the*WithoutRequest & ext tests.

@harshil21 what do you think?

@Bibo-Joshi
Copy link
MemberAuthor

closing in favor of#4454

@Bibo-JoshiBibo-Joshi deleted the pytest-socket branchSeptember 3, 2024 03:25
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsSep 10, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
⚙️ testsaffected functionality: tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

1 participant
@Bibo-Joshi

[8]ページ先頭

©2009-2025 Movatter.jp