fix(build): parse Playwright 1.58+ dry-run output in playwright extension#4300
fix(build): parse Playwright 1.58+ dry-run output in playwright extension#4300ANSHSINGH050404 wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 0663c5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @ANSHSINGH050404, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughUpdated the Playwright build extension to parse both legacy and newer ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| `RUN grep -A5 -m1 "browser: ${browser}" /tmp/browser-info.txt > /tmp/${browser}-info.txt`, | ||
| // Playwright ≤1.57: "browser: chromium version ..." | ||
| // Playwright ≥1.58: "Chrome for Testing ... (playwright chromium v...)" | ||
| `RUN grep -A5 -m1 -E "browser: ${browser}|playwright ${browser}" /tmp/browser-info.txt > /tmp/${browser}-info.txt`, |
There was a problem hiding this comment.
🔍 grep pattern playwright chromium can substring-match the headless-shell line
The new extended-regex browser: ${browser}|playwright ${browser} at packages/build/src/extensions/playwright.ts:322 is unanchored, so for browser === "chromium" the alternative playwright chromium is a substring of playwright chromium-headless-shell. In non-headless mode both chromium and chromium-headless-shell are installed, so if Playwright 1.58+ lists the headless-shell entry before the chromium entry in --dry-run output, the -m1 (first match) for the chromium iteration would grab the wrong entry, producing the wrong install directory/download URL. This substring ambiguity is order-dependent and also existed in the old plain-grep pattern (browser: chromium matched browser: chromium-headless-shell), so it is not newly introduced, but the fix does not resolve it. Worth verifying the actual line ordering of Playwright 1.58+ dry-run output.
Was this helpful? React with 👍 or 👎 to provide feedback.
| `RUN INSTALL_DIR=$(grep "Install location:" /tmp/${browser}-info.txt | cut -d':' -f2- | xargs) && \ | ||
| DIR_NAME=$(basename "$INSTALL_DIR") && \ |
There was a problem hiding this comment.
🔍 Fix assumes Install location / Download url lines are unchanged in 1.58+
Only the first grep was changed; the downstream parsing at packages/build/src/extensions/playwright.ts:324-338 still relies on Install location: and Download url: lines appearing within the 5 lines following the matched header (-A5). This assumes Playwright 1.58+ retained those exact labels and their relative position under the new (playwright chromium v...) header line. If 1.58+ reformatted or relocated those lines, the build would fail at the explicit Failed to extract... guards. The PR description implies the grep is the only needed change, but this coupling is worth confirming against real 1.58+ output.
(Refers to lines 324-338)
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Closes #3089
Checklist
Summary
Playwright 1.58 changed
--dry-runlines frombrowser: chromiumto(playwright chromium v…).Fix
Match either format in the Docker install grep.
Vouch request: #4290