-
Notifications
You must be signed in to change notification settings - Fork 36
Lazy generation of client (and transport) #344
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
base: main
Are you sure you want to change the base?
Conversation
Commit 4d9d3b4 changed when a WSGI app used for intercept is instantiated, and we are no longer able to set up pre-conditions for application using the fixtures hook. Delay creation of the test client instance (which also instantiate the given WSGI app) to restore the mechanism.
|
Something like this might work, but isn't the wsgi app getting initialised for every http class, meaning for every single test? I'm not certain that's the case, but it looks that way. If that's the case, that's probably too expensive for some wsgi apps which might have lots of setup. If there's some way to store the lazily-created client on the Suite, that might achieve what you're after? |
Note that it would need to be the client member of the Http class, not the http class itself, as each test needs to have individual control over the |
|
Yeah, it creates a new client for every intercepted test, meaning it restarts the wsgi app for every test. Somewhere north of 170 times. If we attach the client as a member to the suite it is a more reasonable <50 (which is closer to the number of yaml files). See if a cleaned up version of the following diff works? |
|
It's also at kajinamit#1 |
|
In my PR one thing that will need to be checked for correctness is test suites where requests to "real" hosts are mixed with requests to the intercepted apps. |
It appears to not be working, so the logic probably needs some adjustment. |
|
Thinking about this overnight, the code in this patch maintains the same behaviour as >=4.0: Each test has its own transport, thus each test starts its own WSGI app. As I state above this is probably undesirable. My patch hangs the client on the suite, but then every request uses the WSGITransport meaning any test that is in an intercepted suite that requests an external URL does not work as expected (there's apparently a shortcoming in the existing tests that needs to be fixed as part of addressing this). When a test URL has no scheme (http or https) then it will get a Thoughts? |
Commit 4d9d3b4 changed when a WSGI app used for intercept is instantiated, and we are no longer able to set up pre-conditions for application using the fixtures hook.
Delay creation of the test client instance (which also instantiate the given WSGI app) to restore the mechanism.