-
Notifications
You must be signed in to change notification settings - Fork 345
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
tests: run Test_django_db_blocker in testdir #806
base: main
Are you sure you want to change the base?
Conversation
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.
Placing them in a subprocess fix the issue that django_db_blocker
currently has. WHen django_db_blocker
is requested on its own, the DB is not set up. These tests pass because of the other two tests that trigger the session scope-level django_db_setup
fixture. Because they always run first (due to the test reordering logic), the DB will always be prepared for the other two tests (that would fail if run on their own).
The fix I found was to rename django_db_blocker
to _django_db_blocker
, have django_db_setup
use that, and then create a new session scope-level fixture called django_db_blocker
which requests both of them. This way, whenever django_db_blocker
is called, it ensures the DB is set up.
@SalmonMode thanks! I think this PR/change still makes sense (separately), since it simulates running the tests alone. And it could be extended then to test what your renaming/re-arranging would fix (in a separate test). |
I'd be happy to 😁 There's some already existing tests I can make tweaked copies of to cover this. Sorry for causing all the ruckus, btw. |
Great. I assume this can be done on top of this one?
I'd rather avoid new tests if existing ones can be extended / hardened - the test suite is rather slow already, especially due to all the
No worries.. :) |
Not a problem. I can just tweak them in place.
Sorry, I'm not sure I follow. You mean if I can make changes on top of the current change you're making here? |
Here's the PR #807 |
Ref: #802 (comment)