[Django] #33022: main-random test failures (build #8): field_deconstruction.tests.FieldDeconstructionTests and migrations.test_commands.MigrateTests
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
I noticed that `main-random` job !#8 [https://djangoci.com/job/main-
random/8/ failed recently]. I don't know if these were "real" failures.
The failures were--
* `field_deconstruction.tests.FieldDeconstructionTests.test_foreign_key`
*
`field_deconstruction.tests.FieldDeconstructionTests.test_many_to_many_field`
* `field_deconstruction.tests.FieldDeconstructionTests.test_one_to_one`
*
`migrations.test_commands.MigrateTests.test_migrate_partially_applied_squashed_migration`
* `migrations.test_commands.MigrateTests.test_migrate_plan`
The shuffle seed was `5300426296`.
These recent PR's seem related:
[https://github.com/django/django/pull/14730 PR #14730] (ticket #32983)
for the `FieldDeconstructionTests` ones and
[https://github.com/django/django/pull/14727 PR #14727] (ticket #29063)
for the `MigrateTests` ones.
--
Ticket URL: <https://code.djangoproject.com/ticket/33022>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
I narrowed down the `MigrateTests` failures somewhat to--
{{{
./tests/runtests.py --shuffle 5300426296
migrations.test_commands.MigrateTests migrations.test_executor
...
Ran 62 tests in 2.041s
FAILED (errors=2)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:1>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:2>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Uncategorized | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Chris Jerdonek:
Old description:
> I noticed that `main-random` job !#8 [https://djangoci.com/job/main-
> random/8/ failed recently]. I don't know if these were "real" failures.
> The failures were--
>
> * `field_deconstruction.tests.FieldDeconstructionTests.test_foreign_key`
> *
> `field_deconstruction.tests.FieldDeconstructionTests.test_many_to_many_field`
> * `field_deconstruction.tests.FieldDeconstructionTests.test_one_to_one`
> *
> `migrations.test_commands.MigrateTests.test_migrate_partially_applied_squashed_migration`
> * `migrations.test_commands.MigrateTests.test_migrate_plan`
>
> The shuffle seed was `5300426296`.
>
> These recent PR's seem related:
> [https://github.com/django/django/pull/14730 PR #14730] (ticket #32983)
> for the `FieldDeconstructionTests` ones and
> [https://github.com/django/django/pull/14727 PR #14727] (ticket #29063)
> for the `MigrateTests` ones.
New description:
I noticed that `main-random` job !#8 [https://djangoci.com/job/main-
random/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_key`
*
`field_deconstruction.tests.FieldDeconstructionTests.test_many_to_many_field`
* `field_deconstruction.tests.FieldDeconstructionTests.test_one_to_one`
*
`migrations.test_commands.MigrateTests.test_migrate_partially_applied_squashed_migration`
* `migrations.test_commands.MigrateTests.test_migrate_plan`
The shuffle seed was `5300426296`.
These recent PR's seem related:
[https://github.com/django/django/pull/14730 PR #14730] (ticket #32983)
for the `FieldDeconstructionTests` ones and
[https://github.com/django/django/pull/14727 PR #14727] (ticket #29063)
for the `MigrateTests` ones.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:3>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Uncategorized | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
Yep. Nice work. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:4>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* type: Cleanup/optimization => Bug
* component: Uncategorized => Migrations
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:5>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* severity: Normal => Release blocker
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:6>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* has_patch: 0 => 1
Comment:
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.
[https://github.com/django/django/pull/14780 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:7>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* has_patch: 1 => 0
* severity: Release blocker => Normal
Comment:
It's a test isolation issue not a regression.
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:8>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jacob Walls):
Reiterating from above PR:
The pair of migrations tests that produces a failure is:
- `test_custom_user`
- `test_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",
}}}
[https://docs.djangoproject.com/en/3.2/topics/auth/customizing/#changing-
to-a-custom-user-model-mid-project 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:9>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: nobody
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
> 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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:10>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: nobody => Mariusz Felisiak
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/14957 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:11>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: Migrations | Version: dev
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jacob Walls):
* stage: Accepted => Ready for checkin
Comment:
Verified the `FieldDeconstruction` test isolation issues also were
resolved (using the original shuffle seed).
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:12>
Django
field_deconstruction.tests.FieldDeconstructionTests and
migrations.test_commands.MigrateTests
Reporter: Chris Jerdonek | Owner: Mariusz
| Felisiak
Component: Migrations | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by GitHub <noreply@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"01bf679e59850bb7b3e6395f1683bd1463ed9969" 01bf679e]:
{{{
#!CommitTicketReference repository=""
revision="01bf679e59850bb7b3e6395f1683bd1463ed9969"
Fixed #33022 -- Fixed isolation of
migrations.test_executor.ExecutorTests.test_custom_user().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33022#comment:13>