fix(integration-tests): honor stdinDoesNotEnd option#2966
Open
chinesepowered wants to merge 1 commit intoQwenLM:mainfrom
Open
fix(integration-tests): honor stdinDoesNotEnd option#2966chinesepowered wants to merge 1 commit intoQwenLM:mainfrom
chinesepowered wants to merge 1 commit intoQwenLM:mainfrom
Conversation
The stdinDoesNotEnd option was completely broken. The original code had a conditional stdin.end() scoped to object-type promptOrOptions, followed by an unconditional stdin.end() that always ran — so stdinDoesNotEnd: true had no effect. Restructure as an explicit keepStdinOpen check: close stdin unless the caller passed an options object with stdinDoesNotEnd: true. The string- prompt call path still closes stdin, and null is guarded (typeof null === 'object' in JS).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix
stdinDoesNotEndtest option being completely non-functional.TLDR
integration-tests/test-helper.tshad a conditionalchild.stdin.end()scoped to object-typepromptOrOptions, followed immediately by an unconditionalchild.stdin.end(). The unconditional call always ran, so{ stdinDoesNotEnd: true }had no effect — any integration test that tried to keep stdin open for streaming input had it closed anyway. This PR restructures the check so stdin stays open exactly whenstdinDoesNotEnd: truewas explicitly requested.Screenshots / Video Demo
N/A — test-infrastructure fix, no user-visible UI.
Dive Deeper
TestRig.run()acceptspromptOrOptions: string | { prompt?; stdin?; stdinDoesNotEnd? }. The broken code was:Simply deleting the unconditional line would have broken the string-prompt path — when the caller passes
rig.run('say hi')thetypeof 'object'guard is false, the conditional skips, and the stray unconditional call was actually the catch-all that closed stdin for string callers. Removing it without restructuring the guard would cause every string-prompt test to hang waiting for EOF.The correct fix inverts the logic: default to closing stdin, and only keep it open when
stdinDoesNotEnd: trueis explicitly passed on an options object.Coverage of all call patterns:
keepStdinOpenstdin.end()called?run('say hi')(string)falserun({ prompt: 'hi' })falserun({ stdinDoesNotEnd: true })truerun({ stdinDoesNotEnd: false })falserun(null)(null istypeof 'object')falseThe explicit
!== nullguard covers the JavaScript quirk thattypeof null === 'object'— otherwise the first clause would pass and we'd dereference.stdinDoesNotEndonnull.Modified file:
integration-tests/test-helper.ts— restructured stdin-close logicReviewer Test Plan
stdinDoesNotEndshould still close stdin normallystdinDoesNotEnd: trueshould now actually keep stdin open — this path was previously dead codegrep -n "stdin!.end" integration-tests/test-helper.ts— should show a single call inside theif (!keepStdinOpen)blockTesting Matrix