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.

Sign up for GitHub

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

Jump to bottom

unittest MigratorTestCase broken on Django 5.2 #503

Closed
bckohan opened this issue Mar 18, 2025 · 2 comments · Fixed by #504
Closed

unittest MigratorTestCase broken on Django 5.2 #503

bckohan opened this issue Mar 18, 2025 · 2 comments · Fixed by #504

Comments

@bckohan
Copy link
Contributor

bckohan commented Mar 18, 2025

Django 5.2b1 is out with 5.2 expected in April.

The private method _pre_setup() on Django's SimpleTestCase was changed from an instance method to a class method. This breaks MigratorTestCase because it overrides _pre_setup.
bckohan added a commit to bckohan/django-test-migrations that referenced this issue Mar 18, 2025
@bckohan
add django 5.2b1 to CI matrix to illustrate wemake-services#503
aadb8be
@sobolevn
Copy link
Member

@bckohan PR is welcome!

bckohan added a commit to bckohan/django-test-migrations that referenced this issue Mar 18, 2025
@bckohan
tweak unittest to illustrate wemake-services#503 bug
0b22074
@bckohan
Copy link
Contributor Author

bckohan commented Mar 18, 2025

Bug illustrated here: https://github.com/bckohan/django-test-migrations/actions/runs/13917264417

You have to override setUpClass and call the parent to trigger it.

bckohan added a commit to bckohan/django-test-migrations that referenced this issue Mar 18, 2025
@bckohan
fix wemake-services#503. We simply convert the _pre_setup to a classm…
44ede39 
…ethod. Its fine that it only runs once for all tests on the class. The receivers are restored to the originals each time.
@bckohan bckohan mentioned this issue Mar 18, 2025
Merged
sobolevn added a commit that referenced this issue Mar 19, 2025
@bckohan @sobolevn
Support Django 5.2, Upgrade to Poetry 2.x (#504)
f0523aa 
* add django 5.2b1 to CI matrix to illustrate #503

* add django 5.2 compatible postgres db to docker compose

* tweak unittest to illustrate #503 bug

* pass lint

* second attempt to make the linter happy

* are you kidding me? lol

* codecov-action file is deprecated - use files isntead

* Fix unit target in makefile, spaces instead of tabs meant tests were not being run. Use alternate port for second postgres container. Override setUpClass to illustrate the bug

* fix #503. We simply convert the _pre_setup to a classmethod. Its fine that it only runs once for all tests on the class. The receivers are restored to the originals each time.

* upgrade to poetry 2.x

* silence deprecation warning by removing unnecessary i18n setting

* switch _pre_setup definition based on django version

* because we now have different code paths depending on django versions we need to aggregate our coverage files from all CI jobs to see the real total

* fail CI if aggregated coverage is less than 100

* make coverage file name artifacts unique

* add missing poetry instal to coverage-combine

* fix coverage tool combine invocations to use poetry run

* Update pyproject.toml

Co-authored-by: sobolevn <[email protected]>

* revert back to failing on <100% test coverage - exempt _pre_setup from coverage

* fix requires-python - uv balked at the carrot format?

* remove coverage aggregation

* update lock file

---------

Co-authored-by: sobolevn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support Django 5.2, Upgrade to Poetry 2.x bckohan/django-test-migrations
2 participants
@sobolevn @bckohan