Repository navigation
test(cli): --status regression test reaches the TTY branch via an isTTY shim - #354
Conversation
…TY shim The test added in #351 spawns cli.js with piped stdin, so isTTY is never true and the test passes with or without the !statusOnly fix it guards. Preload a data: module that reports stdin as a TTY so the interactive decision is observable, and keep a positive control showing the same shim without --status takes the ask path. Closes #353 Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Read against src/cli.ts and src/resolver.ts at main: the test does what it claims and looks correct to me.
src/cli.ts:118—interactive = !flags['no-interactive'] && !statusOnly && process.stdin.isTTY === true. The--import data:shim setsisTTYtotrueon the spawned process beforecli.jsloads, so this is the first test that actually evaluates the!statusOnlyterm rather than short-circuiting onisTTY.src/resolver.ts:230-237— withinput: ''stdin hits EOF immediately,rl.on('close')fires and rejects withVariable "file" has no value and stdin is closed, which is exactly the positive-control assertion. Deterministic, no timing dependence.- The positive control (exit 1 without
--status) is the important half: it is what makes the--statusassertion falsifiable if someone drops!statusOnlyfrom the condition. --importwith adata:URL needs Node ≥ 20.6;engines.nodeis>=22and the CI matrix is 22/24/26, so no portability concern.dist/cli.test.js+.mapare committed, which the dist-sync gate requires;verifyis green.
Test-only change plus changelog fragment. Not approving — that needs a human signature.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
Test Improvement
The regression test added in #351 ("
--status alone never prompts or errors on an unfilled blank") spawnscli.jswith piped stdin, soprocess.stdin.isTTYis nevertrueandinteractiveis alreadyfalsewith or without the!statusOnlyterm the test is meant to guard. Reverting that one-line fix left the suite green (512/512).This PR adds
--status never asks even when stdin is a TTY, which spawns the CLI with--import 'data:text/javascript,Object.defineProperty(process.stdin,"isTTY",{value:true})'so the TTY branch is reached without a pty (works on both the ubuntu and macOS matrix legs). A positive control in the same test runs the identical shim without--statusand asserts the ask path fails with thestdin is closeddiagnostic, so the--statusassertion cannot be satisfied vacuously.Validation at
8d26afe, Node v26.12.0:npm test— 513/513 pass, coverage 99.60% lines / 97.69% branches / 100% functions (gate unchanged)!statusOnlyterm removed fromdist/cli.js, the new test fails (actual: 1, expected0); the pre-existing test still passeddist/cli.test.js+.mapregenerated for the dist-sync gate;changelog.d/added-status-tty-regression-test.mdaddedCloses #353
Filed by quality agent (ACMM L4/L6 — full mode)
— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88