[Django] #23663: Commands classes are not properly testable due to their initialization in execute() method!

Django

unread,

Oct 16, 2014, 6:39:34 AM10/16/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7
Component: Core (Management | Keywords: test, tdd, management,
commands) | commands
Severity: Normal | Has patch: 0
Triage Stage: Unreviewed | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
I was writing a series of tests for a custom command, but I get:

'''AttributeError: 'Command' object has no attribute 'stderr''''''''

I realized that the problem is that references to "stderr" and "stdout"
are initialized in the execute() method!
This makes commands classes untestable, or better, I can write tests but
tests that are not unitary, this is against the principles of TDD, I
should not run a command to test it, instead I should write tests for each
single little piece of logic that makes my command do the job.

Please, move initialization logic into __init__ so we can test commands
methods separately.

--
Ticket URL: <https://code.djangoproject.com/ticket/23663>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Oct 16, 2014, 6:53:09 AM10/16/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

Currently I bypassed the issue by mocking '''stdout''' and '''stderr''':

class OutputMock(object):
@staticmethod
def write(message):
print message

def setUp(self):
self.command = Command()
self.command.stdout = OutputMock()
self.command.stderr = OutputMock()

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:1>

Django

unread,

Oct 16, 2014, 7:34:09 AM10/16/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by berkerpeksag):

* cc: berker.peksag@… (added)

Comment:

Alternatively, you can use `io.StringIO` instead of `OutputMock`.

{{{#!python
import io

class SpamTest(TestCase):

def setUp(self):
self.command = Command()

self.command.stdout = io.StringIO()
self.command.stderr = io.StringIO()
}}}

This way, you can also test the output easily (e.g. something like
`self.assertFalse(self.command.stderr.getvalue())`).

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:2>

Django

unread,

Oct 20, 2014, 7:12:40 AM10/20/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

* has_patch: 0 => 1
* easy: 1 => 0
* stage: Unreviewed => Accepted

Comment:

PR https://github.com/django/django/pull/3391 addresses this in one of the
individual commits.

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:3>

Django

unread,

Oct 20, 2014, 11:11:50 AM10/20/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Component: Core (Management | Resolution:
commands) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0

Keywords: test, tdd, | Patch needs improvement: 1

management, commands | UI/UX: 0
Has patch: 1 |
Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by claudep):

* needs_better_patch: 0 => 1

Comment:

I'd really like to address this issue in a separate pull request. Moreover
the commit you reference is a mix of code and stylistic changes, which
makes it hard to review, especially in tests. Could you please work on
that separation?

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:4>

Django

unread,

Oct 21, 2014, 1:31:26 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Component: Core (Management | Resolution:
commands) | Triage Stage: Ready for
Severity: Normal | checkin

Keywords: test, tdd, | Needs documentation: 0
management, commands | Patch needs improvement: 0

Has patch: 1 | UI/UX: 0

Needs tests: 0 |
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by timgraham):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:5>

Django

unread,

Oct 21, 2014, 1:58:16 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Comment (by loic):

@berkerpeksag patching `stderr/stdout` as suggested doesn't allow setting
`no_color` which is very useful in tests.

@claudep sorry I didn't notice your comment earlier. Yes the stylistic
changes belonged to another commit, it's now done. I was mostly using the
PR for CI which gave me a really bad time (turns out Jenkin's doesn't run
in a TTY). I can still make a PR just for this fix if you want.

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:6>

Django

unread,

Oct 21, 2014, 2:16:22 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Changes (by loic):

* cc: loic (added)

Comment:

PR https://github.com/django/django/pull/3402.

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:7>

Django

unread,

Oct 21, 2014, 3:14:50 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: 1.7

Comment (by claudep):

Loïc, thanks, that's easier to review!
Traditionally, we have favored two interfaces to management commands: the
command line and `call_command`. I understand the need for fine-grained
unit testing, I just hope this won't entice users to run commands in new
"imaginary" ways... Whatever, go for it!

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:8>

Django

unread,

Oct 21, 2014, 10:31:53 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody

Type: | Status: closed
Cleanup/optimization | Version: 1.7
Component: Core (Management | Resolution: fixed

Changes (by Loic Bistuer <loic.bistuer@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"533532302ae842c95cf7294ef6cd7f3e2bfaca65"]:
{{{
#!CommitTicketReference repository=""
revision="533532302ae842c95cf7294ef6cd7f3e2bfaca65"
Fixed #23663 -- Initialize output streams for BaseCommand in __init__().

This helps with testability of management commands.

Thanks to trac username daveoncode for the report and to
Tim Graham and Claude Paroz for the reviews.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:9>

Django

unread,

Oct 21, 2014, 10:31:53 PM10/21/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody

Type: | Status: closed
Cleanup/optimization | Version: 1.7

Comment (by Loic Bistuer <loic.bistuer@…>):

In [changeset:"494ba051bb0d3ebbdbea7598251b1ee6fe9b69b4"]:
{{{
#!CommitTicketReference repository=""
revision="494ba051bb0d3ebbdbea7598251b1ee6fe9b69b4"
Made testing of stdout and stderr more consistent.

Refs #23663.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:10>

Django

unread,

Oct 22, 2014, 12:28:08 AM10/22/14

to django-...@googlegroups.com

#23663: Commands classes are not properly testable due to their initialization in
execute() method!
-------------------------------------+-------------------------------------
Reporter: daveoncode | Owner: nobody

Type: | Status: closed
Cleanup/optimization | Version: 1.7

Comment (by Loic Bistuer <loic.bistuer@…>):

In [changeset:"eb82fb0a9d14ca317d366afb65e21d742bbe46a5"]:
{{{
#!CommitTicketReference repository=""
revision="eb82fb0a9d14ca317d366afb65e21d742bbe46a5"
Refactored color_style() and no_style() to improve testability. Refs
#23663.

This includes the following improvements:

- The type of the style object is now called 'Style' rather than 'dummy'.
- The new make_style() function allows generating a Style object directly
from a config string. Before the only way to get a style object was
through the environ and it also required that the terminal supported
colors which isn't necessarily the case when testing.
- The output of no_style() is now cached with @lru_cache.
- The output of no_style() now has the same set of attributes as the
other Style objects. Previously it allowed anything to pass through
with __getattr__.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23663#comment:11>

Reply all

Reply to author

Forward