Conversation
I'm not entirely sure why the test is hanging, but this seems clear enough: - the test setup spawns a (non-daemon) background thread that runs forever, or until it is told to quit by receiving a TCP packet on a certain port - the test teardown tries to tell the background thread to quit (which doesn't work) and waits for that to happen - as a result the entire test run hangs forever This commit adds a timeout as an extra safety net so that the test run will complete even if the clean shutdown procedure fails for some reason.
Closed
Contributor
Author
This is probably because telnetlib.Telnet.write() expects a byte string but receives a plain str instead. I'm sure a fix for that is waiting somewhere in the huge series of patches that comprise #210 and we'll get to it eventually, but I'd rather have a reliable test suite that doesn't hang even if bugs are present in the code. |
anarcat
reviewed
Apr 25, 2019
tox -e py27 -- tests/checker/test_telnet.py takes 30 seconds to complete. That seems excessive to me, but one thing at a time.
In cast we forget or somebody else wants to tackle this. After all, the assertion error + traceback shows up at the end of the test run, and it's not immediately clear which test is to blame for it!
Contributor
|
hum. i thought it was either this PR or #254 but not both, did i miss something? either way, thanks for your work :) |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I'm not entirely sure why the test is hanging, but this seems clear
enough:
forever, or until it is told to quit by receiving a TCP packet on a
certain port
doesn't work) and waits for that to happen
This commit adds a timeout as an extra safety net so that the test run
will complete even if the clean shutdown procedure fails for some
reason.