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
_______________________________________________
python-devel mailing list -- python-devel@lists.fedoraproject.org
To unsubscribe send an email to python-devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to