Skip to content

feat: add worker ramp option#1330

Open
Harshal96 wants to merge 2 commits into
pytest-dev:masterfrom
Harshal96:feat/worker-ramp-option
Open

feat: add worker ramp option#1330
Harshal96 wants to merge 2 commits into
pytest-dev:masterfrom
Harshal96:feat/worker-ramp-option

Conversation

@Harshal96
Copy link
Copy Markdown

Summary

Add --ramp DURATION to stagger when xdist workers begin executing tests after collection.

Refs #1219.

Checklist

  • Make sure to include reasonable tests for your change if necessary
  • Added towncrier changelog entry: changelog/1219.feature.rst

Test Plan

  • uv run --extra testing pytest testing/test_plugin.py testing/test_workermanage.py testing/test_remote.py testing/acceptance_test.py -q
  • uv run --python 3.10 --with pre-commit pre-commit run --files ...

@RonnyPfannschmidt
Copy link
Copy Markdown
Member

This deviates from then proposed mechanism as it sidesteps the start later detail

As such it's a partial solution to the proposal but also a working solution that has to solve far less problems than a full process ramping solution

Its a pretty nice step forward

@Harshal96
Copy link
Copy Markdown
Author

This deviates from then proposed mechanism as it sidesteps the start later detail

As such it's a partial solution to the proposal but also a working solution that has to solve far less problems than a full process ramping solution

Its a pretty nice step forward

@RonnyPfannschmidt any suggested changes to make it an acceptable contribution?

@RonnyPfannschmidt
Copy link
Copy Markdown
Member

It already is

@Harshal96
Copy link
Copy Markdown
Author

@RonnyPfannschmidt awesome, this is my first contribution in this repo, what's the approval/merge process? Anything pending on me?

Comment thread src/xdist/plugin.py
# workaround https://github.com/pypy/pypy/issues/2375
return 2
return None
if os.environ.get("TRAVIS") == "true":
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followup note - travis is no longer a thing in direct use - we might want to drop explicit handling for it

Copy link
Copy Markdown
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the workermanage api change is okay as its a internal class

@RonnyPfannschmidt
Copy link
Copy Markdown
Member

sorry for the delays, i'm a bit stretched

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