[issue18576] Document test.support.script_helper

[Julien Palard]

Julien Palard  added the comment:

All those functions has already been documented.

--
dependencies:  -Make test.script_helper more comprehensive, and use it in the 
test suite
nosy: +mdk
resolution:  -> out of date
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Louie Lu]

Louie Lu added the comment:

The PR on GitHub is based on bobcatfist's patch, addressed on Martin request 
and some minor change.

--
nosy: +louielu

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Louie Lu]

Changes by Louie Lu :


--
pull_requests: +1367

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Martin Panter]

Martin Panter added the comment:

I left some more comments on Reitveld. The main one is I think stderr gets 
stripped with assert_python_ok(), but I don’t think stdout is touched. Correct 
me if I’m wrong though.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Nick Coghlan]

Changes by Nick Coghlan ncogh...@gmail.com:


--
assignee: ncoghlan - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Christie]

Christie added the comment:

Hey @vadmium!

I've finally responded to your review feedback. I've attached a new patch, 
hopefully this time actually review-able. I've removed the docs for methods I 
added in @24033 so that this could be merged without it.

Let me know if you have any more feedback, or if this could be merged.

Thanks!

--
Added file: http://bugs.python.org/file39827/issue18576.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Christie]

Christie added the comment:

 If you diffed your patch from a public revision in the main repository, there 
 should be a nice “Review” link.

Ah, thanks @vadmium!!! I've been confused about why some patches get review 
links and others do not .

I'll have a new patch up shortly which is diffed against a public revision and 
has your suggested changes.

 For autodoc in particular, we prefer to avoid it for the main docs because 
 good docstring and good prose documentation don't necessarily look alike. 

@ncoghlan for some reason I thought that the main docs were also auto generated 
from docstrings in the actual modules, but I can see now that that isn't the 
case.

I'll reach out to d...@python.org to see how they feel about autogenerating the 
test.support docs.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Christie]

Christie added the comment:

@ncoghlan I've taken the test.rst changes from the previous diff and reviewed 
them against the latest script_helper (including updates from issue24033). 
Nothing had fallen behind, but I've added what was missing.

* I've realized that spawn_python and the helper run_python I added in 
issue24033 are basically the same so I will reconcile those
* Is there some way to load the docstrings from script_helper.py in test.rst 
instead of writing explanations for each method twice?

--
Added file: http://bugs.python.org/file39645/issue18576.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Martin Panter]

Martin Panter added the comment:

If you diffed your patch from a public revision in the main repository, there 
should be a nice “Review” link. Anyway, here are some comments on 
issue18576.patch:

+.. function:: assert_python_ok(*args, **env_vars)
+
+   Runs the interpreter with *args* and optional environment
+   variables, asserting that *env_vars* succeeds (``rc == 0``).

Perhaps you mean *env_vars* are the optional environment variables, and that 
the interpreter is asserted to have succeeded.

Also maybe clarify where the *__cleanenv* and *__isolated* keywords come from? 
Function signatures? Also applies to assert_python_failure(), 
run_python/_until_end().

Maybe it is worth mentioning that assert_python_ok/failure() strip whitespace 
from the stderr stream. I seem to remember being tricked by this (writing a 
test to ensure stderr ended in a newline or something).

+   ``stdout`` are configured as binary pipes and ``stderr`` is merged with
+   ``stdout``..

There are two full stops above here..

I encourage you to start sentences with capital letters (although I admit 
sometimes other people do not agree with this). Suggestions:

+   *kw* is passed through to subprocess.Popen as additional keyword
+   arguments.
= The *kw* arguments are passed through to subprocess.Popen.

+   *source* is a Unicode string which will be written to the file as UTF-8.
= The *source* argument is a Unicode string which will be written to the file 
as UTF-8.

+   *depth* controls how many deeply nested the package is.
= The *depth* argument controls how deeply nested the package is.

+   Creates a new zip archive in the given directory, containing the specified
+   Python script/module

Needs one of those full stops I mentioned earlier :)

--
nosy: +vadmium

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Nick Coghlan]

Nick Coghlan added the comment:

As far as using docstrings for the online docs goes, there are some useful 
Sphinx add-ons that we've never enabled for the main Python docs (like autodoc, 
blockdiag, seqdiag) that could be good options to have available. For autodoc 
in particular, we prefer to avoid it for the main docs because good docstring 
and good prose documentation don't necessarily look alike. For ease of 
maintenance of test.support though, I think the write a good docstring, get 
the online docs for free rationale is a compelling one (since these docs are 
core contributor facing, rather than being aimed at Python users in general)

d...@python.org would be the place to ask about the appropriateness of enabling 
some Sphinx add-ons. While enabling them seems like a reasonable idea to me, I 
don't personally know if there are actually technical issues with the idea, or 
if there just hasn't been a volunteer to add/enable them and update 
https://docs.python.org/devguide/documenting.html accordingly.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Christie]

Christie added the comment:

I'm going to take a look at bringing these docs up to date and including the 
changes from issue24033.

--
nosy: +bobcatfish

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Nick Coghlan]

Nick Coghlan added the comment:

script_helper was moved to the test.support package in issue 9517, so this 
issue is just about adding the docs now.

--
dependencies: +Make test.script_helper more comprehensive, and use it in the 
test suite -Refactor the test_runpy walk_package support code into a common 
location
title: Rename and document test.script_helper as test.support.script_helper - 
Document test.support.script_helper
versions: +Python 3.5 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue18576] Document test.support.script_helper

[Nick Coghlan]

Nick Coghlan added the comment:

Also noting that my draft docs are over a year old now, so they need to be 
reviewed against the current state of the helper module.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18576
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com