Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed #27600 -- Improved the way the 'shell' command reads python code from stdin. #7681

Closed
wants to merge 9 commits into from
Closed

Fixed #27600 -- Improved the way the 'shell' command reads python code from stdin. #7681

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fb2dd9a
Fixed #27600 -- Improved how the 'shell' command reads from stdin.
jpic Dec 13, 2016
fd30f66
chopblankline
jpic Dec 20, 2016
b69a946
win32
jpic Dec 20, 2016
66e9814
fixup! win32
jpic Dec 20, 2016
2128a76
tests skip win32
jpic Dec 20, 2016
c3c851b
Rewording for shell Command.help
jpic Dec 20, 2016
1d18ef4
Precise skipIf win32
jpic Dec 20, 2016
8f47b1e
fixup! Precise skipIf win32
jpic Dec 21, 2016
2136c3d
Add docs/ref/django-admin.txt
jpic Dec 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/shell/tests.py
View file
Open in desktop
@@ -1,3 +1,5 @@
import sys

from django import __version__
from django.core.management import call_command
from django.test import SimpleTestCase, mock
Expand All @@ -18,6 +20,7 @@ def test_command_option(self):
self.assertEqual(len(logger), 1)
self.assertEqual(logger[0], __version__)

@unittest.skipIf(sys.platform == 'win32', "Python on Windows doesn't have working select().")
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not exactly true, its select() works, but not on file objects like stdin

@mock.patch('django.core.management.commands.shell.select')
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these aren't necessary, you can pass StringIOs for stdout and stdin to call_command instead. there are lots of examples in existing tests e.g. https://github.com/django/django/blob/master/tests/i18n/test_compilation.py#L41

Copy link
Contributor Author

@jpic jpic Dec 16, 2016 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how we could possibly pass an instance of OutputWrapper to exec(), so passing stdout to call_command has just no effect.

For stdin, I did the same hack as in createsuperuser so it's more consistent. But if you want my own opinion, I would discourage adding production code that's supposed to be run only in the test environment, I thought this was commonly known as a bad practice but am not sure anymore.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha I found what you should be using for both: django.utils.captured_stdin and django.utils.captured_stdout 😉

You could also open a second PR removing the hack from createsuperuser and change its tests to use captured_stdin 👍 Presumably

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh right, thanks a heap Adam, it works great 🎸

def test_stdin_read(self, select):
with captured_stdin() as stdin, captured_stdout() as stdout:
Expand Down