This PR is a first attempt tofix#18246. Try to isolate suspected interactions between tests by using subprocesses liberally. The argument against this might be that subprocess tests are computationally expensive and we might better track down the weird interactions. I'm happy to go with this or try a different idea!

Eagerly making the pull request to engage CI.

PR Checklist

Has Pytest style unit tests
Code isFlake 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

jklymak added the topic: testing label

Aug 14, 2020

Copy link

Member

jklymak commentedAug 14, 2020

I'll take the liberty of converting to draft until you are done experimenting. Feel free to convert back...

jklymak marked this pull request as draft

August 14, 2020 19:44

richardsheridan force-pushed thebackend_tk_tests_to_subprocess branch from9156b85 to1b3d407Compare

August 14, 2020 22:54

richardsheridan mentioned this pull request

Aug 14, 2020

Cleanup tkinter state between tests#18265

Closed

6 tasks

Copy link

Member

QuLogic commentedAug 15, 2020

Itseems to be working; you'll need to rebase to get the docs CI working, and also fix the flake8 issues.

jkseppan approved these changes

Aug 15, 2020

View reviewed changes

Copy link

Member

jkseppan left a comment

Choose a reason for hiding this comment

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

Looks good to me, flake8 has some complaints.

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

richardsheridan marked this pull request as ready for review

August 15, 2020 13:51

jklymak requested a review fromQuLogic

August 15, 2020 15:51

QuLogic approved these changes

Aug 17, 2020

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.

My only question is why sometimes you checksubprocess.TimeoutExpired and sometimessubprocess.CalledProcessError?

Copy link

Member

QuLogic commentedAug 17, 2020

Also, we prefer rebase over merge.

richardsheridan added6 commits

August 17, 2020 19:14

Migrate tk backend tests into subprocesses

6253d37

Try to isolate suspected interactions between tests

Better behavior in case of timeout

96caee2

Consistent environments for subprocesses

725f1b9

H/T@QuLogic

linting

9371cb4

universal subprocesses

0ed7a30

add note to continue the concept going forward

Consistent subprocess error checks and add comments

030943d

richardsheridan force-pushed thebackend_tk_tests_to_subprocess branch fromfa60aa9 to030943dCompare

August 17, 2020 23:28

linting

513034c

Copy link

ContributorAuthor

richardsheridan commentedAug 17, 2020

@QuLogic The timeout checks should be there consistently, since you mention it, but some of the tests are better off to check the content of stdout or stderr before checking the return code. Hopefully the new comments make this clear! Also rebased as requested.