-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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.
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
bpo-18576: Add test.support.script_helper documentation #1252
Conversation
@lulouie, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @ncoghlan and @florentx to be potential reviewers. |
Doc/library/test.rst
Outdated
|
||
Returns a (return code, stdout, stderr) tuple on failure, throws an | ||
appropriate :exc:`AssertionError` if the return code is zero containing | ||
stdout and stderr of the failed process. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion failed, but saying the process failed is confusing. Maybe just drop failed. (Unless this last fragment refers to the tuple return value.) I would rewrite as something like
If the exit status is zero, the function raises an :exc:`AssertionError`
, which includes the *stdout*
and *stderr*
from the interpreter. Otherwise, the function returns an (*exit status*
, *stdout*
, *stderr*
) tuple.
Doc/library/test.rst
Outdated
|
||
.. function:: spawn_python(*args, **kw) | ||
|
||
Runs a Python subprocess with the given arguments and returns the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change Runs to Spawns or Starts, to avoid implying it waits for the interpreter to exit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to Spawns
Doc/library/test.rst
Outdated
By default, *script_dir* and *script_basename* are combined with | ||
:data:`os.extsep` and the normal ``py`` extension to create the full | ||
path to the file. If *omit_suffix* is true, the base name is used | ||
directly as the name of created file with no extension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
of the created file
Doc/library/test.rst
Outdated
|
||
make_zip_pkg('.', 'example', 'mypkg', 'submodule', '', depth=2) | ||
|
||
Would create an archive in the current directory called ``example.zip`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lowercase would; it’s not the start of a sentence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Doc/library/test.rst
Outdated
The init modules in the packages inside the archive are always empty, | ||
while *source* is written to the submodule named by *script_basename*. | ||
|
||
The *depth* argument controls how many deeply nested the package is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
. . . controls how deeply nested the package is.
or
. . . controls how many packages the submodule is nested inside.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to "controls how deeply nested the package is*
*env_vars*. Pipes are created for stdout, stdin and stderr. | ||
|
||
Waits until the process completes and returns the runcode, stdout and | ||
stderr as a `_PythonRunResult`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns a tuple containing the _PythonRunResult
and the command executed (cmd_line
).
status is zero (for :func:`assert_python_failure`), the function raises an | ||
:exc:`AssertionError`, which includes the *stdout* and *stderr* from the | ||
interpreter. Otherwise, the function returns an (*exit status*, *stdout*, | ||
*stderr*) tuple. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these both return a _PythonRunResult
namedtuple as obtained from run_python_until_end
Please sync the branch with the latest doc version in github and consider DimitrisJim's comments. |
Hi @louisom Thank you for your contribution. Since the commit 988fb28 some functions have been documented, I propose to you to update your code with the new documentation and submit it again. Thank you. |
@louisom did you want to update this to resolve the merge conflict before I review it? |
All those functions has now been documented. Thanks for the pull request in any cases! |
Based on
bobcatfist
's patch, addressed on Martin request and some minor change.