fix(playwright): shell- and regex-escape test paths#134
Open
pbomb wants to merge 2 commits into
Open
Conversation
Test file paths containing shell metacharacters like (, ), $, [, ], or spaces broke the shell when Knapsack invoked Playwright via `shell: true`. This happens in real-world repos that use route-group folder names like `pages/(layout)/...`. Single-quote each path before joining.
Playwright's CLI treats positional args as regex filters against test file paths. Paths containing regex metacharacters like `(`, `)`, `.`, `[`, `]`, `*`, etc. don't match — Playwright errors with "No tests found". Real-world repos with route-group folders like `pages/(layout)/` hit this. Regex-escape each path before shell-quoting.
Contributor
|
Hey Matt, thanks for the PR and the detailed description. We'll review as soon as possible. I see this is a draft, are you planning to do more work on this? Does this fix all the issues you have with PW? |
Author
|
Hi @3v0k4. I just marked it ready to review after bringing another change that actually fixed the issue for us. It wasn't quite right after the first commit. Happy to work with you on and test out any different approaches you might have. |
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
packages/playwright'sonSuccessjoins test file paths with spaces and passes them tonpx playwright testviashell: true. Two related bugs make this break on real-world repos:1. Shell metacharacters break
/bin/shparsingPaths are not shell-escaped, so any shell metacharacter in a path breaks the parser before Playwright ever runs. The batch never produces
.knapsack-pro/batch.json, and the nextreadFileSync('.knapsack-pro/batch.json')throws ENOENT, failing the whole CI job.We hit this at Gusto. Our Playwright tests live under route-group folders like
apps/gusto/libs/app-*/src/pages/(layout)/. The(breaks/bin/shparsing on the Knapsack-spawned command:2. Playwright treats positional args as regex filters
Even with shell-escaping, Playwright's CLI interprets each positional argument as a regex matched against test file paths, not a literal path. Paths containing regex metacharacters (
.,(,),[,],*,+,?,^,$,{,},|,\) fail to match the intended file, and Playwright errors with:I confirmed this layer was necessary by running CI with only the shell-escape fix: batches whose paths had no regex metacharacters ran cleanly, but every batch that included a
(...)-named folder hit "No tests found".Fix
Two-layer escape on each path before joining:
'\''inner-quote escape) so the shell passes the regex-escaped string through unchanged.Organized as two commits:
fix(playwright): shell-escape test paths in onSuccessfix(playwright): regex-escape test paths for Playwright CLICompatibility
No public API change. Paths with no shell or regex metacharacters behave identically.