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.

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

Closed
wants to merge 4 commits into from
Closed

bpo-18576: Add test.support.script_helper documentation #1252

wants to merge 4 commits into from

Conversation

louisom
Copy link
Contributor

@louisom louisom commented Apr 22, 2017

Based on bobcatfist's patch, addressed on Martin request and some minor change.

@mention-bot
Copy link

@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 Show resolved Hide resolved
Doc/library/test.rst Outdated Show resolved Hide resolved

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.
Copy link
Member

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.


.. function:: spawn_python(*args, **kw)

Runs a Python subprocess with the given arguments and returns the
Copy link
Member

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.

Copy link
Contributor Author

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 Show resolved Hide resolved
Doc/library/test.rst Outdated Show resolved Hide resolved
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.
Copy link
Member

Choose a reason for hiding this comment

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

of the created file


make_zip_pkg('.', 'example', 'mypkg', 'submodule', '', depth=2)

Would create an archive in the current directory called ``example.zip``
Copy link
Member

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

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.
Copy link
Member

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.

Copy link
Contributor Author

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*

Doc/library/test.rst Outdated Show resolved Hide resolved
*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`.
Copy link
Contributor

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.
Copy link
Contributor

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

@gpshead gpshead added docs Documentation in the Doc dir skip news labels Sep 11, 2018
@gpshead
Copy link
Member

gpshead commented Sep 11, 2018

Please sync the branch with the latest doc version in github and consider DimitrisJim's comments.

@matrixise
Copy link
Member

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.

@brettcannon
Copy link
Member

@louisom did you want to update this to resolve the merge conflict before I review it?

@JulienPalard
Copy link
Member

All those functions has now been documented. Thanks for the pull request in any cases!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants