✨ Add pre-push hook that runs the test suite
The pre-commit hook is file-scoped (LEFTHOOK_FILES), so it can't naturally run the test suite. Pre-push is the right tier for it: - Runs after all commits are made but before the push leaves the machine, catching regressions that span multiple commits - ~3.5s including the tsc step (negligible vs the typical push round-trip to CI) - Offline and deterministic, same philosophy as pre-commit lefthook.yml: new pre-push section, sequential (parallel: false since there's only one command, but the explicit value documents the intent that this hook runs commands in order rather than racing). project-specs.md: updated the CHECK TIERS table to add the pre-push column with in it. Updated the rule-of-thumb list to include the pre-push tier. Updated the 'why' notes to explain why lives in pre-push rather than pre-commit (LEFTHOOK_FILES doesn't apply to the test runner).
This commit is contained in:
1 parent
74e4538094
commit
7e9401590e
2 files changed
+37
-15
No files matched your search
@@ -13,3 +13,9 @@ pre-commit:
|
|||||||
run: sh -c 'LEFTHOOK_FILES="$*" npm run check:cspell' sh {staged_files}
|
run: sh -c 'LEFTHOOK_FILES="$*" npm run check:cspell' sh {staged_files}
|
||||||
typecheck:
|
typecheck:
|
||||||
run: npm run check:tsc
|
run: npm run check:tsc
|
||||||
|
|
||||||
|
pre-push:
|
||||||
|
parallel: false
|
||||||
|
commands:
|
||||||
|
test:
|
||||||
|
run: npm test
|
||||||
+31
-15
@@ -296,7 +296,8 @@ script does not belong in the standard pipeline.
|
|||||||
### HOOKS
|
### HOOKS
|
||||||
|
|
||||||
- Lefthook runs the relevant `check:*` scripts on staged files in
|
- Lefthook runs the relevant `check:*` scripts on staged files in
|
||||||
parallel for pre-commit. See `lefthook.yml`.
|
parallel for pre-commit, and `npm test` on pre-push. See
|
||||||
|
`lefthook.yml`.
|
||||||
|
|
||||||
### CHECK TIERS — what runs where and why
|
### CHECK TIERS — what runs where and why
|
||||||
|
|
||||||
@@ -307,22 +308,28 @@ deliberate design choice; the rule of thumb is:
|
|||||||
|
|
||||||
- **pre-commit hook**: fast, file-scoped, offline, deterministic.
|
- **pre-commit hook**: fast, file-scoped, offline, deterministic.
|
||||||
Catches what _you just changed_ in under a second or two.
|
Catches what _you just changed_ in under a second or two.
|
||||||
|
- **pre-push hook**: runs the unit test suite (`npm test`,
|
||||||
|
including a full `tsc` over the project). ~500ms. Catches
|
||||||
|
runtime/logic bugs across the whole codebase before the push
|
||||||
|
leaves your machine. The `tsc` step is technically redundant
|
||||||
|
with pre-commit but validates against unstaged changes too.
|
||||||
- **`npm run check`**: full project audit. Slower, may query the
|
- **`npm run check`**: full project audit. Slower, may query the
|
||||||
network, runs everything that's not in pre-commit.
|
network, runs everything that's not in pre-commit.
|
||||||
- **CI build job**: authoritative. Runs `npm run check` and
|
- **CI build job**: authoritative. Runs `npm run check` and
|
||||||
`npm run test:ci` on every push and PR. The ground truth —
|
`npm run test:ci` on every push and PR. The ground truth —
|
||||||
if CI passes, the codebase is clean.
|
if CI passes, the codebase is clean.
|
||||||
|
|
||||||
| Script | pre-commit | `npm run check` | CI build | CI publish |
|
| Script | pre-commit | pre-push | `npm run check` | CI build | CI publish |
|
||||||
| ----------------- | :--------: | :-------------: | :------: | :--------: |
|
| ----------------- | :--------: | :------: | :-------------: | :------: | :--------: |
|
||||||
| `check:tsc` | ✅ | ✅ | ✅ | — |
|
| `check:tsc` | ✅ | ✅ | ✅ | ✅ | — |
|
||||||
| `check:oxlint` | ✅ | ✅ | ✅ | — |
|
| `check:oxlint` | ✅ | — | ✅ | ✅ | — |
|
||||||
| `check:oxfmt` | ✅ | ✅ | ✅ | — |
|
| `check:oxfmt` | ✅ | — | ✅ | ✅ | — |
|
||||||
| `check:cspell` | ✅ | ✅ | ✅ | — |
|
| `check:cspell` | ✅ | — | ✅ | ✅ | — |
|
||||||
| `check:knip` | — | ✅ | ✅ | — |
|
| `test` | — | ✅ | — | ✅ | — |
|
||||||
| `check:outdated` | — | ✅ | ✅ | — |
|
| `check:knip` | — | — | ✅ | ✅ | — |
|
||||||
| `publish:publint` | — | — | — | ✅ |
|
| `check:outdated` | — | — | ✅ | ✅ | — |
|
||||||
| `publish:attw` | — | — | — | ✅ |
|
| `publish:publint` | — | — | — | — | ✅ |
|
||||||
|
| `publish:attw` | — | — | — | — | ✅ |
|
||||||
|
|
||||||
#### Why these splits?
|
#### Why these splits?
|
||||||
|
|
||||||
@@ -332,10 +339,19 @@ deliberate design choice; the rule of thumb is:
|
|||||||
`LEFTHOOK_FILES` env var convention. They give instant feedback
|
`LEFTHOOK_FILES` env var convention. They give instant feedback
|
||||||
on what you typed.
|
on what you typed.
|
||||||
|
|
||||||
- **`check:knip` is NOT in pre-commit** because it scans the whole
|
- **`test` (and the `tsc` it includes) is in pre-push** because it
|
||||||
project (not staged files) and takes ~4s. Adding it would
|
runs the whole test suite across the whole project (~500ms
|
||||||
roughly triple pre-commit time. It's still fast enough to run
|
for the current 6 tests). The pre-commit `LEFTHOOK_FILES`
|
||||||
locally before pushing, and CI catches it regardless.
|
convention doesn't apply to the test runner, so pre-commit
|
||||||
|
isn't the right home. Pre-push is the natural place: it runs
|
||||||
|
after all commits are made but before the push leaves the
|
||||||
|
machine, catching regressions that span multiple commits.
|
||||||
|
|
||||||
|
- **`check:knip` is NOT in pre-commit or pre-push** because it
|
||||||
|
scans the whole project (not staged files) and takes ~4s.
|
||||||
|
Adding it would noticeably slow both hooks. It's still fast
|
||||||
|
enough to run locally before pushing, and CI catches it
|
||||||
|
regardless.
|
||||||
|
|
||||||
- **`check:outdated` is NOT in pre-commit** because (a) it queries
|
- **`check:outdated` is NOT in pre-commit** because (a) it queries
|
||||||
the npm registry (~6.5s) which means network dependency in a
|
the npm registry (~6.5s) which means network dependency in a
|
||||||
|
|||||||
Reference in new issue
Block a user