-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
WIP/RFC pdb: fix usage in child thread after main thread exited #5244
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
|
Fixed the non-related test in #5245. |
| pytestPDB._saved.append( | ||
| (pdb.set_trace, pytestPDB._pluginmanager, pytestPDB._config, pytestPDB._pdb_cls) | ||
| ) | ||
| pytestPDB._saved.append(pdb.set_trace) |
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'm a little worried about this because this leaves config and pluginmanager alive after pytest.main(), which might be a problem for someone embedding pytest I guess.
I wonder if we can only pass what is is actually needed to pytestPDB instead of the entire config and pluginmanager objects?
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.
Well, the main point here is to keep them alive after pytest exits, isn't it?.. ;)
(The whole wrapping can be improved in general - I'm doing this basically step by step when running into issues)
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.
Well, the main point here is to keep them alive after pytest exits, isn't it?.. ;)
Yep, and I'm not sure that's a good thing 😅, specially if I'm not using threads at all.
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.
An alternative would be to fix the debugging plugin to not crash in this case I think.
Fixes pytest-dev#5228. Original PR (rejected): pytest-dev#5244
Fixes pytest-dev#5228. Original PR (rejected): pytest-dev#5244
Fixes #5228.
(not done against master to avoid conflicts)