mirror of
https://github.com/affaan-m/ECC.git
synced 2026-08-17 21:15:40 +02:00
5deee34c93
* fix(hooks): remove stray '?' that made every 'yarn <anything>' trigger tmux reminder
The tmux-reminder matcher uses one alternation per package manager. Each
branch requires a subcommand (install|test) — except yarn, whose subcommand
group carried a trailing `?`:
yarn (install|test)?
That made the subcommand optional, so the branch degraded to "yarn " plus
anything: `yarn add foo`, `yarn build`, `yarn dev`, even `yarn --version`
all matched and spammed the "Consider running in tmux" hint into the
additional-context channel.
Drop the `?` so yarn matches parity with npm/pnpm/bun. Verified locally
against 14 cases (yarn install/test still fire; yarn add/build/dev/… no
longer do; npm/pnpm/bun/pytest behavior unchanged).
Fixes #2514
* test(hooks): add pre-bash-tmux-reminder regression tests
Add coverage for the tmux-reminder matcher following the auto-tmux-dev.test.js
structure — the regex-first hook now has direct regression tests for the yarn
branch fix in this PR (and for the sibling package managers, other matched
tools, TMUX bypass, and malformed input).
16 assertions total:
- fires for: yarn install, yarn test, npm install, pnpm test, bun install,
pytest tests/, cargo build
- does NOT fire for: yarn add react, yarn build, yarn dev, yarn --version,
bare `yarn`, npm run dev
- respects TMUX env var
- tolerates invalid JSON and missing command field
Verified the tests actually catch the bug: reintroducing the buggy
`yarn (install|test)?` fails 4 of the 5 yarn non-match cases (the fifth,
bare `yarn`, stays passing because even the buggy branch requires a trailing
space after yarn).
Addresses CodeRabbit review on #2517.
* test(hooks): fail loudly on spawn errors, use destructuring, split runTests
Address three CodeRabbit review notes on tests/hooks/pre-bash-tmux-reminder.test.js:
- Fail loudly on spawnSync errors: raise instead of coercing
`result.status || 0`, which would mask spawn errors, timeouts, or signal
termination as a successful exit 0 (masks legitimate test failures).
- Use destructuring (`const { TMUX, ...env } = process.env`) instead of
copy-then-`delete` so the base env is built immutably.
- Split `runTests` (was 66 lines) into small per-group helpers
(runYarnTests, runSiblingPackageManagerTests, runOtherToolTests,
runTmuxBypassTests, runEdgeCaseTests). `runTests` is now 18 lines and
purely orchestrates.
16 assertions still pass; no coverage changes.
The 4th CodeRabbit note (avoid console.log in test files) is intentionally
not adopted here — every sibling hook test in this repo
(auto-tmux-dev.test.js, bash-hook-dispatcher.test.js, block-no-verify.test.js,
etc.) writes to console.log because the project's own test runner
(tests/run-all.js) is console-log based and there is no Jest/Mocha
dependency. Diverging from the established convention in a bugfix PR is
out of scope.
* test(hooks): trim tmux reminder regression coverage
---------
Co-authored-by: Haley Chen <2022hachen@gmail.com>
79 lines
2.3 KiB
JavaScript
79 lines
2.3 KiB
JavaScript
const assert = require('assert');
|
|
const path = require('path');
|
|
const { spawnSync } = require('child_process');
|
|
|
|
const script = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'pre-bash-tmux-reminder.js');
|
|
|
|
function run(command, extraEnv = {}) {
|
|
const { TMUX: _tmux, ...envWithoutTmux } = process.env;
|
|
const result = spawnSync(process.execPath, [script], {
|
|
encoding: 'utf8',
|
|
input: JSON.stringify({ tool_input: { command } }),
|
|
timeout: 10000,
|
|
env: { ...envWithoutTmux, ...extraEnv }
|
|
});
|
|
|
|
if (result.error) throw result.error;
|
|
if (result.signal) throw new Error(`hook terminated by ${result.signal}`);
|
|
|
|
assert.strictEqual(result.status, 0, `unexpected exit for ${command}: ${result.stderr || ''}`);
|
|
return result.stdout || '';
|
|
}
|
|
|
|
function hasReminder(command, extraEnv) {
|
|
return run(command, extraEnv).includes('Consider running in tmux');
|
|
}
|
|
|
|
function runTests() {
|
|
console.log('\n=== Testing pre-bash-tmux-reminder.js ===\n');
|
|
|
|
if (process.platform === 'win32') {
|
|
console.log(' SKIP: hook is a no-op on win32');
|
|
return true;
|
|
}
|
|
|
|
const cases = [
|
|
['fires for yarn install and yarn test', () => {
|
|
assert.ok(hasReminder('yarn install'));
|
|
assert.ok(hasReminder('yarn test'));
|
|
}],
|
|
['does not fire for ordinary yarn commands', () => {
|
|
assert.ok(!hasReminder('yarn add react'));
|
|
assert.ok(!hasReminder('yarn build'));
|
|
assert.ok(!hasReminder('yarn dev'));
|
|
assert.ok(!hasReminder('yarn --version'));
|
|
assert.ok(!hasReminder('yarn'));
|
|
}],
|
|
['keeps sibling package-manager behavior', () => {
|
|
assert.ok(hasReminder('npm install'));
|
|
assert.ok(hasReminder('pnpm test'));
|
|
assert.ok(hasReminder('bun install'));
|
|
assert.ok(!hasReminder('npm run dev'));
|
|
}],
|
|
['suppresses reminders inside tmux', () => {
|
|
assert.ok(!hasReminder('yarn install', { TMUX: '/tmp/tmux-1000/default,1,0' }));
|
|
}]
|
|
];
|
|
|
|
let failed = 0;
|
|
for (const [name, fn] of cases) {
|
|
try {
|
|
fn();
|
|
console.log(` PASS ${name}`);
|
|
} catch (error) {
|
|
failed++;
|
|
console.log(` FAIL ${name}`);
|
|
console.log(` ${error.message}`);
|
|
}
|
|
}
|
|
|
|
console.log(`\nResults: ${cases.length - failed} passed, ${failed} failed\n`);
|
|
return failed === 0;
|
|
}
|
|
|
|
if (require.main === module) {
|
|
process.exit(runTests() ? 0 : 1);
|
|
}
|
|
|
|
module.exports = { runTests };
|