mirror of
https://github.com/nestriness/nestri.git
synced 2026-09-19 17:25:19 +03:00
ci: run each half only when that half changes (#329)
## Why
Both jobs ran on every pull request. A change to a Rust binary waited on
a
Postgres service and a full TypeScript test run; a change to a
TypeScript route
spent a runner compiling Rust. Neither result told anyone anything.
## What changed
A `paths:` filter belongs to a **workflow**, not to a job — so the two
jobs
become two workflows. That is the entire cost of the change:
| | |
|---|---|
| `.github/workflows/web.yml` | the TypeScript half — `bun test` over
the control-plane apps and shared packages |
| `.github/workflows/nesdoctor.yml` | the Rust half — `fmt`, `clippy`,
`test`, and a no-network run |
| `.github/workflows/ci.yml` | deleted; it was the two of them together
|
**Both job bodies are carried over unchanged.** Parsed and compared
rather than
eyeballed:
```
web job body identical to ci.yml: True
nesdoctor job body identical to ci.yml: True
```
Only the triggers differ. `push` is untouched (see the first note
below).
## The filters, and where they come from
**`web`** — every TypeScript workspace member, plus the things that
reach all of
them. `packages/` is entirely TypeScript so it is taken whole; `apps/`
is mostly
Rust, so its two TypeScript members are named.
```
apps/api/** apps/auth/** packages/**
package.json bun.lock tsconfig.json oxlintrc.json
.github/workflows/web.yml
```
**`nesdoctor`** — its own directory, plus the workspace root and
lockfile, which
pin every version it builds against. No other member is listed because
it
depends on no other member; its dependency tree is four external crates
deep and
that is deliberate.
```
apps/nesdoctor/** Cargo.toml Cargo.lock
.github/workflows/nesdoctor.yml
```
## Verified by simulating the filters, not by reading them
The failure mode of a path filter is *silence* — a wrong pattern means
the job
never runs and the pull request goes green. So the globs were
implemented in
GitHub's dialect (`*` stops at a slash, `**` crosses them) and run
against real
change sets, including the actual file list of the last merged PR:
```
the enrolment PR, actual file list → web
a TS route only → web
a core module only → web
a migration only → web
the auth worker → web
the shared auth package → web
the bun lockfile → web
lint config → web
nesdoctor source → nesdoctor
nesdoctor README → nesdoctor
the Rust workspace root → nesdoctor
the Cargo lockfile → nesdoctor
another Rust app → (nothing)
a shared Rust crate → (nothing)
the web workflow itself → web
the nesdoctor workflow itself → nesdoctor
docs only → (nothing)
the root README → (nothing)
a stray root artefact → (nothing)
both halves at once → web, nesdoctor
```
`another Rust app` and `a shared Rust crate` firing nothing is correct
**today**
— CI covers `nesdoctor` alone, and the rest of the Rust half has never
been
under it. It stops being correct the moment a second member is added to
CI, and
each new member wants its own filter alongside its own job.
Both jobs were also run locally with the exact commands the workflows
use:
`nesdoctor` — fmt clean, clippy clean under `-D warnings`, 18 tests
pass, and
the binary runs; `web` — migrations apply and 363 tests pass, 0 fail.
## Three things found on the way, none of them fixed here
1. **`push: branches: [main]` is inert.** There is no `main` branch —
the
default is `dev` — so the push half of this trigger has never fired and
does
not fire now. I carried it over verbatim rather than "fixing" it to
`dev`,
because that would *add* CI runs and this PR exists to remove them. One
word
either way; your call.
2. **A new TypeScript app will silently not be tested** until someone
adds it to
`web.yml`'s list. Nothing detects this. It is written as a comment in
the
file, in the place someone editing that list will be looking.
3. **`nesdoctor.json` is committed at the repo root** and appears to be
a report
generated on someone's machine — it carries a specific CPU, kernel and
disk
layout. It is an output rather than an input, so no filter references
it.
Probably wants deleting and gitignoring, separately.
## Before turning on required status checks
Path-filtered workflows do not report at all when they do not match,
which
branch protection reads as *expected but missing* — a pull request that
touches
only docs would never become mergeable. There are no required checks on
`dev`
today (`required_status_checks: null`, checked), so nothing is broken by
this.
If you enable them later, the usual answer is a companion job that
always runs
and reports success under the same name.
## What this does not verify
- **The simulation implements GitHub's glob dialect; it is not GitHub.**
This
pull request is the first real exercise of it: it changes both workflow
files, each of which lists itself, so **both jobs should run here** —
which
is the intended behaviour, since a change to how the tests run is a
change
worth running. Anything else on the checks tab means a filter is wrong.
(An earlier draft of this section predicted *neither* would run. That
was
wrong, and the simulator says so: `this PR itself → web, nesdoctor`.
Left
visible because it is exactly the mistake path filters invite —
reasoning
about which files a change touches without checking.)
**Confirmed on the runner**, which is no longer a prediction:
```
nesdoctor / nesdoctor → success (pull_request)
web / web → success (pull_request)
```
- **`actionlint` was not available**, so the workflow files are
validated as
YAML and by parsing their trigger and job structure, not by a
schema-aware
linter.
- **Nothing is measured.** No before/after timings — the saving is "a
job that
had no reason to run does not run", not a number I benchmarked.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR splits the combined CI workflow into independently filtered web
and nesdoctor workflows while preserving their existing job bodies.
- Web tests now run for changes to current TypeScript workspace members
and their shared configuration.
- Nesdoctor checks now run for changes to its crate, Cargo workspace
inputs, or its workflow.
- Unrelated pull requests no longer start both test stacks.
<h3>Confidence Score: 5/5</h3>
The PR appears safe to merge; the new filters cover the current inputs
of both preserved CI jobs.
No actionable failure remains: current workspace members and build
inputs are covered, no in-repository consumer relies on the old workflow
identity, and the split does not increase permissions or action
exposure.
<h3>Important Files Changed</h3>
| Filename | Overview |
|----------|----------|
| .github/workflows/web.yml | Extracts the unchanged TypeScript test job
into a workflow filtered to all current web workspace members and
relevant shared inputs. |
| .github/workflows/nesdoctor.yml | Extracts the unchanged nesdoctor
checks into a workflow filtered to the crate and its Cargo workspace
inputs. |
<h3>Flowchart</h3>
```mermaid
%%{init: {'theme': 'neutral'}}%%
flowchart TD
PR[Pull request changes] --> W{Matches web paths?}
PR --> N{Matches nesdoctor paths?}
W -->|Yes| WT[Run Bun install, migrations, and tests]
W -->|No| WS[Skip web workflow]
N -->|Yes| NT[Run fmt, clippy, tests, and no-network smoke run]
N -->|No| NS[Skip nesdoctor workflow]
```
<sub>Reviews (1): Last reviewed commit: ["ci: run each half only when
that half
ch..."](f27ea3a132)
| [Re-trigger
Greptile](https://app.greptile.com/api/retrigger?id=60935535)</sub>
<!-- /greptile_comment -->
This commit is contained in:
48
.github/workflows/nesdoctor.yml
vendored
Normal file
48
.github/workflows/nesdoctor.yml
vendored
Normal file
@@ -0,0 +1,48 @@
|
|||||||
|
# `nesdoctor`, and deliberately nothing else in the Rust workspace.
|
||||||
|
#
|
||||||
|
# The rest of the Rust half has never been under CI, so widening this to
|
||||||
|
# `--workspace` would turn every pull request red for reasons unrelated to the
|
||||||
|
# pull request. Widen it one member at a time, as each is made to pass — and
|
||||||
|
# give each one its own `paths` filter when you do.
|
||||||
|
name: nesdoctor
|
||||||
|
|
||||||
|
on:
|
||||||
|
push:
|
||||||
|
branches: [main]
|
||||||
|
pull_request:
|
||||||
|
paths:
|
||||||
|
- "apps/nesdoctor/**"
|
||||||
|
# The workspace root pins every dependency version and owns the build
|
||||||
|
# profiles, so a change to either reaches this binary even though nothing
|
||||||
|
# under `apps/nesdoctor` moved. `Cargo.lock` is the resolved answer to
|
||||||
|
# the same question.
|
||||||
|
#
|
||||||
|
# No other member is listed because `nesdoctor` depends on no other
|
||||||
|
# member: its dependency tree is part of what it asks to be trusted on,
|
||||||
|
# and it is four external crates deep. If that ever stops being true,
|
||||||
|
# the crate it takes belongs here.
|
||||||
|
- "Cargo.toml"
|
||||||
|
- "Cargo.lock"
|
||||||
|
- ".github/workflows/nesdoctor.yml"
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
test:
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
- name: Rust toolchain
|
||||||
|
run: rustup toolchain install stable --profile minimal --component clippy,rustfmt --no-self-update
|
||||||
|
- uses: Swatinem/rust-cache@v2
|
||||||
|
with:
|
||||||
|
workspaces: ". -> target"
|
||||||
|
- name: Format
|
||||||
|
run: cargo fmt -p nesdoctor -- --check
|
||||||
|
- name: Clippy
|
||||||
|
run: cargo clippy -p nesdoctor --all-targets -- -D warnings
|
||||||
|
- name: Test
|
||||||
|
run: cargo test -p nesdoctor
|
||||||
|
# Runs without touching the network, so this stays fast and cannot fail
|
||||||
|
# on a runner's egress rules. The network path is exercised by the
|
||||||
|
# release workflow's smoke test, where it belongs.
|
||||||
|
- name: Runs at all
|
||||||
|
run: cargo run -p nesdoctor -- --quiet --no-net --no-steam --json "$RUNNER_TEMP/nd.json" < /dev/null
|
||||||
@@ -1,12 +1,35 @@
|
|||||||
name: CI
|
# The TypeScript half: the control-plane apps and the packages they share.
|
||||||
|
#
|
||||||
|
# Split from the Rust half rather than being two jobs in one workflow, because
|
||||||
|
# a job cannot carry its own `paths` filter — only a workflow can. Two files is
|
||||||
|
# what "run this only when its own code changes" costs, and the alternative
|
||||||
|
# spends a runner on every pull request deciding it has nothing to do.
|
||||||
|
name: web
|
||||||
|
|
||||||
on:
|
on:
|
||||||
push:
|
push:
|
||||||
branches: [main]
|
branches: [main]
|
||||||
pull_request:
|
pull_request:
|
||||||
|
paths:
|
||||||
|
# Every TypeScript workspace member. `packages/` is entirely TypeScript
|
||||||
|
# so it is taken whole; `apps/` is mostly Rust, so its two members are
|
||||||
|
# named. **Adding a TypeScript app means adding it here** — nothing
|
||||||
|
# detects that on its own, and the failure is silent: the tests do not
|
||||||
|
# run and the pull request goes green.
|
||||||
|
- "apps/api/**"
|
||||||
|
- "apps/auth/**"
|
||||||
|
- "packages/**"
|
||||||
|
# The workspace itself. A lockfile change reaches every member, and the
|
||||||
|
# linter and compiler settings decide whether any of it passes.
|
||||||
|
- "package.json"
|
||||||
|
- "bun.lock"
|
||||||
|
- "tsconfig.json"
|
||||||
|
- "oxlintrc.json"
|
||||||
|
# This file. A change to how the tests run is a change worth running.
|
||||||
|
- ".github/workflows/web.yml"
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
web:
|
test:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
services:
|
services:
|
||||||
postgres:
|
postgres:
|
||||||
@@ -42,28 +65,3 @@ jobs:
|
|||||||
run: bun test
|
run: bun test
|
||||||
env:
|
env:
|
||||||
TEST_DATABASE_URL: postgres://postgres:postgres@localhost:5432/nestri
|
TEST_DATABASE_URL: postgres://postgres:postgres@localhost:5432/nestri
|
||||||
|
|
||||||
# Scoped to `nesdoctor` deliberately. The rest of the Rust half has never
|
|
||||||
# been under CI, so widening this to `--workspace` would turn every PR red
|
|
||||||
# for reasons unrelated to the PR. Widen it one member at a time, as each is
|
|
||||||
# made to pass.
|
|
||||||
nesdoctor:
|
|
||||||
runs-on: ubuntu-latest
|
|
||||||
steps:
|
|
||||||
- uses: actions/checkout@v4
|
|
||||||
- name: Rust toolchain
|
|
||||||
run: rustup toolchain install stable --profile minimal --component clippy,rustfmt --no-self-update
|
|
||||||
- uses: Swatinem/rust-cache@v2
|
|
||||||
with:
|
|
||||||
workspaces: ". -> target"
|
|
||||||
- name: Format
|
|
||||||
run: cargo fmt -p nesdoctor -- --check
|
|
||||||
- name: Clippy
|
|
||||||
run: cargo clippy -p nesdoctor --all-targets -- -D warnings
|
|
||||||
- name: Test
|
|
||||||
run: cargo test -p nesdoctor
|
|
||||||
# Runs without touching the network, so this stays fast and cannot fail
|
|
||||||
# on a runner's egress rules. The network path is exercised by the
|
|
||||||
# release workflow's smoke test, where it belongs.
|
|
||||||
- name: Runs at all
|
|
||||||
run: cargo run -p nesdoctor -- --quiet --no-net --no-steam --json "$RUNNER_TEMP/nd.json" < /dev/null
|
|
||||||
Reference in New Issue
Block a user