[Django] #25493: Model instances created with unittest.mock can raise confusing errors
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Type: Bug | Status: new
Component: Database | Version: 1.9a1
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Django can raise one of the following
- `ValueError: Failed to insert expression` or;
- `ValueError: Failed to insert expression "<MagicMock
name='create().id.resolve_expression()' id='139633074508184'>" on
fundraising.Payment.stripe_charge_id. F() expressions can only be used to
update, not to insert.`
when using unittest.mock to mock the creation of an object. The
djangoproject.com tests began throwing errors. Tim fixed these problems in
this PR: https://github.com/django/djangoproject.com/pull/524
The PR adds return values for the id of columns that are used to create
new instances to avoid these errors.
What I think is happening is that the lack of id is somehow causing the
mock to be treated as a `Col`. The existence of a `Col` in a
`objects.create()` call is what triggers the confusing error message.
Since no `F()` expression has been used by the user, tracking down the
cause is difficult.
We should try and figure out why the mock is being resolved to a Col and
stop that from happening or improve the error message if that's not
possible.
--
Ticket URL: <https://code.djangoproject.com/ticket/25493>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.9a1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
The difficulty is that all `hasattr` checks on a ` MagicMock` return True
and any attribute or method access return `MagicMock` objects.
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.9a1
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "25493-test.diff" added.
Django
-------------------------------------+-------------------------------------
Reporter: jarshwah | Owner: nobody
Cleanup/optimization |
Component: Database layer | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 1.9a1 => 1.9
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Marcelo Galigniana):
Hello!
I ran into this error and I also found the exception too confusing. I
would like to fix it / improve the error message.
Could be a fix check in `pre_save_val` if the attr value is a primitive
python value? (Because is getting an <MagicMock id='XXX'>) object)
https://github.com/django/django/blob/751d732a3815a68bdb5b7aceda0e7d5981362c4a/django/db/models/sql/compiler.py#L1755
Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Galigniana
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Marcelo Galigniana
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Galigniana
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
unittest.mock is a blunt testing tool which I discourage the use of,
especially for complex things like the ORM. If you’re using it to replace
parts of a system, it’s your responsibility to make that work. I don’t
believe we should try to make the ORM more compatible with parts being
mocked.
If you’re mocking to try speed up tests, there are some safer techniques
to try first. Use setUpTestData to create shared model instances (see my
post: https://adamj.eu/tech/2021/04/12/how-to-convert-a-testcase-from-
setup-to-setuptestdata/ ), or use unsaved instances inside a
SimpleTestCase.
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Galigniana
Cleanup/optimization | Status: assigned
Component: Database layer | Version: 1.9
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Sorry for the delay. Thank you Adam!
> it’s your responsibility to make that work. I don’t believe we should
try to make the ORM more compatible with parts being mocked
Should we document this on some way? Or simply close this issue?
Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Galigniana
(models, ORM) |
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* resolution: => wontfix
* status: assigned => closed
Comment:
Absent a concrete proposal, I think we can close it.
--
Ticket URL: <https://code.djangoproject.com/ticket/25493#comment:7>