GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-108303: Move all doctest related files and tests to `Lib/test/test_doctest/` by sobolevn · Pull Request #112109 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Nov 15, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Move test files into test subdirectories #108303

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sobolevn
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
49a6473
…t/test_doctest/`
@sobolevn
sobolevn requested a review from vstinner November 15, 2023 07:56
@bedevere-app bedevere-app Bot added the awaiting review label Nov 15, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Nov 15, 2023
Move test files into test subdirectories #108303
Closed
@sobolevn sobolevn added skip news needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Nov 15, 2023
@sobolevn
Fix CI
ca8a1f6
@sobolevn
sobolevn requested review from brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners November 15, 2023 10:33
@sobolevn
Fix CI
5f731f5
vstinner
vstinner reviewed Nov 15, 2023
View reviewed changes
Comment thread Lib/test/support/pty_helper.py
self.lines = lines

def readline(self):
line = self.lines.pop(0)

@vstinner vstinner Nov 15, 2023

Copy link
Copy Markdown
Member

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.

Calling list.pop(0) frequently is inefficient. Maybe the constructor should create a collections.deque() and call self.lines.popleft() instead?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sobolevn sobolevn Nov 15, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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 is usually used in cases like sys.stdin = FakeInput(['next', 'print(x)', 'continue']), I don't think it matters much. But, I can do this if you want :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread Makefile.pre.in Resolved

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon
brettcannon removed their request for review November 15, 2023 21:06
brettcannon
brettcannon approved these changes Nov 15, 2023
View reviewed changes
Comment thread Lib/test/support/pty_helper.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/support/pty_helper.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Nov 15, 2023
sobolevn and others added 2 commits November 16, 2023 00:32
@sobolevn @brettcannon
Apply suggestions from code review
2774675
Co-authored-by: Brett Cannon <[email protected]>
@sobolevn
Merge branch 'main' into issue-108303-test_doctest
33c801b
@sobolevn

sobolevn commented Dec 2, 2023

Copy link
Copy Markdown
Member Author

I solved merge conflicts, cc @vstinner

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Dec 4, 2023

Copy link
Copy Markdown
Member

Since we are moving code, would it be worth it to split test_doctest.py which has 3 374 lines into multiple files? I found the following tests / test cases:

  • SampleClass
  • sample_func
  • SampleNewStyleClass
  • test_CLI
  • test_debug
  • test_DocFileSuite
  • test_DocTest
  • test_DocTestFinder
  • test_DocTestParser
  • test_DocTestRunner
  • test_DocTestSuite
  • test_Example
  • test_exception_with_multiple_notes
  • test_exception_with_note
  • test_lineendings
  • test_look_in_unwrapped
  • test_no_trailing_whitespace_stripping
  • test_pdb_set_trace
  • test_pdb_set_trace_nested
  • test_run_doctestsuite_multiple_times
  • test_syntax_error_subclass_from_stdlib
  • test_syntax_error_with_incorrect_expected_note
  • test_syntax_error_with_note
  • test_testfile
  • test_testmod
  • test_testsource
  • test_trailing_space_in_test
  • test_unicode
  • test_unittest_reportflags

For example, it would be nice to have 1000 lines or less per file. If you are interested to make such change, I suggest creating a new PR, so we can compare.

About test_doctest2: why is it a separated file? Should it be renamed? The docstring says:

A module to test whether doctest recognizes some 2.2 features, like static and class methods.

I suppose that 2.2 stands for Python 2.2.

Is it a test on the "parser" or "test discovery"?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn sobolevn mentioned this pull request Dec 22, 2023
gh-113360: Fix doctest failure when __test__ is set to falsy value #113389
Closed
@sobolevn

sobolevn commented Dec 22, 2023

Copy link
Copy Markdown
Member Author

Since we are moving code, would it be worth it to split test_doctest.py which has 3 374 lines into multiple files?

I don't think so, because moving a file is a rather simple operation. It does not spoil git blame. While splitting files can be devastating to its history.

I work with test_doctest quite a lot, I don't think that it has maintability problems.

About test_doctest2: why is it a separated file? Should it be renamed?

I agree that the naming of this file is not optimal. It also clashes with test_doctest2.txt, I don't think that this is intentional.

Looks like that all tests from it are just duplicates. We already test classes, classmethods, staticmethods and everything else in test_doctest. So, it can be simply removed.

It has some details that I want to explicitly port:

  • property with a setter
  • poluting namespace with other objects
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sobolevn
Address review, merge main
a13b649
@sobolevn
sobolevn requested a review from vstinner December 22, 2023 08:19
vstinner
vstinner reviewed Jan 10, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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 cannot see where Lib/test/test_doctest2.py test goes. Did you just remove it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sobolevn

sobolevn commented Jan 11, 2024

Copy link
Copy Markdown
Member Author

Yes:

I agree that the naming of this file is not optimal. It also clashes with test_doctest2.txt, I don't think that this is intentional.

Looks like that all tests from it are just duplicates. We already test classes, classmethods, staticmethods and everything else in test_doctest. So, it can be simply removed.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon
brettcannon requested a review from vstinner January 12, 2024 19:12
@vstinner

vstinner commented Jan 16, 2024

Copy link
Copy Markdown
Member

Looks like that all tests from it are just duplicates. We already test classes, classmethods, staticmethods and everything else in test_doctest. So, it can be simply removed.

I'm not convinced that test_doctest2 is useless. It calls doctest.testmod(sys.modules[__name__]) and test_doctest2.py contains 2 non-ASCII docstrings. Example:

"""...
We include some (random) encoded (utf-8) text in the text surrounding
the example.  It should be ignored:

ЉЊЈЁЂ
"""

It runs tests in verbose mode instead of running them in quiet mode:

  • test_doctest: doctest.testmod(unicodedata, verbose=False) (quiet)
  • test_doctest2: doctest.testmod(sys.modules[__name__]) (verbose)

Even we agree that test_doctest2 is useless, I would prefer to not include this silent removal in the middle of such huge refactoring (move all tests code).

Please add back test_doctest2.py in the new test_doctest/ directory.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn added 2 commits January 18, 2024 11:24
@sobolevn
Merge branch 'main' into issue-108303-test_doctest
a9342bc
@sobolevn
Add test_doctest2.py back
de38766
@vstinner
vstinner merged commit 9c93350 into python:main Jan 18, 2024
@miss-islington-app

miss-islington-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

Thanks @sobolevn for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 18, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 18, 2024
@sobolevn @brettcannon @miss-islington
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
96e1cde
…t/test_doctest/` (pythonGH-112109)

