Skip to content

Move sigint tests into subprocesses #20907

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

Merged
merged 6 commits into from
Oct 1, 2021

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Aug 26, 2021

PR Summary

This prevents them accidentally breaking the test runner itself, depending on platform. Also, enable Qt tests on AppVeyor. A followup to #20883.

I haven't converted test_fig_sigint_override yet, as I want to see if this works on Windows. test_fig_sigint_override doesn't actually fire any signals, but I modified it to ensure it doesn't leave broken global state.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [n/a] New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@QuLogic
Copy link
Member Author

QuLogic commented Aug 26, 2021

Strange, it fails when the entire test suite is run, but not when running only the one test (or even just the one file).

@QuLogic
Copy link
Member Author

QuLogic commented Aug 26, 2021

Ah, it's probably because print is not safe in signal handlers, and the interrupt occurs immediately after instead of waiting a bit.

@QuLogic QuLogic force-pushed the qt-interrupt-popen branch 2 times, most recently from 6eab1f1 to f2740fd Compare August 31, 2021 21:06
@tacaswell tacaswell force-pushed the qt-interrupt-popen branch 3 times, most recently from 112668e to a40759c Compare September 16, 2021 23:05
qt_core.QTimer.singleShot(100, fire_sigint)
def custom_signal_handler(signum, frame):
timer.start()
signal.signal(signal.SIGUSR1, custom_signal_handler)
Copy link
Member

Choose a reason for hiding this comment

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

There is a whole bunch of code here that is not being run. Is that on purpose/it will be run at a later date?

Copy link
Member Author

Choose a reason for hiding this comment

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

Everything in *_impl functions are run in a subprocess which coverage doesn't see.

Copy link
Contributor

@anntzer anntzer left a comment

Choose a reason for hiding this comment

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

+/- explaining why the BlockingIOError can be ignored.

@jklymak jklymak merged commit 5725b91 into matplotlib:master Oct 1, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 1, 2021
@QuLogic QuLogic deleted the qt-interrupt-popen branch October 1, 2021 09:02
timhoffm added a commit that referenced this pull request Oct 1, 2021
…907-on-v3.5.x

Backport PR #20907 on branch v3.5.x (Move sigint tests into subprocesses)
tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request Oct 12, 2021
tacaswell pushed a commit that referenced this pull request Oct 20, 2021
Move sigint tests into subprocesses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants