NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

TST: always set a (long) timeout for subprocess and always use our wrapper#27726

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

greglucas merged 3 commits intomatplotlib:mainfromtacaswell:tst/finite_tests

Feb 14, 2024

Merged

TST: always set a (long) timeout for subprocess and always use our wrapper#27726

greglucas merged 3 commits intomatplotlib:mainfromtacaswell:tst/finite_tests

Feb 14, 2024

Conversation

Copy link

Member

tacaswell commentedJan 31, 2024

If you are using this and really need unbounded timeouts, explicitly passtimeout=None

This is an attempt to avoid azure from hanging and then timing out.

TST: always set a (long) timeout for subprocess

78657d4

If you are using this and really need unbounded timeouts, explicitlypass `timeout=None`

tacaswell added this to thev3.9.0 milestone

Jan 31, 2024

tacaswell changed the title~~TST: always set a (long) timeout for subprocess~~TST: always set a (long) timeout for subprocess and always use our wrapper

Jan 31, 2024

Copy link

MemberAuthor

tacaswell commentedJan 31, 2024

The second commit finally cleans up all of thesubprocess.run,subprocess.call,subprocess.check_output andsubprocess.check_call to be run through our helper.

tacaswell force-pushed thetst/finite_tests branch 2 times, most recently from1bfdf4d toec90d87Compare

January 31, 2024 22:09

QuLogic reviewed

Feb 1, 2024

View reviewed changes

lib/matplotlib/tests/test_backend_nbagg.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_backend_webagg.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_texmanager.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_backend_webagg.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_determinism.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_font_manager.py OutdatedShow resolvedHide resolved

lib/matplotlib/tests/test_matplotlib.py OutdatedShow resolvedHide resolved

tacaswell force-pushed thetst/finite_tests branch from3727a38 to2852ff7Compare

February 8, 2024 21:49

QuLogic reviewed

Feb 9, 2024

View reviewed changes

Copy link

Member

QuLogic left a comment

Choose a reason for hiding this comment

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

Unfortunately, whilesubprocess_run_helper defaults tocheck=True,subprocess_run_for_testing defaults tocheck=False.

lib/matplotlib/tests/test_backend_nbagg.pyShow resolvedHide resolved

lib/matplotlib/tests/test_determinism.py OutdatedShow resolvedHide resolved

tacaswell added2 commits

February 8, 2024 21:37

TST: use our subprocess helper

cc845e7

STY: formatting and import sorting

0515861

tacaswell force-pushed thetst/finite_tests branch from2852ff7 to0515861Compare

February 9, 2024 02:37

QuLogic approved these changes

Feb 9, 2024

View reviewed changes

greglucas approved these changes

Feb 14, 2024

View reviewed changes

lib/matplotlib/testing/__init__.py

		@@ -50,7 +50,7 @@ def setup():
		set_reproducibility_for_testing()


		def subprocess_run_for_testing(command, env=None, timeout=None, stdout=None,
		def subprocess_run_for_testing(command, env=None, timeout=60, stdout=None,
		stderr=None, check=False, text=True,

Copy link

Contributor

greglucasFeb 14, 2024

Choose a reason for hiding this comment

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

Suggested change

	stderr=None,check=False,text=True,
	stderr=None,check=True,text=True,

Since this is used for testing it seems like we should setcheck=True by default. It looks like you currently set this in many of the calls. Are there even more tests without setting check?