Opened 8 years ago

Closed 3 weeks ago

#25493 closed Cleanup/optimization (wontfix)

Model instances created with unittest.mock can raise confusing errors

Reported by: Josh Smeaton Owned by: Marcelo Galigniana
Component: Database layer (models, ORM) Version: 1.9
Severity: Normal Keywords:
Cc: josh.smeaton@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Attachments (1)

25493-test.diff (1.1 KB ) - added by Tim Graham 8 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

The difficulty is that all hasattr checks on a MagicMock return True and any attribute or method access return MagicMock objects.

by Tim Graham, 8 years ago

Attachment: 25493-test.diff added

comment:2 by Tim Graham, 8 years ago

Version: 1.9a11.9

comment:3 by Marcelo Galigniana, 3 months ago

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!

comment:4 by Marcelo Galigniana, 2 months ago

Owner: changed from nobody to Marcelo Galigniana
Status: newassigned

comment:5 by Adam Johnson, 5 weeks ago

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.

comment:6 by Marcelo Galigniana, 3 weeks ago

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

Totally agree!

Should we document this on some way? Or simply close this issue?

Thank you!

comment:7 by Tim Graham, 3 weeks ago

Resolution: wontfix
Status: assignedclosed

Absent a concrete proposal, I think we can close it.

Note: See TracTickets for help on using tickets.
Back to Top