Skip to content

bpo-32843: Additional changes to test.support docs #5774

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
Closed
wants to merge 1 commit into from
Closed

bpo-32843: Additional changes to test.support docs #5774

wants to merge 1 commit into from

Conversation

csabella
Copy link
Contributor

@csabella csabella commented Feb 20, 2018 •
edited by bedevere-bot
Loading

@matrixise matrixise added the docs Documentation in the Doc dir label May 15, 2019
@matrixise matrixise requested a review from vstinner May 15, 2019 21:19
vstinner
vstinner approved these changes May 17, 2019
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. Just the disable_faulthandler() doc is wrong.

Doc/library/test.rst
A context manager that uses ``sys.__stderr__`` instead of
:data:`sys.stderr`, since :mod:`test.regrtest` replaces :data:`sys.stderr`
with a :class:`io.StringIO` which has no file descriptor when a test is run
with ``-W/--verbose3``.
Copy link
Member

Choose a reason for hiding this comment

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

Well, no. The purpose of this context manager is to disable temporarily faulthandler.

@matrixise
Copy link
Member

@csabella Could you update your PR? Thanks

JulienPalard
JulienPalard requested changes Sep 13, 2019
View reviewed changes
Doc/library/test.rst
@@ -249,7 +265,11 @@ The :mod:`test.support` module defines the following constants:

.. data:: FS_NONASCII

A non-ASCII character encodable by :func:`os.fsencode`.
A non-ASCII character encodable by :func:`os.fsencode`. The function
Copy link
Member

Choose a reason for hiding this comment

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

"The function" may be a bit opaque here. Maybe replace The function by Choosen at import time (which breaks the rest of the sentence)?

Copy link
Member

Choose a reason for hiding this comment

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

I think the extra text is unnecessary here; we don't need to explain how the object is derived, only what it's for.

Doc/library/test.rst
@@ -260,7 +280,7 @@ The :mod:`test.support` module defines the following constants:

.. data:: TESTFN_UNICODE

Set to a non-ASCII name for a temporary file.
Set to a non-ASCII filename containing several unicode characters.
Copy link
Member

Choose a reason for hiding this comment

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

Looks redundent to me (non-ASCII and several unicode). Also an "unicode character" may be one in the ASCII table.

Maybe "Set to a non-ASCII filename." is enough? In contrast this one is not tested against being fsencodable.

Doc/library/test.rst

This makes a copy of :data:`sys.path`, appends any directories given
as positional arguments, then reverts :data:`sys.path` to the copied
This makes a copy of ``sys.path``, appends any directories given
Copy link
Member

Choose a reason for hiding this comment

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

:data:`sys.path` was creating a link to it, ``sys.path`` won't, why changing it? (multiple occurrences following)

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@JulienPalard
Copy link
Member

@csabella Hi Cheryl, long time no see, what about this PR? It accumulated a few conflicts :-(

JelleZijlstra
JelleZijlstra reviewed May 8, 2022
View reviewed changes
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

This unfortunately has a lot of conflicts now, enough that it's easier to start from scratch. I'll close this and open a new

Doc/library/test.rst
@@ -225,6 +225,22 @@ This module defines the following exceptions:

The :mod:`test.support` module defines the following constants:


.. data:: HOST
Copy link
Member

Choose a reason for hiding this comment

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

This no longer exists on main

Doc/library/test.rst
@@ -249,7 +265,11 @@ The :mod:`test.support` module defines the following constants:

.. data:: FS_NONASCII

A non-ASCII character encodable by :func:`os.fsencode`.
A non-ASCII character encodable by :func:`os.fsencode`. The function
Copy link
Member

Choose a reason for hiding this comment

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

I think the extra text is unnecessary here; we don't need to explain how the object is derived, only what it's for.

Doc/library/test.rst
@@ -334,12 +351,6 @@ The :mod:`test.support` module defines the following constants:
Set to :data:`sys.maxsize` for big memory tests.


.. data:: max_memuse
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to keep this documented for symmetry with real_max_memuse.

Doc/library/test.rst

Return the original stdout set by :func:`record_original_stdout` or
``sys.stdout`` if it's not set.


.. function:: strip_python_strerr(stderr)
.. function:: strip_python_stderr(stderr)
Copy link
Member

Choose a reason for hiding this comment

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

This was removed in 3.9.0a2

@JelleZijlstra JelleZijlstra mentioned this pull request May 8, 2022
Merged
JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this pull request May 8, 2022
@JelleZijlstra @csabella
pythongh-77024: test.support: Improve documentation
81c870d 
This is a rework of python#5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue python#77024 and the discussion
on python#5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
JelleZijlstra added a commit that referenced this pull request May 27, 2022
@JelleZijlstra @csabella @AlexWaygood
gh-77024: test.support: Improve documentation (#92513)
8995177 
This is a rework of #5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue #77024 and the discussion
on #5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
@miss-islington miss-islington mentioned this pull request May 27, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 27, 2022
@JelleZijlstra @miss-islington
pythongh-77024: test.support: Improve documentation (pythonGH-92513)
0af3eeb 
This is a rework of pythonGH-5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue pythonGH-77024 and the discussion
on pythonGH-5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 8995177)

Co-authored-by: Jelle Zijlstra <[email protected]>
@miss-islington miss-islington mentioned this pull request May 27, 2022
Merged
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 27, 2022
@JelleZijlstra @miss-islington
pythongh-77024: test.support: Improve documentation (pythonGH-92513)
70e49cb 
This is a rework of pythonGH-5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue pythonGH-77024 and the discussion
on pythonGH-5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 8995177)

Co-authored-by: Jelle Zijlstra <[email protected]>
miss-islington added a commit that referenced this pull request May 27, 2022
@miss-islington @JelleZijlstra
gh-77024: test.support: Improve documentation (GH-92513)
34bd36c 
This is a rework of GH-5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue GH-77024 and the discussion
on GH-5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 8995177)

Co-authored-by: Jelle Zijlstra <[email protected]>
miss-islington added a commit that referenced this pull request May 27, 2022
@miss-islington @JelleZijlstra
gh-77024: test.support: Improve documentation (GH-92513)
9912b3d 
This is a rework of GH-5774 on current main. I was a bit more
conservative in making changes than the original PR.

See @csabella's comments on issue GH-77024 and the discussion
on GH-5774 for explanations of several of the changes.

Co-authored-by: Cheryl Sabella <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit 8995177)

Co-authored-by: Jelle Zijlstra <[email protected]>
@csabella csabella mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@JulienPalard JulienPalard JulienPalard requested changes

@vstinner vstinner vstinner approved these changes

Assignees
No one assigned
Labels
awaiting changes docs Documentation in the Doc dir skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@csabella @matrixise @bedevere-bot @JulienPalard @vstinner @JelleZijlstra @the-knights-who-say-ni