Skip to content

gh-132912: Account for race in test_keyboard_interrupt in test_remote_pdb#132929

Merged
pablogsal merged 2 commits into
python:mainfrom
pablogsal:gh-132912
Apr 25, 2025
Merged

gh-132912: Account for race in test_keyboard_interrupt in test_remote_pdb#132929
pablogsal merged 2 commits into
python:mainfrom
pablogsal:gh-132912

Conversation

@pablogsal
Copy link
Copy Markdown
Member

@pablogsal pablogsal commented Apr 25, 2025 •
edited by bedevere-app Bot
Loading

@bedevere-app bedevere-app Bot added awaiting core review tests Tests in the Lib/test dir labels Apr 25, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 25, 2025
Closed
@pablogsal pablogsal requested a review from vstinner April 25, 2025 12:11
@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Apr 25, 2025

There was a very rare race here as the test can be interrupted just when we are leaving the socket synchronization function before we go back to the main function

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Apr 25, 2025

Tested locally as @vstinner did and couldn't find anything for 2000 iterations:

OK
0:02:08 load avg: 38.40 [2021] test_remote_pdb passed
test_breakpoints (test.test_remote_pdb.PdbConnectTestCase.test_breakpoints)
Test setting and hitting breakpoints. ... ok
test_connect_and_basic_commands (test.test_remote_pdb.PdbConnectTestCase.test_connect_and_basic_commands)
Test connecting to a remote debugger and sending basic commands. ... ok
test_handle_eof (test.test_remote_pdb.PdbConnectTestCase.test_handle_eof)
Test that EOF signal properly exits the debugger. ... ok
test_help_system (test.test_remote_pdb.PdbConnectTestCase.test_help_system)
Test that the help system properly sends help text to the client. ... ok
test_keyboard_interrupt (test.test_remote_pdb.PdbConnectTestCase.test_keyboard_interrupt)
Test that sending keyboard interrupt breaks into pdb. ... ok
test_multi_line_commands (test.test_remote_pdb.PdbConnectTestCase.test_multi_line_commands)
Test that multi-line commands work properly over remote connection. ... ok
test_protocol_version (test.test_remote_pdb.PdbConnectTestCase.test_protocol_version)
Test that incompatible protocol versions are properly detected. ... ok
test_cmdloop (test.test_remote_pdb.RemotePdbTestCase.test_cmdloop)
Test the command loop with various commands. ... ok
test_completion (test.test_remote_pdb.RemotePdbTestCase.test_completion)
Test handling completion requests. ... ok
test_detach (test.test_remote_pdb.RemotePdbTestCase.test_detach)
Test the detach method. ... ok
test_do_help (test.test_remote_pdb.RemotePdbTestCase.test_do_help)
Test that do_help sends the help message. ... ok
test_interact_mode (test.test_remote_pdb.RemotePdbTestCase.test_interact_mode)
Test interaction mode setup and execution. ... test
ok
test_message_and_error (test.test_remote_pdb.RemotePdbTestCase.test_message_and_error)
Test message and error methods send correct JSON. ... ok
test_read_command (test.test_remote_pdb.RemotePdbTestCase.test_read_command)
Test reading commands from the socket. ... ok
test_read_command_EOF (test.test_remote_pdb.RemotePdbTestCase.test_read_command_EOF)
Test reading EOF command. ... ok
test_registering_commands (test.test_remote_pdb.RemotePdbTestCase.test_registering_commands)
Test registering breakpoint commands. ... ok

----------------------------------------------------------------------
Ran 16 tests in 4.128s

vstinner
vstinner reviewed Apr 25, 2025
View reviewed changes
Comment thread Lib/test/test_remote_pdb.py Outdated
pablogsal
pablogsal commented Apr 25, 2025
View reviewed changes
Comment thread Lib/test/test_remote_pdb.py Outdated
@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Apr 25, 2025

CC @godlygeek

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Apr 25, 2025

Somehow related: test_remote_pdb can still fail on a heavy loaded machine because of its hardcoded timeout of 5 seconds. I wrote #132939 to fix this issue.

vstinner
vstinner approved these changes Apr 25, 2025
View reviewed changes
Copy link
Copy Markdown
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. I cannot make it fail, it looks way more reliable.

I tested on a Linux laptop with 12 logical CPUs:

  • 4 terminals running: ./python -m test test_remote_pdb -m test_keyboard_interrupt -v --timeout=15 --forever
  • 1 terminal running: ./python -m test test_remote_pdb -m test_keyboard_interrupt -W --timeout=60 --forever -j150

I ran my test for 5 minutes. The system load was around 162.

@pablogsal
Copy link
Copy Markdown
Member Author

pablogsal commented Apr 25, 2025

Fantastic. Thanks for the help @vstinner!

@pablogsal pablogsal merged commit 6cbeb6a into python:main Apr 25, 2025
42 checks passed
@pablogsal pablogsal deleted the gh-132912 branch April 25, 2025 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@gaogaotiantian gaogaotiantian Awaiting requested review from gaogaotiantian gaogaotiantian is a code owner

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pablogsal @vstinner