From f57c65839bccedd4f9fcad4c24d778d3fb3f8c2a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 25 Feb 2024 11:49:41 -0300 Subject: [PATCH 1/4] Fix '-n logical' inconsistencies and review --help formatting (#1022) Fix #1021 and review formatting of the --help messages, following pytest's rules for consistency. --- docs/distribution.rst | 6 ++--- src/xdist/looponfail.py | 4 +-- src/xdist/plugin.py | 60 ++++++++++++++++++++++------------------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/docs/distribution.rst b/docs/distribution.rst index 93d52f67..ebbfa088 100644 --- a/docs/distribution.rst +++ b/docs/distribution.rst @@ -11,11 +11,11 @@ This can lead to considerable speed ups, especially if your test suite takes a noticeable amount of time. With ``-n auto``, pytest-xdist will use as many processes as your computer -has CPU cores. +has physical CPU cores. Use ``-n logical`` to use the number of *logical* CPU cores rather than -physical ones. This currently requires the ``psutil`` package to be installed; -if it is not, pytest-xdist will fall back to ``-n auto`` behavior. +physical ones. This currently requires the `psutil `__ package to be installed; +if it is not or if it fails to determine the number of logical CPUs, fall back to ``-n auto`` behavior. Pass a number, e.g. ``-n 8``, to specify the number of processes explicitly. diff --git a/src/xdist/looponfail.py b/src/xdist/looponfail.py index ab9ff32e..7ef24c15 100644 --- a/src/xdist/looponfail.py +++ b/src/xdist/looponfail.py @@ -29,8 +29,8 @@ def pytest_addoption(parser): action="store_true", dest="looponfail", default=False, - help="run tests in subprocess, wait for modified files " - "and re-run failing test set until all pass.", + help="Run tests in subprocess: wait for files to be modified, then " + "re-run failing test set until all pass.", ) diff --git a/src/xdist/plugin.py b/src/xdist/plugin.py index 5360b5d5..68caa814 100644 --- a/src/xdist/plugin.py +++ b/src/xdist/plugin.py @@ -61,6 +61,10 @@ def parse_numprocesses(s): @pytest.hookimpl def pytest_addoption(parser): + # 'Help' formatting (same rules as pytest's): + # Start with capitalized letters. + # If a single phrase, do not end with period. If more than one phrase, all phrases end with periods. + # Use \n to separate logical lines. group = parser.getgroup("xdist", "distributed and subprocess testing") group._addoption( "-n", @@ -69,10 +73,11 @@ def pytest_addoption(parser): metavar="numprocesses", action="store", type=parse_numprocesses, - help="Shortcut for '--dist=load --tx=NUM*popen'. With 'auto', attempt " - "to detect physical CPU count. With 'logical', detect logical CPU " - "count. If physical CPU count cannot be found, falls back to logical " - "count. This will be 0 when used with --pdb.", + help="Shortcut for '--dist=load --tx=NUM*popen'.\n" + "With 'logical', attempt to detect logical CPU count (requires psutil, falls back to 'auto').\n" + "With 'auto', attempt to detect physical CPU count. If physical CPU count cannot be determined, " + "falls back to 1.\n" + "Forced to 0 (disabled) when used with --pdb.", ) group.addoption( "--maxprocesses", @@ -80,14 +85,15 @@ def pytest_addoption(parser): metavar="maxprocesses", action="store", type=int, - help="limit the maximum number of workers to process the tests when using --numprocesses=auto", + help="Limit the maximum number of workers to process the tests when using --numprocesses " + "with 'auto' or 'logical'", ) group.addoption( "--max-worker-restart", action="store", default=None, dest="maxworkerrestart", - help="maximum number of workers that can be restarted " + help="Maximum number of workers that can be restarted " "when crashed (set to zero to disable this feature)", ) group.addoption( @@ -106,18 +112,18 @@ def pytest_addoption(parser): dest="dist", default="no", help=( - "set mode for distributing tests to exec environments.\n\n" - "each: send each test to all available environments.\n\n" - "load: load balance by sending any pending test to any" + "Set mode for distributing tests to exec environments.\n\n" + "each: Send each test to all available environments.\n\n" + "load: Load balance by sending any pending test to any" " available environment.\n\n" - "loadscope: load balance by sending pending groups of tests in" + "loadscope: Load balance by sending pending groups of tests in" " the same scope to any available environment.\n\n" - "loadfile: load balance by sending test grouped by file" + "loadfile: Load balance by sending test grouped by file" " to any available environment.\n\n" - "loadgroup: like load, but sends tests marked with 'xdist_group' to the same worker.\n\n" - "worksteal: split the test suite between available environments," - " then rebalance when any worker runs out of tests.\n\n" - "(default) no: run tests inprocess, don't distribute." + "loadgroup: Like 'load', but sends tests marked with 'xdist_group' to the same worker.\n\n" + "worksteal: Split the test suite between available environments," + " then re-balance when any worker runs out of tests.\n\n" + "(default) no: Run tests inprocess, don't distribute." ), ) group.addoption( @@ -127,8 +133,8 @@ def pytest_addoption(parser): default=[], metavar="xspec", help=( - "add a test execution environment. some examples: " - "--tx popen//python=python2.5 --tx socket=192.168.1.102:8888 " + "Add a test execution environment. Some examples:\n" + "--tx popen//python=python2.5 --tx socket=192.168.1.102:8888\n" "--tx ssh=user@codespeak.net//chdir=testcache" ), ) @@ -137,29 +143,29 @@ def pytest_addoption(parser): action="store_true", dest="distload", default=False, - help="load-balance tests. shortcut for '--dist=load'", + help="Load-balance tests. Shortcut for '--dist=load'.", ) group.addoption( "--rsyncdir", action="append", default=[], metavar="DIR", - help="add directory for rsyncing to remote tx nodes.", + help="Add directory for rsyncing to remote tx nodes", ) group.addoption( "--rsyncignore", action="append", default=[], metavar="GLOB", - help="add expression for ignores when rsyncing to remote tx nodes.", + help="Add expression for ignores when rsyncing to remote tx nodes", ) group.addoption( "--testrunuid", action="store", help=( - "provide an identifier shared amongst all workers as the value of " - "the 'testrun_uid' fixture,\n\n," - "if not provided, 'testrun_uid' is filled with a new unique string " + "Provide an identifier shared amongst all workers as the value of " + "the 'testrun_uid' fixture.\n" + "If not provided, 'testrun_uid' is filled with a new unique string " "on every test run." ), ) @@ -168,13 +174,13 @@ def pytest_addoption(parser): action="store", type=int, help=( - "Maximum number of tests scheduled in one step for --dist=load. " + "Maximum number of tests scheduled in one step for --dist=load.\n" "Setting it to 1 will force pytest to send tests to workers one by " - "one - might be useful for a small number of slow tests. " + "one - might be useful for a small number of slow tests.\n" "Larger numbers will allow the scheduler to submit consecutive " - "chunks of tests to workers - allows reusing fixtures. " + "chunks of tests to workers - allows reusing fixtures.\n" "Due to implementation reasons, at least 2 tests are scheduled per " - "worker at the start. Only later tests can be scheduled one by one. " + "worker at the start. Only later tests can be scheduled one by one.\n" "Unlimited if not set." ), ) From 51ef94439c0b3c6b3c1804796bf6a4372a0160fe Mon Sep 17 00:00:00 2001 From: Jeremy Hiatt Date: Tue, 27 Feb 2024 20:31:00 -0800 Subject: [PATCH 2/4] Avoid modifying path placeholders created by editable installs The setuptools implementation of editable installs will insert a placeholder entry into sys.path as part of its magic to register its custom import mechanism. These are not real filesystem paths and as such should not be rewritten to absolute paths. --- changelog/1028.bugfix | 1 + src/xdist/looponfail.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog/1028.bugfix diff --git a/changelog/1028.bugfix b/changelog/1028.bugfix new file mode 100644 index 00000000..39bc7a95 --- /dev/null +++ b/changelog/1028.bugfix @@ -0,0 +1 @@ +Fix compatiblity issue between `looponfail` and editable installs. diff --git a/src/xdist/looponfail.py b/src/xdist/looponfail.py index 7ef24c15..370cb8bf 100644 --- a/src/xdist/looponfail.py +++ b/src/xdist/looponfail.py @@ -160,7 +160,8 @@ def init_worker_session(channel, args, option_dict): newpaths = [] for p in sys.path: if p: - if not os.path.isabs(p): + # Ignore path placeholders created for editable installs + if not os.path.isabs(p) and not p.endswith(".__path_hook__"): p = os.path.abspath(p) newpaths.append(p) sys.path[:] = newpaths From 79fb6ff9c0c2bc0db97a40ce3a81806e376d4807 Mon Sep 17 00:00:00 2001 From: Jeremy Hiatt Date: Wed, 28 Feb 2024 15:11:07 -0800 Subject: [PATCH 3/4] testing: Add test to assert path hooks ignored in sys.path --- testing/test_looponfail.py | 39 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/testing/test_looponfail.py b/testing/test_looponfail.py index 92d43848..65a89fbf 100644 --- a/testing/test_looponfail.py +++ b/testing/test_looponfail.py @@ -1,3 +1,5 @@ +import pathlib +import tempfile import unittest.mock from typing import List @@ -191,6 +193,43 @@ def test_func(): control.loop_once() assert control.failures + def test_ignore_sys_path_hook_entry( + self, pytester: pytest.Pytester, monkeypatch: pytest.MonkeyPatch + ) -> None: + # Modifying sys.path as seen by the worker process is a bit tricky, + # because any changes made in the current process do not carry over. + # However, we can leverage the `sitecustomize` behavior to run arbitrary + # code when the subprocess interpreter is starting up. We just need to + # install our module in the search path, which we can accomplish by + # adding a temporary directory to PYTHONPATH. + tmpdir = tempfile.TemporaryDirectory() + with open(pathlib.Path(tmpdir.name) / "sitecustomize.py", "w") as custom: + print( + textwrap.dedent( + """ + import sys + sys.path.append('dummy.__path_hook__') + """ + ), + file=custom, + ) + + monkeypatch.setenv("PYTHONPATH", tmpdir.name, prepend=":") + + item = pytester.getitem( + textwrap.dedent( + """ + def test_func(): + import sys + assert "dummy.__path_hook__" in sys.path + """ + ) + ) + control = RemoteControl(item.config) + control.setup() + topdir, failures = control.runsession()[:2] + assert not failures + class TestLooponFailing: def test_looponfail_from_fail_to_ok(self, pytester: pytest.Pytester) -> None: From 0596f2755bb7abdbda4f4ff113214700936fa375 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:15:58 -0300 Subject: [PATCH 4/4] Bump the github-actions group with 1 update (#1035) Bumps the github-actions group with 1 update: [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish). Updates `pypa/gh-action-pypi-publish` from 1.8.11 to 1.8.12 - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](https://github.com/pypa/gh-action-pypi-publish/compare/v1.8.11...v1.8.12) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 37a46f62..69bb823a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -39,7 +39,7 @@ jobs: path: dist - name: Publish package to PyPI - uses: pypa/gh-action-pypi-publish@v1.8.11 + uses: pypa/gh-action-pypi-publish@v1.8.12 - name: Push tag run: |