[Django] #26315: Allow call_command() to accept a Command object as the first argument
Django
-----------------------------------+--------------------
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/26315>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
I think this makes sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:1>
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
Pending a few cosmetic tweaks.
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:2>
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"4115288b4f7bbd694946a1ddef0f0ba85c03f9a1" 4115288b]:
{{{
#!CommitTicketReference repository=""
revision="4115288b4f7bbd694946a1ddef0f0ba85c03f9a1"
Fixed #26315 -- Allowed call_command() to accept a Command object as the
first argument.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:3>
Django
Reporter: jdufresne | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"1845bc1d1007751a7f65c66aeddc35f032f6bf41" 1845bc1d]:
{{{
#!CommitTicketReference repository=""
revision="1845bc1d1007751a7f65c66aeddc35f032f6bf41"
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().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:4>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Melvyn Sopacua):
* status: closed => new
* resolution: fixed =>
Comment:
Replying to [comment:4 Tim Graham <timograham@…>]:
> In [changeset:"1845bc1d1007751a7f65c66aeddc35f032f6bf41" 1845bc1d]:
> {{{
> #!CommitTicketReference repository=""
revision="1845bc1d1007751a7f65c66aeddc35f032f6bf41"
> 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:
{{{#!python
call_command('name', options_i_know_about=value)
}}}
Evident
[https://bitbucket.org/stephenmcd/mezzanine/src/82939507b6dc2519161db6ee49fec1bd194520de/mezzanine/core/management/commands/createdb.py?at=default&fileviewer
=file-view-default#createdb.py-48 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
[https://bitbucket.org/stephenmcd/mezzanine/src/82939507b6dc2519161db6ee49fec1bd194520de/mezzanine/utils/docs.py?at=default&fileviewer
=file-view-default#docs.py-259 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" - in fact 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:5>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => fixed
Comment:
Please open a new ticket rather than reopening one that's already been
released. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:6>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: New feature | Status: closed
Component: Testing framework | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by Claude Paroz):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/26315#comment:7>