Skip to content

fix(consume): better xdist detection for consume#2793

Draft
danceratopz wants to merge 1 commit into
ethereum:forks/amsterdamfrom
danceratopz:enginex-fix-client-cleanup
Draft

fix(consume): better xdist detection for consume#2793
danceratopz wants to merge 1 commit into
ethereum:forks/amsterdamfrom
danceratopz:enginex-fix-client-cleanup

Conversation

@danceratopz
Copy link
Copy Markdown
Member

WIP: This did fix client not being correctly cleaned up, but solution looks like overkill. Will self-review first

EngineX relies on each pre-alloc group being executed by a single xdist worker so that the per-worker client manager sees every test in the group and can stop the shared client as soon as the group completes.

The previous parallelism detection only matched '-n' as a separate argument. Common spellings such as '-n=6', '-n6', and '--numprocesses=6' did not trigger the EngineX loadgroup override, so pytest-xdist could use its default distribution and split one pre-alloc group across workers.

Detect all supported xdist parallelism spellings and ensure consume enginex always runs with '--dist=loadgroup', overriding incompatible distribution modes with a warning. Keep the behavior scoped to EngineX so consume engine and the other simulators are unchanged.

🗒️ Description

🔗 Related Issues or PRs

N/A.

✅ Checklist

  • All: Ran fast static checks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:
    just static
  • All: PR title adheres to the repo standard - it will be used as the squash commit message and should start type(scope):.
  • All: Considered updating the online docs in the ./docs/ directory.
  • All: Set appropriate labels for the changes (only maintainers can apply labels).

Cute Animal Picture

image

EngineX relies on each pre-alloc group being executed by a single xdist worker so that the per-worker client manager sees every test in the group and can stop the shared client as soon as the group completes.

The previous parallelism detection only matched '-n' as a separate argument. Common spellings such as '-n=6', '-n6', and '--numprocesses=6' did not trigger the EngineX loadgroup override, so pytest-xdist could use its default distribution and split one pre-alloc group across workers.

Detect all supported xdist parallelism spellings and ensure consume enginex always runs with '--dist=loadgroup', overriding incompatible distribution modes with a warning. Keep the behavior scoped to EngineX so consume engine and the other simulators are unchanged.
@danceratopz danceratopz added C-bug Category: this is a bug, deviation, or other problem A-test-consume Area: execution_testing.cli.pytest_commands.plugins.consume labels May 1, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.17%. Comparing base (9d18733) to head (da36290).

Additional details and impacted files
@@               Coverage Diff                @@
##           forks/amsterdam    #2793   +/-   ##
================================================
  Coverage            88.17%   88.17%           
================================================
  Files                  577      577           
  Lines                35659    35659           
  Branches              3490     3490           
================================================
  Hits                 31442    31442           
  Misses                3654     3654           
  Partials               563      563           
Flag Coverage Δ
unittests 88.17% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-test-consume Area: execution_testing.cli.pytest_commands.plugins.consume C-bug Category: this is a bug, deviation, or other problem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant