Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#26315 closed New feature (fixed)

Allow call_command() to accept a Command object as the first argument

Reported by: Jon Dufresne Owned by: nobody
Component: Testing framework 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

Several of Django's tests assert the state of a command object. For example:ManageRunserver.assertServerSettings.

https://github.com/django/django/blob/50931df/tests/admin_scripts/tests.py#L1306

The preferred interface for testing commands is call_command(). When using call_command() the caller has no reference to the Command object used during execution. This makes it difficult to use this interface in some tests.

One solution would be to allow call_command() to accept a Command object as the first argument instead of the name of the command. The caller would then have a reference to the object used in asserts.

https://github.com/django/django/pull/6217

Change History (7)

comment:1 by Claude Paroz, 8 years ago

Triage Stage: UnreviewedAccepted

I think this makes sense.

comment:2 by Tim Graham, 8 years ago

Triage Stage: AcceptedReady for checkin

Pending a few cosmetic tweaks.

comment:3 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 4115288b:

Fixed #26315 -- Allowed call_command() to accept a Command object as the first argument.

comment:4 by Tim Graham <timograham@…>, 8 years ago

In 1845bc1d:

Refs #26315 -- Cleaned up argparse options in commands.

  • Removed type coercion. Options created by argparse are already coerced to the correct type.
  • Removed fallback default values. Options created by argparse already have a default value.
  • Used direct indexing. Options created by argparse are always set. This eliminates the need to use dict.get().

in reply to:  4 comment:5 by Melvyn Sopacua, 7 years ago

Resolution: fixed
Status: closednew

Replying to Tim Graham <timograham@…>:

In 1845bc1d:

Refs #26315 -- Cleaned up argparse options in commands.

  • Removed type coercion. Options created by argparse are already coerced to the correct type.
  • Removed fallback default values. Options created by argparse already have a default value.
  • Used direct indexing. Options created by argparse are always set. This eliminates the need to use dict.get().

The final statement is untrue. Various management commands from reusable apps are now failing (most notably Mezzanine and django-extensions), because they invoke:

call_command('name', options_i_know_about=value)

Evident here.

This means those commands blow up on options['no_color'] with a KeyError. When the exception is caught and reformatted things get worse, like here, where the resulting error message (Couldn't build model_graph, graph_models failed on: 'no_color') made me think of lacking LCMS support in Pillow.

I'm proposing a temporary fix to check if all "options created by argparse" - being the options that should always be supported on any management command are in fact present in the passed in options dict and if not issue a DeprecationWarning that calling call_command() without passing the original options dictionary will be unsupported in the future.

Last edited 7 years ago by Melvyn Sopacua (previous) (diff)

comment:6 by Tim Graham, 7 years ago

Resolution: fixed
Status: newclosed

Please open a new ticket rather than reopening one that's already been released. Thanks.

comment:7 by Claude Paroz, 7 years ago

Note that in your problematic case, the code is directly calling graph_models.Command().execute(*apps, **options) instead of using call_command which takes care of populating all default arguments.

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