Skip to content

Conversation

skirpichev
Copy link
Collaborator

Closes #99

conftest.py moved to enable reset_mp_globals() fixture for doctests

Closes mpmath#99
@oscarbenjamin
Copy link

Obviously it is useful to be able to run the tests in parallel locally so this is good.

When I've tried to use pytest-xdist before in CI though it hasn't seemed to bring the expected speed up in CI itself. I think GA runners only have two cores and I guess there's just too much overhead in the main thread to get the benefit of distributing the work over two threads. I guess it doesn't hurt though and it is good to ensure that being able to run the tests in parallel is itself tested in CI.

One potential downside is that if the tests fail because of something hanging then it can be difficult to know from the xdist output which test has failed which is particularly awkward if the failure is seen in CI but not locally. I haven't found anything better for this than using pytest's verbose mode to print out each test name as it starts and finishes.

@skirpichev
Copy link
Collaborator Author

skirpichev commented Apr 20, 2023

it hasn't seemed to bring the expected speed up in CI itself

Well, we have 5-8 min's in this pr for py3.10/11 and 8-11 in the master.

I guess it doesn't hurt though and it is good to ensure that being able to run the tests in parallel is itself tested in CI.

And we can drop workarounds for resetting globals (dps, prec, etc).

One potential downside is that if the tests fail because of something hanging then it can be difficult to know from the xdist output which test has failed

I don't think it's a special downside of the xdist plugin. Same will be for plain pytest.

I haven't found anything better for this than using pytest's verbose mode to print out each test name as it starts and finishes.

Probably we could use the pytest-timeout plugin with some "big enough" default.

@oscarbenjamin
Copy link

Well, we have 5-8 min's in this pr for py3.10/11 and 8-11 in the master.

Okay, that's good. That didn't happen when I tried before.

I don't think it's a special downside of the xdist plugin. Same will be for plain pytest.

No because plain pytest shows which test module it is currently working on and how many tests have already run like:

mpmath/usertools.py .                                                    [ 16%]
mpmath/visualization.py .                                                [ 17%]
mpmath/calculus/approximation.py ..                                      [ 17%]
mpmath/calculus/differentiation.py .....

With xdist the default output is just

gw0 I / gw1 I
gw0 [632] / gw1 [632]

........................................................................ [ 11%]
........................................................................ [ 22%]
........................................................................ [ 34%]
........................................................................ [ 45%]
........................................................................ [ 56%]
........................................................................ [ 68%]
........................................................................ [ 79%]
.......................................x.......s...................x.... [ 91%]
........................................................                 [100%]

If one of the two workers hangs then the other will complete the test suite and it will just show that it's stuck at 99%.

Probably we could use the pytest-timeout plugin with some "big enough" default.

That's probably a better idea. None of the tests here take that long.

@oscarbenjamin
Copy link

And we can drop workarounds for resetting globals (dps, prec, etc).

Maybe it would be better not to have tests/doctests that modify the globals in the first place (i.e. gh-683).

@skirpichev
Copy link
Collaborator Author

mpmath/calculus/differentiation.py .....
vs
........................................................ [100%]

Neither variant shows a test that hangs.

That's probably a better idea.

But I doubt we should add such dependency right now.

Maybe it would be better not to have tests/doctests that modify the globals in the first place (i.e. #683).

Sure, but that's a different issue.

@skirpichev skirpichev merged commit b37a5ec into mpmath:master Apr 21, 2023
@skirpichev skirpichev deleted the xdist branch April 21, 2023 00:24
@skirpichev skirpichev added this to the 1.4 milestone May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multithreaded runtests.py
2 participants