Opened 4 years ago
Closed 4 years ago
#33022 closed Bug (fixed)
main-random test failures (build #8): field_deconstruction.tests.FieldDeconstructionTests and migrations.test_commands.MigrateTests
| Reported by: | Chris Jerdonek | Owned by: | Mariusz Felisiak |
|---|---|---|---|
| Component: | Migrations | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
I noticed that main-random job #8 failed recently. I don't know if these were "real" failures. The failures were (database=spatialite,bionic,python3.9)--
field_deconstruction.tests.FieldDeconstructionTests.test_foreign_keyfield_deconstruction.tests.FieldDeconstructionTests.test_many_to_many_fieldfield_deconstruction.tests.FieldDeconstructionTests.test_one_to_onemigrations.test_commands.MigrateTests.test_migrate_partially_applied_squashed_migrationmigrations.test_commands.MigrateTests.test_migrate_plan
The shuffle seed was 5300426296.
These recent PR's seem related: PR #14730 (ticket #32983) for the FieldDeconstructionTests ones and PR #14727 (ticket #29063) for the MigrateTests ones.
Change History (13)
comment:2 by , 4 years ago
Here is a shuffle seed of the same migration tests as above, but with 4 failures:
./tests/runtests.py migrations.test_commands.MigrateTests migrations.test_executor.ExecutorTests --shuffle 3472950421
comment:3 by , 4 years ago
| Description: | modified (diff) |
|---|
comment:5 by , 4 years ago
| Component: | Uncategorized → Migrations |
|---|---|
| Type: | Cleanup/optimization → Bug |
comment:6 by , 4 years ago
| Severity: | Normal → Release blocker |
|---|
comment:7 by , 4 years ago
| Has patch: | set |
|---|
Thanks, Chris. For the migration tests, I've narrowed down the pair of problematic tests. Just not 100% certain yet which one needs to be adjusted until I understand the test I didn't author a bit more. Any advice is welcome.
comment:8 by , 4 years ago
| Has patch: | unset |
|---|---|
| Severity: | Release blocker → Normal |
It's a test isolation issue not a regression.
comment:9 by , 4 years ago
Reiterating from above PR:
The pair of migrations tests that produces a failure is:
test_custom_usertest_migrate_partially_applied_squashed_migration
Can be produced with:
./tests/runtests.py migrations.test_commands.MigrateTests migrations.test_executor.ExecutorTests -k test_custom_user -k test_migrate_partially --shuffle 1022528553
The failure occurs during the migrate (to zero) command in the finally block with:
raise ValueError("\n".join(error.msg for error in errors))
ValueError: The field migrations.Book.author was declared with a lazy reference to 'auth.user', but app 'auth' isn't installed.
test_custom_user overrides the AUTH_USER_MODEL setting like so:
AUTH_USER_MODEL="migrations.Author",
Documentation for the setting says that it's cumbersome to change this after migrations have been made:
Changing AUTH_USER_MODEL after you’ve created database tables is significantly more difficult since it affects foreign keys and many-to-many relationships, for example. This change can’t be done automatically and requires manually fixing your schema, moving your data from the old user table, and possibly manually reapplying some migrations. See #25313 for an outline of the steps.
This makes sense, because in the ordinary test execution order, the squashed migration in test_migrate_partially_applied_squashed_migration creates a Book.Author field like this:
('author', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='migrations.author')),
But in the failing test order, instead creates this:
('author', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)),
Since the documentation mentions this is cumbersome to address, the thought in the PR was just to keep all tests running on the app "migrations" with the same AUTH_USER_MODEL, but if that's not satisfactory, is the idea that we should rewrite test_migrate_partially_applied_squashed_migration to use a different app to run the test cases? Otherwise I'm not sure how to prevent squashmigrations from picking up the swappable user model.
Thanks, and happy to keep digging if helpful.
comment:10 by , 4 years ago
Can be produced with:
FYI, once you have specific tests to reproduce with, you can dispense with the --shuffle argument and just provide the order manually (not needing -k, etc):
./tests/runtests.py migrations.test_executor.ExecutorTests.test_custom_user migrations.test_commands.MigrateTests.test_migrate_partially_applied_squashed_migration
comment:12 by , 4 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
Verified the FieldDeconstruction test isolation issues also were resolved (using the original shuffle seed).
I narrowed down the
MigrateTestsfailures somewhat to--