Skip to content

gh-110944: Move pty helper to test.support and add basic pdb completion test #111826

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
Merged
merged 5 commits into from
Nov 13, 2023
Merged

gh-110944: Move pty helper to test.support and add basic pdb completion test #111826

merged 5 commits into from
Nov 13, 2023

Conversation

gaogaotiantian
Copy link
Member

@gaogaotiantian gaogaotiantian commented Nov 7, 2023 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Nov 7, 2023
Closed
@gaogaotiantian gaogaotiantian mentioned this pull request Nov 7, 2023
Merged
encukou
encukou reviewed Nov 9, 2023
View reviewed changes
Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

Looks good, thank you! I have a few nitpicks, sorry for not thinking about these earlier.

Lib/test/test_pdb.py Outdated
Comment on lines 3261 to 3263
self.assertIn(b'cont', output)
self.assertIn(b'condition', output)
self.assertIn(b'continue', output)
Copy link
Member

Choose a reason for hiding this comment

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

The cont assert is useless since continue is checked later. How about checking that it appears as a separate word?

Also, IMO it would be nice to assert that thecontinue command caused the print to run (outputting a string that doesn't appear in the source).

Suggested change
self.assertIn(b'cont', output)
self.assertIn(b'condition', output)
self.assertIn(b'continue', output)
self.assertIn(b'cont', output.split())
self.assertIn(b'condition', output)
self.assertIn(b'continue', output)
self.assertIn(b'hello!', output)

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 9, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit ddd67d5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 9, 2023
@encukou
Copy link
Member

encukou commented Nov 9, 2023

(I just enabled extra CI that isn't always "green", but includes more platforms & configurations, which seems fitting for a PTY-related PR. Don't worry about the test failures, I'll check whether they're related.)

gaogaotiantian and others added 2 commits November 9, 2023 10:45
@gaogaotiantian @encukou
Update Lib/test/test_pdb.py
2bc94b7 
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@gaogaotiantian
Use commands instead of cont
fe6c8ab
@gaogaotiantian
Copy link
Member Author

You are right about cont, but spliting output makes the code more complicated than it needs to be. I just replaced it with commands. The hello! check works for sure, but if continue does not run properly, the test will timeout because it will just stuck there. So here it does not check for whether continue is correctly completed, it actually checks whether continue works as documented - to continue the program.

@encukou
Copy link
Member

encukou commented Nov 9, 2023 via email

@encukou encukou added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Nov 13, 2023
@encukou encukou merged commit 1c7ed7e into python:main Nov 13, 2023
@miss-islington-app
Copy link

Thanks @gaogaotiantian for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2023
@gaogaotiantian @miss-islington
pythongh-110944: Move pty helper to test.support and add basic pdb co…
f247734 
…mpletion test (pythonGH-111826)

(cherry picked from commit 1c7ed7e)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Nov 13, 2023

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2023
@gaogaotiantian @miss-islington
pythongh-110944: Move pty helper to test.support and add basic pdb co…
d38c778 
…mpletion test (pythonGH-111826)

(cherry picked from commit 1c7ed7e)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 13, 2023
@bedevere-app
Copy link

bedevere-app bot commented Nov 13, 2023

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

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Nov 13, 2023
@gaogaotiantian gaogaotiantian deleted the move-pty-helper branch November 13, 2023 20:05
@gaogaotiantian gaogaotiantian mentioned this pull request Nov 13, 2023
Merged
encukou pushed a commit that referenced this pull request Nov 14, 2023
@miss-islington @gaogaotiantian
[3.12] gh-110944: Move pty helper to test.support and add basic pdb c…
c003de9 
…ompletion test (GH-111826) (GH-112024)

gh-110944: Move pty helper to test.support and add basic pdb completion test (GH-111826)
(cherry picked from commit 1c7ed7e)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
encukou pushed a commit that referenced this pull request Nov 14, 2023
@miss-islington @gaogaotiantian
[3.11] gh-110944: Move pty helper to test.support and add basic pdb c…
e73216d 
…ompletion test (GH-111826) (GH-112025)

gh-110944: Move pty helper to test.support and add basic pdb completion test (GH-111826)
(cherry picked from commit 1c7ed7e)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@gaogaotiantian @aisk
pythongh-110944: Move pty helper to test.support and add basic pdb co…
20a58ac 
…mpletion test (pythonGH-111826)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@gaogaotiantian @Glyphack
pythongh-110944: Move pty helper to test.support and add basic pdb co…
7b9b8c2 
…mpletion test (pythonGH-111826)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@encukou encukou Awaiting requested review from encukou

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gaogaotiantian @bedevere-bot @encukou