-
Notifications
You must be signed in to change notification settings - Fork 346
Exposes the underlining TestCase instance to avoid using "asserts" module #1191
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
@bluetech thoughts? |
I have a (vain?) hope that someday pytest will be thread safe (i.e. specifically, it should be possible to run multiple pytest instances concurrently in the same process). Currently it isn't, and even pytest-django adds some issues itself. But I think it better to avoid adding new ones. I think if we decide that's it worth putting effort into this issue, we should find a way to make the assert "bind" to the test it is running under. And if there's no clean way to do that, then consider an alternative interface which does make it possible (e.g. a |
This makes sense.
I have some ideas on how to make this work |
@bluetech what do you think of this approach? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- docs/helpers.rst: Language not supported
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Files not reviewed (1)
- docs/helpers.rst: Language not supported
Comments suppressed due to low confidence (2)
pytest_django/fixtures.py:221
- [nitpick] The fixture name 'django_testcase_class' might be ambiguous; consider renaming it to 'DjangoTestCaseClass' to clearly indicate that it returns a TestCase class.
def django_testcase_class(
pytest_django/asserts.py:34
- [nitpick] The deprecation warning message could be made clearer by explicitly specifying the recommended usage of the 'django_testcase' fixture along with its attributes.
message = (
@bluetech thoughts how I can improve this? |
@kingbuzzman That's a creative solution, I like it! I will definitely try to review this before the next release. |
I think the fixture will make tests too verbose, and the deprecation will cause unnecessary churn. I think it would be better with a shorter fixture name and a retention of the asserts module. Perhaps the asserts module can be rewritten with wrapper functions that call the fixture methods? |
@adamchainz understand the making the fixture name smaller, but now sure about how the wrapper would work without frame hacking.. can you provide an example? |
What do you guys think of renaming "django_testcase" to just |
Related to issue: #1155