-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-27200: fix pathlib, ssl, turtle and weakref doctests #616
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
Conversation
@marco-buttu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @eliben, @birkenfeld, @vsajip, @tiran and @loewis to be potential reviewers. |
The setcontext(Context()) in testsetup does not guarantee that the tests outside this file have a clean context. We need to clean the context at the end of the tests.
Doc/library/decimal.rst
Outdated
|
||
# make sure other tests (also outside this file) get a fresh context | ||
setcontext(Context()) | ||
|
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 setcontext(Context())
in testsetup
does not guarantee that the tests outside this file get a fresh context. We need to clean the context at the end of the tests.
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.
Please leave decimal.rst changes out of this PR and open a separate PR and then ping Stefan Krah.
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.
I think we also need to following change to ssl examples:
diff --git a/Doc/library/ssl.rst b/Doc/library/ssl.rst
index 01dbecd..02a0c79 100644
--- a/Doc/library/ssl.rst
+++ b/Doc/library/ssl.rst
@@ -2262,7 +2262,7 @@ SSL versions 2 and 3 are considered insecure and are therefore dangerous to
use. If you want maximum compatibility between clients and servers, it is
recommended to use :const:`PROTOCOL_TLS_CLIENT` or
:const:`PROTOCOL_TLS_SERVER` as the protocol version. SSLv2 and SSLv3 are
-disabled by default.
+disabled by default. ::
>>> client_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
>>> client_context.options |= ssl.OP_NO_TLSv1
And there are the other failures:
**********************************************************************
File "library/ssl.rst", line 421, in newcontext
Failed example:
timestamp
Expected:
1515144883
Got:
1515134083.0
**********************************************************************
File "library/ssl.rst", line 424, in newcontext
Failed example:
print(datetime.utcfromtimestamp(timestamp))
Expected:
2018-01-05 09:34:43
Got:
2018-01-05 06:34:43
Doc/library/decimal.rst
Outdated
|
||
# make sure other tests (also outside this file) get a fresh context | ||
setcontext(Context()) | ||
|
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.
Please leave decimal.rst changes out of this PR and open a separate PR and then ping Stefan Krah.
Doc/library/shlex.rst
Outdated
@@ -43,15 +43,16 @@ The :mod:`shlex` module defines the following functions: | |||
string that can safely be used as one token in a shell command line, for | |||
cases where you cannot use a list. | |||
|
|||
This idiom would be unsafe:: | |||
This idiom would be unsafe: |
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.
You have already modified this file in #604.
Doc/library/pathlib.rst
Outdated
@@ -46,7 +46,7 @@ Pure paths are useful in some special cases; for example: | |||
Basic use | |||
--------- | |||
|
|||
Importing the main class:: | |||
Importing the main class: |
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.
Please revert this change. The current version looks correct to me and the tests passed when I revert it to use ::.
Thank you very much for your time @berkerpeksag! I made all changes you requested me. Just one question: you got a |
I don't know, it's probably worth doing some VCS archaeology :) I built and ran tests in Python 3.4 with an ancient OpenSSL so this might be the culprit. |
Doc/library/ssl.rst
Outdated
@@ -2257,7 +2262,7 @@ SSL versions 2 and 3 are considered insecure and are therefore dangerous to | |||
use. If you want maximum compatibility between clients and servers, it is | |||
recommended to use :const:`PROTOCOL_TLS_CLIENT` or | |||
:const:`PROTOCOL_TLS_SERVER` as the protocol version. SSLv2 and SSLv3 are | |||
disabled by default. | |||
disabled by default. :: |
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.
I know I suggested this style in my earlier comment, but could you move :: to a new line so we don't lose git blame
information? This also applies to the changes in Doc/library/pathlib.rst.
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.
Ok, done :-) Thanks againt @berkerpeksag
Thanks! |
This PR is the last but one in the series bpo-27200. The first one was #240, the second one #401, and the third #604. To run the doctests (from the
Doc
directory):