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

Add scope_client parameter to TestClient #2430

Closed
wants to merge 2 commits into from

Conversation

pythonweb2
Copy link

@pythonweb2 pythonweb2 commented Jan 20, 2024

Summary

A breaking change was made when the TestClient's scope["client"] was set to None. In cases where an application uses the scope["client"] in places like custom middleware, logging, etc, upgrading to 0.35 becomes a very hard task. Testing middleware/logging/business logic that relies on the client being populated becomes hard if not impossible. Middleware that worked before breaks, and now we need to add checks in application code to disable certain things based on whether or not the test suite is running.

This PR adds the ability for users to specify what the scope["client"] should be when using the test client. This way, when using fixtures in a framework like pytest (which is the recommended setup), a one line change is all that is required.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.

@pythonweb2
Copy link
Author

I made it so instead of reverting the change, there is a new option to keep the old functionality, which should be less controversial (if some have already change their code for the 0.35.0 release). Would this be OK to add?

@Kludex

@pythonweb2 pythonweb2 changed the title Undo test client change Add scope_client parameter to TestClient Jan 20, 2024
@Kludex
Copy link
Member

Kludex commented Jan 20, 2024

Let's discuss it first.

@Kludex Kludex closed this Jan 20, 2024
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