Skip to content

Remove xdist_group("build") serialization from slow tests#415

Merged
bschwedler merged 3 commits into
mainfrom
remove-xdist-build-group
Apr 3, 2026
Merged

Remove xdist_group("build") serialization from slow tests#415
bschwedler merged 3 commits into
mainfrom
remove-xdist-build-group

Conversation

@bschwedler
Copy link
Copy Markdown
Contributor

The build group forced all slow tests onto a single pytest-xdist
worker, serializing ~19 Docker build tests even though most have no
shared state that would cause conflicts:

  • Each test uses isolated temp directories via get_tmpconfig/tmp_path
  • RegistryContainer uses ephemeral ports (no port collisions)
  • Different test suites produce different image names
  • BuildKit handles concurrent builds without cache corruption

The serialization was overly conservative, preventing xdist from
distributing slow tests across available workers.

@bschwedler bschwedler requested a review from ianpittwood as a code owner April 2, 2026 19:28
@bschwedler bschwedler force-pushed the remove-xdist-build-group branch from 7452c82 to 2e65813 Compare April 2, 2026 19:34
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 2, 2026

Test Results

1 243 tests  ±0   1 243 ✅ ±0   7m 52s ⏱️ - 5m 34s
    1 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit eed791f. ± Comparison against base commit b88bfc7.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

@ianpittwood ianpittwood left a comment

Choose a reason for hiding this comment

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

I think this is a reasonable change, but I want to ask a few questions before we proceed:

  1. Are all of the build calls using get_tmpconfig? They should be and I see many have been changed here already, but we should confirm one more time that there aren't any other build calls buried in the tests for other modules.
  2. Would it be good practice to use --no-cache for the in pytest builds too? All of the Dockerfiles should be relatively quick to build and it may be good practice to ensure they are built from end-to-end each time to ensure things like our macros are still functioning properly.
  3. Buildkit can be quite hefty on RAM. Do we want to make any developer notes on restricting simultaneous builds to a certain number?

@bschwedler bschwedler force-pushed the remove-xdist-build-group branch 5 times, most recently from 0e60d83 to 0ebf6cf Compare April 3, 2026 12:46
Replace @slow with @image_build on tests that build real Docker
images, and remove the xdist_group("build") serialization that
forced them onto a single pytest-xdist worker.

Isolate tests via unique registry namespaces so image tags never
collide on the shared Docker daemon:

- get_tmpcontext rewrites bakery.yaml registry namespaces with a
  unique suffix for image_build tests (e.g., posit-dev/t-a1b2c3d4)
- BDD verification/cleanup steps use get_tmpconfig to read from
  the temp copy with rewritten tags
- test_image_target.py build tests use get_tmpconfig instead of
  get_targets to build from isolated copies
- just test-all uses -n 4 to limit parallel builds on dev machines
More cores means more pytest-xdist workers, which lets the now
un-serialized image build tests run in parallel and reduces wall
clock time.
@bschwedler bschwedler force-pushed the remove-xdist-build-group branch from 0ebf6cf to 80c9aba Compare April 3, 2026 12:53
Autouse fixture patches ImageTarget.build and BakePlan.build to
default cache=False for all image_build-marked tests. This ensures
templates and macros are tested end-to-end without stale layers
masking regressions.

Covers both direct Python .build() calls and BDD scenarios that
invoke the bakery CLI (which calls the same methods internally).
@bschwedler bschwedler force-pushed the remove-xdist-build-group branch from 62d8d04 to eed791f Compare April 3, 2026 13:12
@bschwedler
Copy link
Copy Markdown
Contributor Author

1. Are _all_ of the build calls using `get_tmpconfig`? They should be and I see many have been changed here already, but we should confirm one more time that there aren't any other build calls buried in the tests for other modules.

I verified that the bakery build calls all use this fixture. I also changed the mark to image_build to make it more obvious.

2. Would it be good practice to use `--no-cache` for the in pytest builds too? All of the Dockerfiles should be relatively quick to build and it may be good practice to ensure they are built from end-to-end each time to ensure things like our macros are still functioning properly.

I added a commit to disable the cache by default to see what it does to the build time. My intent w/ this PR was to speed up the test run.

3. Buildkit can be quite hefty on RAM. Do we want to make any developer notes on restricting simultaneous builds to a certain number?

I changed the justfile default to -n 4 with a comment.

Copy link
Copy Markdown
Contributor

@ianpittwood ianpittwood left a comment

Choose a reason for hiding this comment

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

Let's give it a try!

@bschwedler bschwedler added this pull request to the merge queue Apr 3, 2026
Merged via the queue into main with commit eb174fb Apr 3, 2026
27 of 29 checks passed
@bschwedler bschwedler deleted the remove-xdist-build-group branch April 3, 2026 13:43
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