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

Async/Sync Tests Rewrite #30

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Async/Sync Tests Rewrite #30

wants to merge 3 commits into from

Conversation

trevorflahardy
Copy link
Collaborator

Async/Sync Tests Rewrite

Currently, the tests for both the async and sync clients have separate files, and thus, have a lot of repeated code.

I aim to merge this into one, ensuring that both clients are functioning as expected and strengthening the safety of all available tests.

Overall Idea

I'm planning to create some sort of mock Client/SyncClient hybrid that calls methods from both clients at once when requested. Pytest tests do not run in parallel, so although this is not the best solution it will ensure the following:

  • That the Client and SyncClient return the same values, and
  • That the Client and SyncClient have all the same functions.

This will effectively merge the two independent test files into one, allowing for easier maintainability and readability.

This PR stems from the ongoing #29 PR to migrate all new features to this new test system.

Start working on ClientHybrid class. Denotes a type of client that calls both the sync and async versions of a client's fetch_x methods.

This is so that, when testing, you don't need two tests for each client type. The guarantee is that the return types between the two clients are the same, so the subsequent tests must also be valid for both of them.

This is backed up by a `HybridMethodProxy` class which basically acts as the actual caller returned by the ClientHyrbid to do some validation.
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.

2 participants