[JacobSzwejbka]

pull.yml runs the full unit-test suite twice on every PR, on Linux, macOS and
Windows: once via unittest and again via unittest-editable. The second
copy adds coverage of the editable install path only, but costs ~200
self-hosted minutes per PR run (~104 min on windows.4xlarge, ~49 on
mt-l-x86iavx512-8-64, plus a macos-m1-stable slot).

Gate unittest-editable on pull_request to the files that affect an editable
install: setup.py, pyproject.toml, install_executorch*, install_requirements*,
install_utils.py, requirements-*, src/executorch, the unittest/setup CI
scripts, and the workflow files. It still runs on every push to main and
release branches, so an editable-only break is caught right after it lands.
unittest itself (including Windows) is unchanged.

Follow-up option, not done here: moving the Windows leg of unittest to
trunk (trunk already runs Windows unittest-release) would save another
~$4k/month of windows.4xlarge but drops pre-merge Windows signal.

Context

Part of a set of CI cost reductions for self-hosted (OSDC / AWS) runners. Estimates come from the last 30 days of GitHub Actions run counts times per-job runtimes of a sample run, priced at AWS on-demand rates as a proxy. Independent of the other PRs in the set; they all merge cleanly together.

Validation

Edited YAML parses, and actionlint 1.7.12 reports no diagnostics introduced by this change compared with the base commit. Pre-existing diagnostics were recorded separately. GitHub CI is pending.

Authored with AI assistance; this handoff was reviewed and the PR was opened with Codex.

[pytorch-bot]

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24674

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 4ced71a with merge base d1f597e ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

• pull / unittest-editable / windows / windows-job (gh) (matched win rule in flaky-rules.json)
  ##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[github-actions]

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

[psiddh]

@claude Review this change and check for any potential issues / regressions

[claude]

Claude finished @psiddh's task in 3m 27s —— View job

---

The gating logic is correct: push and workflow_dispatch still always run the job, nothing requires unittest-editable as a merge check, and Linux editable installs are still exercised on every PR by the QNN/NXP jobs. However, the path filter leaves out the CMake templates that only the editable install path uses, and PRs that delete files never trigger the job. I recommend extending the filter list before merging.

Detailed review

PR Review: #24674

Summary

The PR skips unittest-editable on PRs unless the changed files touch Python packaging or the unittest/setup scripts. It does not create a regression for push builds or merge checks, but the filter has gaps in its coverage, and some of its patterns match far more files than intended.

Infrastructure

• .github/workflows/pull.yml:893-908: the filter misses inputs used only by the editable install path.
  • In editable mode, setup.py takes its own paths:
    • _resubstitute_tracer_definition (setup.py:1738-1750) and the no-cache tracer substitution (setup.py:2614-2652) fill @EXECUTORCH_TRACER_DEFINITION@ from tools/cmake/executorch-wheel-config.cmake.
    • The in-place extension copy (setup.py:1767-1810) depends on the extensions built by extension/pybindings/ and CMakeLists.txt.
  • None of these paths is in the list, so a PR that changes only that template, a preset, or the pybindings CMake can break the editable install while passing unittest. The break would then show up only on main, and only for macOS and Windows (Linux editable is still covered, see below).
  • test-minimal-wheel-linux already gates on tools/cmake/ (pull.yml:97).
  • Suggested fix: add tools/cmake/, CMakePresets.json, and extension/pybindings/.
• PRs that delete or rename packaging files never trigger the job. _get-changed-files.yml drops removed files (select(.status != "removed")), and a PR whose only changes are deletions emits '*', which none of the contains() patterns match.
  • Example: deleting a symlink under src/executorch/ (every entry there is a symlink into the source tree), deleting install_utils.py, or renaming a requirements-*.txt skips the job. These are exactly the layout changes this job exists to catch.
  • The other gated jobs share this weakness, but it matters most here.
  • Possible fixes: treat '*' as "run" in this gate, or include removed files in the changed-files output.
• _get-changed-files.yml is not in the list, so a PR that changes the gate's own input format can silently skip this job on itself. llm-server (pull.yml:866) and test-arm-backend-docgen (pull.yml:981) both list it.
• A failed changed-files lookup silently skips the job. The new needs: changed-files (pull.yml:892) means that if the gh api .../pulls/N/files call fails on a PR, the step still succeeds: the run uses GitHub's default bash -e with no pipefail, the empty result becomes '*', and the job is skipped. On push events the job now also depends on changed-files succeeding, where it used to run unconditionally. This is low risk, and adding '*' handling as suggested above would also fix it.

Performance

• Bare substring matches cancel out part of the savings (pull.yml:896-902). Checked with git ls-files:
  • 'requirements-' matches .ci/docker/requirements-ci.txt, 7 backends/arm/requirements-arm-*.txt files and the backends/nxp/requirements-*.txt files.
  • 'install_requirements' matches about 13 backend, devtools and example scripts, such as examples/models/llama/install_requirements.sh and backends/cadence/install_requirements.sh.
  • 'setup.py' matches backends/native/python/setup.py and backends/qualcomm/utils/qnn_sdk_setup.py.
  • 'pyproject.toml' matches backends/native/python/pyproject.toml.
  • As a result, any Arm, NXP or QNN dependency bump still runs the editable suite on all 3 OSes.
  • contains() cannot anchor to the repo root on its own, but matching with a leading space (' requirements-', alongside a check for the first entry) or listing the exact root files would narrow it.

Code Quality

• The new comment (pull.yml:888-890) is inaccurate.
  • It says the job runs on "every push to main", but it also runs on release/* pushes and workflow_dispatch.
  • It says the job runs "the same unit tests" as unittest, but the regular wheel prunes unreachable test modules (setup.py:2216-2225), while the editable install exposes the whole source tree.
  • The neighbouring gated jobs have no comment at all. Per CLAUDE.md ("minimal comments"), consider dropping it or cutting it to one line.

Recommendation

Request Changes. These are small fixes to the filter list. Adding tools/cmake/, CMakePresets.json, extension/pybindings/ and .github/workflows/_get-changed-files.yml, and handling '*', would close the editable-only gaps without giving back much of the savings. Note that I can't push this edit myself, because the GitHub App cannot modify .github/workflows/.

Fix this →