Opened 6 years ago

Closed 6 years ago

#29658 closed Cleanup/optimization (fixed)

Use a context manager to unregister model field lookups in tests

Reported by: Mads Jensen Owned by: Srinivas Reddy Thatiparthy
Component: Core (Other) 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: yes UI/UX: no

Description (last modified by Tim Graham)

There are some places in the test suite that follow this pattern:

try:
   CharField.register_lookup(Length)
   # some test code
finally:
   CharField._unregister_lookup(Length)

It'd be better to use a context manager (addCleanup() was originally proposed) to handle this stuff.

Change History (9)

comment:1 by Claude Paroz, 6 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Dominic White, 6 years ago

Owner: changed from nobody to Dominic White
Status: newassigned

comment:3 by Srinivas Reddy Thatiparthy, 6 years ago

Last edited 6 years ago by Srinivas Reddy Thatiparthy (previous) (diff)

comment:4 by Srinivas Reddy Thatiparthy, 6 years ago

Owner: changed from Dominic White to Srinivas Reddy Thatiparthy

comment:5 by Claude Paroz, 6 years ago

Has patch: set

The current patch is using a context manager instead of the try/finally method.
Mads, is that something you are comfortable with, or do you still see some benefit with addCleanup?

comment:6 by Tim Graham, 6 years ago

Triage Stage: AcceptedReady for checkin

Patch looks good to me, unless there's some reason to prefer addCleanup().

comment:7 by Simon Charette, 6 years ago

Using context managers should be fine as well.

comment:8 by Tim Graham, 6 years ago

Description: modified (diff)
Summary: Use unittest.addCleanup to unregister lookups.Use a context manager to unregister model field lookups in tests

comment:9 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 233c70f0:

Fixed #29658 -- Registered model lookups in tests with a context manager.

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