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

Use unasync for tests #3520

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Use unasync for tests #3520

wants to merge 15 commits into from

Conversation

karpetrosyan
Copy link
Member

@karpetrosyan karpetrosyan commented Feb 27, 2025

This PR attempts to use the unasync strategy to keep both async and sync tests of the client synchronized. Previously, we encountered issues where something worked for the async client but not for the sync client. I tried using unasync to generate the synchronous client code, but it seems too risky to do so without first applying the unasync strategy to the tests. This way, in the next step, we can safely generate the implementation without any risks.

Ref: #3046

I tried to make the changes incrementally to make the review process easier. The changes include:

  • Removing sync tests completely and replacing them with async ones (we don't need to write any sync code as it will be autogenerated).
  • In async tests, avoiding test names that mention "async," such as "test_async_client_request." We don't need this, as it would only complicate our unasync code by requiring additional rules.
  • Copying unasync code from httpcore and adding it to the lint/check scripts.

@karpetrosyan
Copy link
Member Author

What do you think, @tomchristie ? I would appreciate a review. These kinds of PRs are hard to keep in sync with the master branch.

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.

1 participant