Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Scheduler for Grouping Tests w/ Variable number of Workers #1130

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ciaconet
Copy link

@ciaconet ciaconet commented Sep 18, 2024

This pull request addresses Issue 1014. It adds a new scheduler that allows a user to add tests to arbitrary groups using the pytest mark xdist_custom. Using the mark a group name and number of workers to use for that group is specified. Groups are ran sequentially but tests in each group are ran in parallel with the given number of workers.

This pull request is a rough prototype. We aren't too familiar with pytest-xdist internals and we don't expect it is merge-able as is. However, we're hoping to open the conversation to see if there are improvements we can make to get this merged into pytest-xdist.

Since we wanted groups of tests to execute sequentially, we did not want to schedule tests from a pending group until after the currently running group completed all tests. We found that in order to obtain the status of the final test scheduled on a worker, we needed to send a shutdown signal to the worker. Because of this, between groups a shutdown signal is sent to each worker, and then workers are initialized again prior to scheduling the next group of tests.

We left the directory xdist-testing-ntop in as an example but will remove that and add proper tests.

Will add a changelog/1014.feature if this PR gains traction.

ciaconet and others added 5 commits September 16, 2024 12:05
TUR-21619 is a prototype method of executing groups of pytest test cases in series with each group running it's specific test cases in parallel with a variable number of workers.

Tests can be marked with a custom mark. This custom mark specifies an arbitrary group name, and the number of processes to run tests in that group with, with the number of processes separated from the group name by an underscore.

Due to limitations in pytest/execnet a shutdown signal must be sent to a node when one test case remains on the node (the test case is actually ran immediately, but we do not receive feedback through the channel until a shutdown signal is sent due to an old design decision in pytest). Because of this, in order to run additional tests after a group is nearly complete (when each worker has at most 1 test remaining), we must shutdown each node, recreate each node, and then schedule more tests. This adds overhead as we must teardown + startup nodes, however, this overhead is on the order of 1 second per teardown/startup. If this custom scheduling method allows us to save more than 1 second in test execution, it is worth the increased overhead cost.

Note: The -n argument with --dist customgroup is the maximum number of worker processes. If -n specifies 4 but a specific group specifies 8, the specific group will be ran across only 4 nodes, we will never spin up more nodes than the -n argument allows.

---------

Co-authored-by: Alvaro Barbeira <alvarobarbeira@gmail.com>
* fix pytest error

* clearer if
)

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1120)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.1 → v0.6.2](astral-sh/ruff-pre-commit@v0.6.1...v0.6.2)
- [github.com/pre-commit/mirrors-mypy: v1.11.1 → v1.11.2](pre-commit/mirrors-mypy@v1.11.1...v1.11.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* build(deps): bump pypa/gh-action-pypi-publish (pytest-dev#1123)

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.9.0 to 1.10.0
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.9.0...v1.10.0)

---
updated-dependencies:
- dependency-name: pypa/gh-action-pypi-publish
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1124)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.2 → v0.6.3](astral-sh/ruff-pre-commit@v0.6.2...v0.6.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Publish package with attestations (pytest-dev#1125)

Follow up to pytest-dev#1123.

* build(deps): bump the github-actions group with 2 updates (pytest-dev#1127)

Bumps the github-actions group with 2 updates: [hynek/build-and-inspect-python-package](https://github.com/hynek/build-and-inspect-python-package) and [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish).


Updates `hynek/build-and-inspect-python-package` from 2.8 to 2.9
- [Release notes](https://github.com/hynek/build-and-inspect-python-package/releases)
- [Changelog](https://github.com/hynek/build-and-inspect-python-package/blob/main/CHANGELOG.md)
- [Commits](hynek/build-and-inspect-python-package@v2.8...v2.9)

Updates `pypa/gh-action-pypi-publish` from 1.10.0 to 1.10.1
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.10.0...v1.10.1)

---
updated-dependencies:
- dependency-name: hynek/build-and-inspect-python-package
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
- 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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1128)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.3 → v0.6.4](astral-sh/ruff-pre-commit@v0.6.3...v0.6.4)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1129)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.4 → v0.6.5](astral-sh/ruff-pre-commit@v0.6.4...v0.6.5)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* all ruff checks should pass

* mypy fixes

* undo protocol/other scheduler changes, add asserts for mypy check

* undo newline add/remove changes in diff for other schedulers

* remove unused function from dsession.py

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>
* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1120)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.1 → v0.6.2](astral-sh/ruff-pre-commit@v0.6.1...v0.6.2)
- [github.com/pre-commit/mirrors-mypy: v1.11.1 → v1.11.2](pre-commit/mirrors-mypy@v1.11.1...v1.11.2)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* build(deps): bump pypa/gh-action-pypi-publish (pytest-dev#1123)

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.9.0 to 1.10.0
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.9.0...v1.10.0)

---
updated-dependencies:
- dependency-name: pypa/gh-action-pypi-publish
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1124)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.2 → v0.6.3](astral-sh/ruff-pre-commit@v0.6.2...v0.6.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Publish package with attestations (pytest-dev#1125)

Follow up to pytest-dev#1123.

* build(deps): bump the github-actions group with 2 updates (pytest-dev#1127)

Bumps the github-actions group with 2 updates: [hynek/build-and-inspect-python-package](https://github.com/hynek/build-and-inspect-python-package) and [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish).


Updates `hynek/build-and-inspect-python-package` from 2.8 to 2.9
- [Release notes](https://github.com/hynek/build-and-inspect-python-package/releases)
- [Changelog](https://github.com/hynek/build-and-inspect-python-package/blob/main/CHANGELOG.md)
- [Commits](hynek/build-and-inspect-python-package@v2.8...v2.9)

Updates `pypa/gh-action-pypi-publish` from 1.10.0 to 1.10.1
- [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases)
- [Commits](pypa/gh-action-pypi-publish@v1.10.0...v1.10.1)

---
updated-dependencies:
- dependency-name: hynek/build-and-inspect-python-package
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
- 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] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1128)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.3 → v0.6.4](astral-sh/ruff-pre-commit@v0.6.3...v0.6.4)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (pytest-dev#1129)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.6.4 → v0.6.5](astral-sh/ruff-pre-commit@v0.6.4...v0.6.5)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* run ruff format

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>
@dmakhno
Copy link

dmakhno commented Sep 27, 2024

@ciaconet , I wonder does it address #385 too?
For my need a concept of pytest-mp (which was archieved) is more suitabe. And this scheduler seems a good fit - and open new strategies for scheduler.

Just my naive visualization on need:
image

And your CR, "custom marker", is exactly "bucket", "batch".

@ciaconet
Copy link
Author

@ciaconet , I wonder does it address #385 too? For my need a concept of pytest-mp (which was archieved) is more suitabe. And this scheduler seems a good fit - and open new strategies for scheduler.

Just my naive visualization on need: image

And your CR, "custom marker", is exactly "bucket", "batch".

@ciaconet , I wonder does it address #385 too? For my need a concept of pytest-mp (which was archieved) is more suitabe. And this scheduler seems a good fit - and open new strategies for scheduler.

Just my naive visualization on need: image

And your CR, "custom marker", is exactly "bucket", "batch".

Unfortunately as is this PR doesn't address #385. The custom mark would give you the option to run some tests sequentially, but this would happen in it's own "bucket". With the changes on this PR we can't have some tests run sequentially while simultaneously running other tests in parallel.

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.

2 participants