Add package CI content filter to skip dependency-only changes#23192
Add package CI content filter to skip dependency-only changes#23192Sewer56 wants to merge 2 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
|
|
||
| - name: Use Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version-file: package.json | ||
|
|
||
| - name: Test package CI filter | ||
| run: node --test .github/scripts/package-ci-filter.test.mjs |
GitHub Actions path filters are file-level only, so dependency-only edits in pnpm-workspace.yaml, package.json, or src/main/package.json unnecessarily trigger the full Windows packaging job. Add a second-stage filter script (package-ci-filter.mjs) that inspects changed file contents and only runs Package CI when package-relevant fields change: - pnpm deploy/security settings (minimumReleaseAge, trustPolicy, etc.) - root package.json runtime inputs (packageManager, engines, scripts) - src/main/package.json metadata, scripts, and nx targets - electron-builder config (always) Dependency-only changes (catalog additions, new deps) now skip Package CI, since Main CI already covers install, build, and test failures. Also adds: - Validation that alwaysRun paths exist (fails with annotation if drifted) - Unit tests covering dependency-only skips and package-triggering edits - CI workflow to run filter tests on PR
Sewer56
force-pushed
the
task/filter-package-tests
branch
from
301c7bc to
0388095
Compare
|
I don't believe this approach is maintainable nor a good tradeoff. Package runs nightly so if a PR breaks it you'd only know on the next run. If you want to know the exact PR that broke the pipeline you have to run the pipeline on every PR. So why don't we run the package pipeline on every PR? With the recent move to nx for task orchestration I changed our performance profile for pipelines. You don't have to manually select the projects you want to build anymore, you just build everything and not have to worry about it taking forever because of nx caching feature. If you're worried about the pipeline taking too long, then there are many things we can do to improve it. I haven't enabled nx caching on CI yet because I want to validate it locally first but that would easily cut the package workflow time from 20 minutes to 5 minutes or less. |
|
I appreciate the feedback, but I disagree on a few points. Why not run Package on every PR?Package should only run on changes that affect packaging. Build CI already covers install, build, and test - most PRs don't need packaging. Running it on every dependency bump is wasteful for time and the environment. This PR is an extra safeguard - minimum effort, maximum gain. Catches the specific breakages build doesn't (#23157) without penalising every PR. The time argument cuts both waysIn the context of formatting, ~2m5s was previously flagged by some folks - yourself included - as unacceptable for a blocking CI failure or run due to being a hit on productivity. Now a hypothetical 5 minutes for packaging on every PR is fine? Current reality is 20. The Windows package workflow is flaky. Being blocked from merging because a 20-minute workflow randomly failed, forcing a restart and another wait, is a real productivity hit. I'm aware Nx caching could improve things, but until validated and in place, the current reality is a 20-minute flaky workflow. MaintainabilityCalling this "unmaintainable" ignores that times have changed. I may have agreed with this 2 years ago, but this can now be done by an LLM in under 5 minutes. Whole script, maybe in 10; just with less polish. The filter is a small, easy to read script - tests are just a sanity check. Switching from Not like you'll ever really need to update it; until updating e.g. from Flaky CI is outside our scopeFixing the flaky Windows CI has been assigned to Platform, and we've been told this isn't our responsibility (unfortunately, in this case). We also shouldn't design CI gating around the assumption a workflow we don't own will become reliable. |
|
This PR has been marked as stale due to inactivity. |
3 participants
Extracted from #23164.
Why
Build does not catch packaging breaks.
As an example, #23157 (pnpm 10 to 11) changed
minimumReleaseAge, build and other checks passed, but packaging unexpectedly broke without any indication in CI.This PR makes us run the
PackageCI workflow on PRs that change project-wide settings which the build check does not validate, so breaks are caught at the source instead of discovered later.What
Run Package workflow when any of the following change:
packageManager,engines)electron-builderconfigI try to be conservative here with an opt-in approach; so only specified files+fields are checked.
This means that differences such as dependencies are skipped from checks; as regular build CI workflow would handle that just fine.
Safety
A tiny mechanism. If files are moved/renamed, CI fails with an error annotation describing the issue clearly.
Files
.github/scripts/package-ci-filter.mjs.github/scripts/package-ci-filter.test.mjs.github/workflows/package-ci-filter-test.yml.github/workflows/package.ymlchangesjob +ifgate onbuild