build(hooks): widen pre-push fast path to test/ci/.githooks + guard test (T-393)
The fast-path regex matched only `^(lib/|pubspec\.)`, so a push that ONLY changed a test, a ci/ gate script, or the pre-push hook itself skipped the entire ~2min suite — exactly the paths most able to break the suite or the gate went unchecked. Widen the trigger to include test/, ci/, and .githooks/, and extract it to a single `trigger_re` variable. Also drop the header comment's false claim that "release CI runs it forced on a tagged version" — there is no release CI. Add test/tooling/pre_push_hook_test.dart: it reads the live trigger_re from the hook and asserts the load-bearing dirs force the full gate while docs/assets ride along — so the regex can't be silently narrowed again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+20
-12
@@ -5,14 +5,16 @@
|
||||
# Bypass: never. If this runs slowly, fix the slow test; don't reach
|
||||
# for --no-verify (git-commit skill forbids it).
|
||||
#
|
||||
# Fast path (T-348): run the full ~2min test suite only when the push touches
|
||||
# lib/ (app + runtime Dart source) or pubspec.* (deps / version). test/,
|
||||
# assets/, docs, and tooling changes ride along with a lib change in practice,
|
||||
# and an otherwise-skipped push is covered by the next one that does touch lib.
|
||||
# The full suite is always available via `make push-check`, and the release CI
|
||||
# runs it forced on a tagged version. So a lib/pubspec-free push runs just the
|
||||
# instant decisions + changelog gates. A state we can't classify (unfetched
|
||||
# remote, new branch) runs the full gate.
|
||||
# Fast path (T-348, widened T-393): run the full ~2min test suite when the push
|
||||
# touches lib/ (app + runtime Dart source), pubspec.* (deps / version), or the
|
||||
# things that can themselves break the suite or this gate — test/, ci/, and
|
||||
# .githooks/. (The old regex matched only lib/ and pubspec.*, so a push that
|
||||
# ONLY changed a test, a ci/ gate script, or this hook skipped the whole suite.)
|
||||
# Pure assets/docs changes still ride along with the next lib-touching push.
|
||||
# There is no release CI — the full suite is only ever run here or via
|
||||
# `make push-check`. So a push touching none of the above runs just the instant
|
||||
# decisions + changelog gates. A state we can't classify (unfetched remote, new
|
||||
# branch) runs the full gate.
|
||||
set -euo pipefail
|
||||
|
||||
cd "$(git rev-parse --show-toplevel)"
|
||||
@@ -40,16 +42,22 @@ while read -r _local_ref local_sha _remote_ref remote_sha; do
|
||||
changed+=$'\n'"$(git diff --name-only "$base" "$local_sha")"
|
||||
done
|
||||
|
||||
# Run the full gate when lib/ (app + runtime source) or pubspec.* (deps /
|
||||
# version) is touched, or when we couldn't classify above.
|
||||
# Paths that force the full gate: source (lib/), deps/version (pubspec.*), and
|
||||
# the dirs that can themselves break the suite or this gate (test/, ci/,
|
||||
# .githooks/). Single source of truth — test/tooling/pre_push_hook_test.dart
|
||||
# reads this exact pattern, so narrowing it fails that test (the T-393 guard).
|
||||
trigger_re='^(lib/|test/|ci/|\.githooks/|pubspec\.)'
|
||||
|
||||
# Run the full gate when a trigger path is touched, or when we couldn't classify
|
||||
# above.
|
||||
needs_gate=1
|
||||
if [[ "$force_full" -eq 0 ]]; then
|
||||
trigger_files="$(printf '%s\n' "$changed" | grep -E '^(lib/|pubspec\.)' || true)"
|
||||
trigger_files="$(printf '%s\n' "$changed" | grep -E "$trigger_re" || true)"
|
||||
[[ -z "$trigger_files" ]] && needs_gate=0
|
||||
fi
|
||||
|
||||
if [[ "$needs_gate" -eq 0 ]]; then
|
||||
echo "==> pre-push: no lib/ or pubspec change — decisions + changelog gates, skipping tests"
|
||||
echo "==> pre-push: no source/test/ci/hook/pubspec change — decisions + changelog gates, skipping tests"
|
||||
make decisions-validate changelog-gate
|
||||
else
|
||||
echo "==> pre-push: make push-check"
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
/// T-393: guard the pre-push fast-path classifier.
|
||||
///
|
||||
/// The hook runs the full ~2min gate only when the pushed diff touches a
|
||||
/// "trigger" path; everything else rides along on the next triggering push.
|
||||
/// The regex once matched ONLY `lib/` + `pubspec.*`, so a push that ONLY
|
||||
/// changed a test, a ci/ gate script, or the hook itself skipped the entire
|
||||
/// suite. This test reads the live `trigger_re` from `.githooks/pre-push` (the
|
||||
/// single source of truth) and asserts the load-bearing dirs force the gate, so
|
||||
/// the regex can't be silently narrowed back.
|
||||
@TestOn('vm')
|
||||
library;
|
||||
|
||||
import 'dart:io';
|
||||
|
||||
import 'package:test/test.dart';
|
||||
|
||||
void main() {
|
||||
final hook = File('.githooks/pre-push');
|
||||
|
||||
test('the pre-push hook exists', () {
|
||||
expect(hook.existsSync(), isTrue, reason: 'expected .githooks/pre-push at the repo root');
|
||||
});
|
||||
|
||||
// Extract `trigger_re='...'` from the hook and apply it the way grep -E does.
|
||||
final src = hook.readAsStringSync();
|
||||
final m = RegExp(r"""trigger_re='([^']*)'""").firstMatch(src);
|
||||
|
||||
test('the hook defines a trigger_re pattern', () {
|
||||
expect(m, isNotNull, reason: 'could not find trigger_re=\'...\' in .githooks/pre-push');
|
||||
});
|
||||
|
||||
final pattern = RegExp(m!.group(1)!);
|
||||
bool forcesGate(String path) => pattern.hasMatch(path);
|
||||
|
||||
group('forces the full gate', () {
|
||||
const triggers = [
|
||||
'lib/main.dart',
|
||||
'lib/src/editor/registry.dart',
|
||||
'test/foo_test.dart',
|
||||
'test/a11y/contrast_test.dart',
|
||||
'ci/test.sh',
|
||||
'ci/changelog_gate.sh',
|
||||
'.githooks/pre-push',
|
||||
'pubspec.yaml',
|
||||
'pubspec.lock',
|
||||
];
|
||||
for (final p in triggers) {
|
||||
test(p, () => expect(forcesGate(p), isTrue, reason: '$p must run the full gate'));
|
||||
}
|
||||
});
|
||||
|
||||
group('rides along (no full gate)', () {
|
||||
// Docs, assets, governance, and top-level notes can ride along with the
|
||||
// next lib-touching push — they cannot break the test suite or the gate.
|
||||
const ridesAlong = ['docs/initial-plan.md', 'assets/logo/logo.svg', 'governance/decisions/process.md', 'README.md', 'CHANGELOG.md'];
|
||||
for (final p in ridesAlong) {
|
||||
test(p, () => expect(forcesGate(p), isFalse, reason: '$p should not force the full gate'));
|
||||
}
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user