Closed luhn closed 3 years ago
Ok so here's my thinking behind the fixtures, including the improvements in this PR.
So if we add this dummy_config
then we basically have two ways of writing tests.
app + app_request
is basically a fully functional environment with request extensions etc applied. I've seen a couple complaints about this setup.
app
is a session fixture so you can't modify it per-test to override policies etc. But it's way more performant for integration tests.app_request
are hard to mock/modify because they are using @property
without a setter. I introduced a settable_property
in pyramid.utils
in 2.0 which we could use more often to alleviate this issue.app + testapp
for black-box integration tests. This is great, people can update testapp
with features like we do in wiki2 to be able to grab csrf tokens from the app
config.
dummy_config + dummy_request
for made-up light-weight tests that are piecing together only parts of your app's config, overriding settings, etc. This is great for unit tests. It's the workflow being enabled by this PR.
Finally there's app + dummy_request
which doesn't entirely work because threadlocals are not pushed as we'd like them to be. Hopefully this is simply discouraged in the dummy_request
docstring.
@luhn Can you help improve the cookiecutter with this in mind? Do you have any corrections / improvements you'd like to make to my breakdown / goals?
With respect to app_request
not being used anywhere in the default test suite - the entire point of the cookiecutters is to give people good patterns to get started. Just because we aren't using it in the limited tests we ship doesn't mean it's not a good pattern that should remain in the cookiecutter.
I do want to see the fixture get updated to use the context manager so maybe we could add that to this PR:
@pytest.fixture
def app_request(app):
with prepare(registry=app.registry) as env:
request = env['request']
request.host = 'example.com'
yield request
I think that all makes sense.
I agree keeping app_request
, so I'll bring it back to the wiki2 PR.
I'm going to fully root out app + dummy_request
, I didn't realize that was still in there.
Beyond that I think the cookiecutter services the goals pretty well. Is there anything else we need in this PR?
BTW, we will be sprinting this weekend, and I can ask newbies to test drive the revised cookiecutter and wiki tutorial, and solicit feedback. Details of the sprint: https://groups.google.com/g/pylons-discuss/c/76p3Amh4F88/m/c9ybcZV2BQAJ
Beyond that I think the cookiecutter services the goals pretty well. Is there anything else we need in this PR?
Sounds great to me.
Made the updates, how does that look?
Once approved I'll port it over to wiki2 PR.
done, thank you @luhn!
Bringing cookiecutter in line with https://github.com/Pylons/pyramid/pull/3629
I'm leaving
app_request
as-is for now. I'm not sure where we're leaning with it. @mmerickel, should I bringapp_request
back to the Pyramid PR?