Skip to content

fix(test): remove shell-escaped newline from hooks runner test#2187

Open
panandicoding wants to merge 1 commit into
MoonshotAI:mainfrom
panandicoding:fix/test-hooks-shell-escaping
Open

fix(test): remove shell-escaped newline from hooks runner test#2187
panandicoding wants to merge 1 commit into
MoonshotAI:mainfrom
panandicoding:fix/test-hooks-shell-escaping

Conversation

@panandicoding

Copy link
Copy Markdown

Related Issue

N/A (small test-only fix < 100 lines)

Description

The externalHooksRunner test "does not dedupe hooks that share a command but have different cwd" appends "\\n" to process.stdout.write(process.cwd()) via shell escaping. On Windows, cmd.exe misinterprets the escaped sequence and outputs the literal two characters \n instead of a newline, causing .trim() to fail to strip it and the assertion to fail.

Since the assertion already applies .trim() to the stdout, the trailing newline is unnecessary. Removing it makes the test pass on both Unix and Windows.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.

The test appended a literal backslash-n via shell escaping which is misinterpreted on Windows (cmd.exe produces the two characters backslash+n instead of a newline). Since the assertion already uses .trim(), the trailing newline is unnecessary.
@changeset-bot

changeset-bot Bot commented Jul 25, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d2833cf

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant