classification
Title: test the executable arg to Popen() and improve related docs
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: chris.jerdonek Nosy List: Arfrever, asvetlov, chris.jerdonek, kushal.das, python-dev
Priority: normal Keywords: easy, patch

Created on 2012-10-03 06:19 by chris.jerdonek, last changed 2012-10-11 00:59 by chris.jerdonek. This issue is now closed.

Files
File name Uploaded Description Edit
executable_precedence.patch kushal.das, 2012-10-04 05:09 patch which has a new test called test_executable_precedence to test precedence of executable argument over args[0] review
issue-16115-1.patch chris.jerdonek, 2012-10-08 08:27 review
issue-16115-2.patch chris.jerdonek, 2012-10-08 16:13 review
issue-16115-3.patch chris.jerdonek, 2012-10-10 03:26 review
Messages (22)
msg171856 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-03 06:19
The executable argument to Popen() takes precedence over args[0] when the executable argument is provided.  The test suite should include a test of this that runs on all systems.  The test suite does not currently include such a test.  This test is a precursor to more specific, platform-specific tests that will be added for issue 16114.
msg171919 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2012-10-04 05:09
Attached patch which has a new test called test_executable_precedence to test precedence of executable argument over args[0].
msg172123 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-05 19:53
New changeset 0df5aeab229f by Andrew Svetlov in branch '3.3':
Issue #16115: Add test for check that executable arg to Popen() takes precedence over args[0] arg\n                            \n                            Patch by Kushal Das
http://hg.python.org/cpython/rev/0df5aeab229f

New changeset 0fcfb6066e17 by Andrew Svetlov in branch 'default':
Merge issue #16115: Add test for check that executable arg to Popen() takes precedence over args[0] arg\n                            \n                            Patch by Kushal Das
http://hg.python.org/cpython/rev/0fcfb6066e17
msg172124 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-05 20:01
Committed. Thank you, Kushal Das.
BTW, please fill http://www.python.org/psf/contrib/ as contributor of Python project.
We would to get that agreement from everybody who has pushed any patch.
Thanks again.
msg172127 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-05 20:11
Actually, I was still working on this.  I had assigned it to myself.

+        p = subprocess.Popen(["nonexistent","-c",'import sys; sys.exit(42)'],
+                            executable=sys.executable, cwd=python_dir)

The test for the executable argument should really test the executable argument independent of the cwd argument.  Also, it would be better if the test uses a valid args[0] to demonstrate precedence.  I had prepared a test but did not upload it yet, because I also wanted to add an accompanying clarification to the docs which I am still preparing.  Reopening.
msg172132 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-05 20:32
ok, it will be better.
msg172133 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-05 20:35
Thanks, Andrew.  I'm hoping to finish preparing the patch sometime this weekend.
msg172194 - (view) Author: Kushal Das (kushal.das) * (Python committer) Date: 2012-10-06 12:58
On Saturday, October 6, 2012, Andrew Svetlov wrote:

>
> Andrew Svetlov added the comment:
>
> Committed. Thank you, Kushal Das.
> BTW, please fill http://www.python.org/psf/contrib/ as contributor of
> Python project.
> We would to get that agreement from everybody who has pushed any patch.
> Thanks again.

Hi,
I already filled and sent it. Thanks for the commit.

Kushal
msg172366 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-08 08:27
Proposed patch attached.

I verified that the tests included in the patch pass on 2.7, 3.2, 3.3, and default (replacing FileNotFoundError with OSError as necessary).  I also verified that the tests pass on Windows for default.

The patch also includes improvements to the parts of Popen()'s documentation that relate to the added tests.  The improvements simplify the documentation by more clearly separating the argument-specific documentation from the default/general behavior.  Previously, certain general information was under various argument-specific paragraphs which made the documentation less clear.
msg172370 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-08 09:44
I also checked the tests in the patch on Windows for 2.7.
msg172380 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-08 13:39
Hmm, I dislike "Normally, *args* should be a sequence."
From my perspective better to say something like:
*args* should be a sequence if *shell* is *False* or string of *shell* is *True* 
or something like this.
I would to directly recommend reader to follow that schema, see also http://bugs.python.org/issue7839
msg172393 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-08 16:13
Thanks for the good suggestion and pointer to the relevant discussion, Andrew.

I agree re: the sentence beginning with "normally."  Incidentally, that was existing language that I had preserved.

I'm attaching an updated patch that incorporates your suggestion.
msg172394 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2012-10-08 16:22
LGTM. Thanks, Chris.
msg172424 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-08 23:02
New changeset 862430c68fec by Chris Jerdonek in branch '3.3':
Issue #16115: Improve testing of the executable argument to subprocess.Popen().
http://hg.python.org/cpython/rev/862430c68fec

New changeset 1b37fc50dc1b by Chris Jerdonek in branch 'default':
Issue #16115: Merge test improvements from 3.3.
http://hg.python.org/cpython/rev/1b37fc50dc1b
msg172425 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-08 23:03
I am pushing the documentation changes separately (which will include 2.7 and 3.2).
msg172426 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-08 23:19
The test_executable test fails on AMD64 Ubuntu LTS.  For example:

http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.3/builds/38/steps/test/logs/stdio

FAIL: test_executable (test.test_subprocess.ProcessTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 205, in test_executable
    self._assert_python(["doesnotexist", "-c"], executable=sys.executable)
  File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 201, in _assert_python
    self.assertEqual(47, p.returncode)
AssertionError: 47 != -6

======================================================================
FAIL: test_executable (test.test_subprocess.ProcessTestCaseNoPoll)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 205, in test_executable
    self._assert_python(["doesnotexist", "-c"], executable=sys.executable)
  File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_subprocess.py", line 201, in _assert_python
    self.assertEqual(47, p.returncode)
AssertionError: 47 != -6

This test passed locally (at least for me) on Mac OS X and Windows.
msg172429 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-09 00:55
New changeset ef90c5e482f4 by Chris Jerdonek in branch '3.3':
Issue #16115: Skip a newly added subprocess.Popen() test on Linux.
http://hg.python.org/cpython/rev/ef90c5e482f4

New changeset 96b8eb3d3208 by Chris Jerdonek in branch 'default':
Issue #16115: Merge test skip from 3.3.
http://hg.python.org/cpython/rev/96b8eb3d3208
msg172430 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-09 01:03
More information on the failure above (from the same buildbot link):

test_executable (test.test_subprocess.ProcessTestCase) ... Could not find platform independent libraries <prefix>
Could not find platform dependent libraries <exec_prefix>
Consider setting $PYTHONHOME to <prefix>[:<exec_prefix>]
Fatal Python error: Py_Initialize: Unable to get the locale encoding
ImportError: No module named 'encodings'
FAIL
msg172433 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-09 01:43
I created issue 16170 to track fixing the Linux-skip for the test_executable() test.
msg172451 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-09 06:19
New changeset 3dd012397b11 by Chris Jerdonek in branch '3.2':
Issue #16115: Improve subprocess.Popen() documentation around args, shell, and executable arguments.
http://hg.python.org/cpython/rev/3dd012397b11

New changeset 0ef3b801ccbc by Chris Jerdonek in branch '3.3':
Issue #16115: Merge subprocess.Popen() documentation improvements from 3.2.
http://hg.python.org/cpython/rev/0ef3b801ccbc

New changeset 91d4593ac974 by Chris Jerdonek in branch 'default':
Issue #16115: Merge subprocess.Popen() documentation improvements from 3.3.
http://hg.python.org/cpython/rev/91d4593ac974

New changeset 11171bf6a6bd by Chris Jerdonek in branch '2.7':
Issue #16115: Backport subprocess.Popen() documentation improvements from 3.2.
http://hg.python.org/cpython/rev/11171bf6a6bd
msg172550 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-10-10 03:26
I would like to make a few more miscellaneous improvements to the Popen() docs, including a clarification resulting from issue #16170, which stemmed from this issue.  The proposed patch is attached.
msg172611 - (view) Author: Roundup Robot (python-dev) Date: 2012-10-11 00:58
New changeset 41ffed580eef by Chris Jerdonek in branch '3.2':
Issue #16115: Make further improvements to subprocess.Popen() documentation.
http://hg.python.org/cpython/rev/41ffed580eef

New changeset bae644e46556 by Chris Jerdonek in branch '3.3':
Issue #16115: Merge subprocess.Popen() documentation improvements from 3.2.
http://hg.python.org/cpython/rev/bae644e46556

New changeset 178ba363af1c by Chris Jerdonek in branch 'default':
Issue #16115: Merge subprocess.Popen() documentation improvements from 3.3.
http://hg.python.org/cpython/rev/178ba363af1c

New changeset f051e37ac11d by Chris Jerdonek in branch '2.7':
Issue #16115: Backport subprocess.Popen() documentation improvements from 3.2.
http://hg.python.org/cpython/rev/f051e37ac11d
History
Date User Action Args
2015-10-19 01:21:59martin.panterlinkissue13195 dependencies
2012-10-11 00:59:13chris.jerdoneksetstatus: open -> closed
resolution: fixed
stage: resolved
2012-10-11 00:58:20python-devsetmessages: + msg172611
2012-10-10 03:26:15chris.jerdoneksetstatus: closed -> open
files: + issue-16115-3.patch
title: test that executable arg to Popen() takes precedence over args -> test the executable arg to Popen() and improve related docs
messages: + msg172550

resolution: fixed -> (no value)
stage: resolved -> (no value)
2012-10-09 06:22:28chris.jerdoneksetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-10-09 06:19:48python-devsetmessages: + msg172451
2012-10-09 01:43:31chris.jerdoneksetmessages: + msg172433
2012-10-09 01:03:26chris.jerdoneksetmessages: + msg172430
2012-10-09 00:55:12python-devsetmessages: + msg172429
2012-10-08 23:19:22chris.jerdoneksetmessages: + msg172426
2012-10-08 23:03:52chris.jerdoneksetmessages: + msg172425
2012-10-08 23:02:16python-devsetmessages: + msg172424
2012-10-08 16:22:15asvetlovsetmessages: + msg172394
2012-10-08 16:13:16chris.jerdoneksetfiles: + issue-16115-2.patch

messages: + msg172393
stage: test needed -> patch review
2012-10-08 13:39:52asvetlovsetmessages: + msg172380
2012-10-08 09:44:09chris.jerdoneksetmessages: + msg172370
2012-10-08 08:27:40chris.jerdoneksetfiles: + issue-16115-1.patch

messages: + msg172366
versions: + Python 2.7, Python 3.2
2012-10-06 12:58:39kushal.dassetmessages: + msg172194
title: test that executable arg to Popen() takes precedence over args[0] arg -> test that executable arg to Popen() takes precedence over args
2012-10-05 20:35:29chris.jerdoneksetmessages: + msg172133
2012-10-05 20:32:52asvetlovsetmessages: + msg172132
2012-10-05 20:11:11chris.jerdoneksetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg172127

stage: resolved -> test needed
2012-10-05 20:01:38asvetlovsetstatus: open -> closed
resolution: fixed
stage: test needed -> resolved
2012-10-05 20:01:18asvetlovsetmessages: + msg172124
2012-10-05 19:53:23python-devsetnosy: + python-dev
messages: + msg172123
2012-10-04 05:09:15kushal.dassetfiles: + executable_precedence.patch

nosy: + kushal.das
messages: + msg171919

keywords: + patch
2012-10-03 12:51:14Arfreversetnosy: + Arfrever
2012-10-03 06:23:32chris.jerdoneklinkissue16114 dependencies
2012-10-03 06:19:47chris.jerdonekcreate