Fix issue:validate tests on Windows - #49523
Open
LuckWzx wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Windows-specific failures in the issue:validate test suite by avoiding shell argument parsing/quoting issues when passing multiline issue bodies to the validator command.
Changes:
- Replace
execSync(shell-based) withexecFileSyncusing an args array to preserve multiline--bodyvalues on Windows. - Invoke the validator directly via
node --import tsx scripts/commands/issue/validate.tsinstead ofnpm run .... - Replace
cross-envusage withenvinjection via theexecFileSyncoptions.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Problem
The
issue:validatetests fail on Windows:npm testreportsin all three test cases (
streams:add,streams:remove,streams:edit).Root cause: the tests built a shell command and ran it with
execSync:On Windows,
execSyncexecutes throughcmd.exe, which truncates the--bodyargument at the first newline — the issue body is inherentlymultiline. The script only received
--body=### Stream IDand lost the--labelsoption entirely (verified with a minimal repro:["--body=### Stream ID"]was the entire argv). With no labels matching,none of the
streams:add/streams:remove/streams:editbranches run,so the command exits with code
0and never writeserrors.txt, and thetest fails trying to read it.
This is Windows-specific: the production workflow
(
.github/workflows/validate_issue.yml) passes the body through a bashvariable (
--body "$RAW_BODY") onubuntu-latest, where multiline quotedarguments work correctly.
Changes
In
tests/commands/issue/validate.test.ts:execSyncwithexecFileSync, passing the multiline issue bodyand labels as an args array — no shell involved, so no quoting or
newline truncation issues.
node --import tsx scripts/commands/issue/validate.tsinstead ofnpm run.envoption instead ofcross-env.Verification
npm test→ 19/19 tests pass (10/10 suites), including the previouslyfailing
tests/commands/issue/validate.test.ts.npm run lint→ no errors.Note (alternative approach)
An alternative fix would be to add a
--body-file <path>option toscripts/commands/issue/validate.tsand have the test write the body to atemporary file. This keeps the
npm runinvocation pattern but changesproduction code. The chosen test-only fix keeps the change minimal and has
no impact on the CLI or the GitHub Actions workflow. Happy to switch if the
maintainers prefer the other approach.