Skip to content

Run the v1 multi-rank CPU suite on pull requests #8944

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.

Sign up for GitHub

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

Open
delock wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Run the v1 multi-rank CPU suite on pull requests #8944

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/workflows/cpu-torch-latest-v1-multirank.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ name: cpu-torch-latest-v1-multirank
# DS_MULTIRANK_ONLY drops single-rank tests, so a run spends its time on
# world_size 2-4 CPU tests.
#
# Trigger rollout: manual dispatch only for now. Starting as a required
# pull_request check would put an unvalidated ~1h suite in front of every PR;
# once this run is green across a few real PRs, add `pull_request:` back to the
# `on:` block and register the check with branch protection.
# Trigger rollout: manual dispatch validated the suite on master, so it now
# also runs on pull requests and in the merge queue; register the unit-tests
# check with branch protection to make it required.

on:
workflow_dispatch:
Expand Down Expand Up @@ -49,6 +48,9 @@ on:
required: false
default: 'main'
type: string
pull_request:

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fail the required check when collection fails

Once this pull-request job is made required, a failure in check-paths or collect-tests (checkout, selector, validation, artifact upload, or runner failure) leaves needs.collect-tests.result != 'success', so the condition at .github/workflows/cpu-torch-latest-v1-multirank.yml:122 skips unit-tests; skipped dependent jobs satisfy a required check, as the Modal workflow notes and guards against at .github/workflows/modal-torch-latest.yml:271-280, and the PR can merge without tests. Make the required job enter an explicit failing guard whenever collection was expected but failed, while retaining intentional path and mode=none skips.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Run the selector from trusted base-branch code

For a PR that changes ci/tests_fetcher.py, the checkout at line 85 leaves the candidate's CI code in place and line 98 executes it, so that version can emit a valid mode=none result and skip the required test job even though the trusted selector's ci/** rule would force a full run. This is the exact required-check bypass that .github/workflows/TEST_SELECTION_STATE.md:21-24 says the base-branch selector flow prevents; execute trusted CI code over the candidate tree instead.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Handle subsets with no applicable multi-rank tests

For a PR whose selected subset contains no retained multi-rank item—for example, a direct edit to tests/unit/v1/zero/test_overlap_comm_record_stream.py, whose tests are not DistributedTests—the selector returns subset and starts this job, but DS_MULTIRANK_ONLY at line 292 makes tests/conftest.py:77-93 deselect every item and pytest exits 5 for no tests. Manual dispatch always selects the full scope, so it does not exercise this case, and the new required check will block otherwise valid PRs; detect an empty applicable subset before invoking pytest or handle that no-test outcome explicitly.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Diff pull requests from the merge commit's first parent

When a PR's target branch has advanced, github.event.pull_request.base.sha used at line 96 can be days older than the base in the checked-out merge commit, so the selector's merge-base diff includes intervening master changes and may choose mode=all for an unrelated PR (for example after a ci/** or core-runtime change), spending the roughly one-hour suite this trigger is intended to avoid. The repository documents this payload behavior at .github/workflows/TEST_SELECTION.md:87-93; use the merge commit's exact first parent as check-paths.yml does.

Useful? React with 👍 / 👎.

merge_group:
branches: [ master ]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand Down
Loading