Skip to content

build: add make target to run distributed tests locally - #2017

Merged
gabrielfruet merged 3 commits into
lightly-ai:masterfrom
saud5150:build/test-distributed-make-target
Aug 7, 2026
Merged

build: add make target to run distributed tests locally#2017
gabrielfruet merged 3 commits into
lightly-ai:masterfrom
saud5150:build/test-distributed-make-target

Conversation

@saud5150

@saud5150 saud5150 commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #2011, requested by @liopeer in #2011 (comment).

Description

  • My change is breaking

Adds make test-distributed to run the @pytest.mark.DDP tests on the shared gloo pool locally, mirroring the CI distributed-tests job (USE_PYTEST_POOL=1 ... --runslow -m DDP) so the invocation doesn't have to be reconstructed by hand.

One non-obvious detail, documented as a comment on the target: it must use python -m pytest rather than plain pytest like the neighbouring targets. python -m puts the repo root on sys.path, which the spawned pool workers inherit; with the bare pytest entry point the workers crash on their first task with ModuleNotFoundError: No module named 'tests' and the session hangs silently at the pool's starmap. (CI never hits this because it already invokes python -m pytest.)

--runslow is kept for parity with the CI job even though neither current DDP test is slow-marked, so a future slow+DDP test behaves the same locally and in CI. On platforms without a usable backend (e.g. Windows), USE_PYTEST_POOL folds in the platform checks, so the target just reports the DDP tests as skipped.

Tests

  • My change is covered by existing tests.
  • My change needs new tests.
  • I have added/adapted the tests accordingly.
  • I have manually tested the change.

macOS arm64, Python 3.12, fresh venv (.[dev,matplotlib,minimal,timm,video]):

make test-distributed
# => 2 passed, 1 skipped, 2002 deselected in 15.70s   # BarlowTwins DDP tests run on the pool

# control — same selection with the flag off:
python -m pytest tests --runslow -m DDP
# => 3 skipped, 2002 deselected                       # DDP tests correctly skip

# regression check for the sys.path detail — bare pytest instead of python -m pytest:
USE_PYTEST_POOL=1 pytest tests --runslow -m DDP
# => workers die with ModuleNotFoundError: No module named 'tests'; session hangs

Documentation

  • I have added docstrings to all public functions/methods.
  • My change requires a change to the documentation ( .rst files).
  • I have updated the documentation accordingly.
  • The autodocs update the documentation accordingly.

Makefile-only change; the python -m pytest requirement is documented on the target itself.

Implications / comments / further issues

  • .PHONY is omitted to match the neighbouring test targets (test, test-fast, lint, ...), none of which declare it. Happy to add it here (or as a repo-wide .PHONY cleanup in a separate PR) if you prefer.

Follow-up to lightly-ai#2011: add `make test-distributed` so the DDP-marked tests
can be run on the shared gloo pool locally without reconstructing the CI
invocation by hand.
@saud5150
saud5150 force-pushed the build/test-distributed-make-target branch from 4083475 to 5465050 Compare August 6, 2026 10:00
@gabrielfruet

Copy link
Copy Markdown
Contributor

/review

@gabrielfruet gabrielfruet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!! Thank you

@gabrielfruet
gabrielfruet enabled auto-merge August 6, 2026 20:01
@gabrielfruet
gabrielfruet disabled auto-merge August 6, 2026 21:02
@gabrielfruet
gabrielfruet enabled auto-merge August 6, 2026 21:03
@liopeer

liopeer commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Very confused that I somehow can't run the workflows here 🤔

@gabrielfruet
gabrielfruet added this pull request to the merge queue Aug 7, 2026
Merged via the queue into lightly-ai:master with commit f937010 Aug 7, 2026
13 checks passed
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.

3 participants