On 7/7/22 05:38, Miro Hrončok wrote:
In the spirit of other packaging guidelines, I believe we should use
this instead:
%pytest -n %{_smp_build_ncpus}
I agree that this is more strictly correct.
Should I do this in a mass change? Not so many packages use pytest -n
auto in the spec:
I think this would be reasonable.
And, considering many other packages might want to benefit from that,
should this be:
1) encouraged in the Python packaging guidelines
I think it would be great to at least document how to do it in the
guidelines. I had a vague sense that pytest-xdist allowed
parallelization, but I didn’t know off the top of my head that it added
a straightforward pytest option.
I’m torn on whether we should make this a SHOULD, and on whether we
should try to automate it, expecting packagers to opt out as needed. On
one hand, both would be consistent with the existing guidelines for
Make[1]. On the other hand, I think it may be even more common in
Python-land to encounter test suites that aren’t parallel-safe due to
things like filesystem race conditions, and these issues can be
difficult to diagnose and debug if one isn’t already looking for them.
Where upstream already pulls in pytest-xdist and uses “-n auto” or
similar—but perhaps in a tox.ini, shell script, CI configuration,
Makefile, etc. that isn’t used when running the tests for the RPM—it’s
very safe to parallelize the tests in the RPM build, and a good idea to
do so.
A good example of the ideal case and potential benefit is
python-aws-sam-translator. It has a huge test suite; upstream brings in
pytest-xdist via the “dev” extra; and there is a Makefile (not suitable
for use in the RPM build) that uses “-n auto”, so I know the tests are
safe to parallelize. Adding “-n %{_smp_build_ncpus}” to “%pytest” speeds
up the tests from 130 seconds to 40 seconds on a 16-core machine.
2) macronized (I was thinking %pytest_parallel, but TBD)
I’m
not as certain about this; it usefully hides the extra option to
pytest, but the abstraction leaks because the packager still has to
understand that pytest-xdist is responsible and add the necessary BR (or
verify that it is already generated).
[1]
https://docs.fedoraproject.org/en-US/packaging-guidelines/#_parallel_make
– Ben