(cherry picked from commit 9c93350)

Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
@bedevere-app

bedevere-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

GH-114254 is a backport of this pull request to the 3.12 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app

miss-islington-app Bot commented Jan 18, 2024

Copy link
Copy Markdown

Sorry, @sobolevn and @vstinner, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 9c93350f582fe6f5fed2cd873869dfe4fbf2dfe8 3.11
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 18, 2024
@miss-islington-app miss-islington-app Bot assigned vstinner Jan 18, 2024
@vstinner

vstinner commented Jan 18, 2024

Copy link
Copy Markdown
Member

Merged, thanks.

@sobolevn: The automated backport to 3.11 failed, do you want to attempt to backport the change to 3.11 manually?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner pushed a commit that referenced this pull request Jan 18, 2024
@miss-islington @sobolevn @brettcannon
[3.12] gh-108303: Move all doctest related files and tests to `Lib/te…
a76d010
…st/test_doctest/` (GH-112109) (#114254)

gh-108303: Move all doctest related files and tests to `Lib/test/test_doctest/` (GH-112109)
(cherry picked from commit 9c93350)

Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: Brett Cannon <[email protected]>
@sobolevn

sobolevn commented Jan 18, 2024

Copy link
Copy Markdown
Member Author

Sure thing!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon

brettcannon commented Jan 19, 2024

Copy link
Copy Markdown
Member

FYI this broke the WASI builds; https://buildbot.python.org/all/#/builders/1046/builds/4033 and https://buildbot.python.org/all/#/builders/1124/builds/623 .

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@brettcannon brettcannon mentioned this pull request Jan 19, 2024
test_pdb broken under WASI #114275
Closed
sobolevn added a commit to sobolevn/cpython that referenced this pull request Jan 19, 2024
@sobolevn
pythongh-114275: Revert "pythongh-108303: Move all doctest related fi…
c3701e9
…les and tests to `Lib/test/test_doctest/` (python#112109)"

This reverts commit 9c93350.
sobolevn added a commit to sobolevn/cpython that referenced this pull request Jan 19, 2024
@sobolevn
Revert "pythongh-114275: Revert "pythongh-108303: Move all doctest re…
973510f
…lated files and tests to `Lib/test/test_doctest/` (python#112109)""

This reverts commit c3701e9.
@vstinner

vstinner commented Jan 19, 2024

Copy link
Copy Markdown
Member

FYI this broke the WASI builds; https://buildbot.python.org/all/#/builders/1046/builds/4033 and https://buildbot.python.org/all/#/builders/1124/builds/623 .

Fixed by efb81a6 of #114275

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sobolevn added a commit to sobolevn/cpython that referenced this pull request Jan 19, 2024
@sobolevn
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
6f06346
…t/test_doctest/` (pythonGH-112109)
@bedevere-app

bedevere-app Bot commented Jan 19, 2024

Copy link
Copy Markdown

GH-114313 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.11 only security fixes label Jan 19, 2024
vstinner pushed a commit that referenced this pull request Jan 19, 2024
@sobolevn
[3.11] gh-108303: Move all doctest related files and tests to `Lib/te…
a4ad7a0
…st/test_doctest/` (GH-112109) (#114313)

gh-108303: Move all doctest related files and tests to `Lib/test/test_doctest/` (GH-112109)
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
@sobolevn @brettcannon @kulikjak
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
808a427
…t/test_doctest/` (python#112109)

Co-authored-by: Brett Cannon <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@sobolevn @brettcannon @aisk
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
7733692
…t/test_doctest/` (python#112109)

Co-authored-by: Brett Cannon <[email protected]>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@sobolevn @brettcannon @Glyphack
pythongh-108303: Move all doctest related files and tests to `Lib/tes…
bce5c23
…t/test_doctest/` (python#112109)

Co-authored-by: Brett Cannon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brettcannon brettcannon brettcannon approved these changes
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@vstinner vstinner Awaiting requested review from vstinner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sobolevn @vstinner @brettcannon
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.