Rename browser-rendering fixture and test file to browser-run#13566
Rename browser-rendering fixture and test file to browser-run#13566NuroDev merged 3 commits intocloudflare:mainfrom
Conversation
Follow-up cleanup from cloudflare#13516 per review feedback from @petebacondarwin: - fixtures/browser-rendering/ → fixtures/browser-run/ - browser-rendering.test.ts → browser-run.test.ts - Updated fixture package.json name and pnpm-lock.yaml beep-boop-🤖
|
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
|
Codeowners approval required for this PR:
Show detailed file reviewers |
|
I thought from the previous PR description that you explicitly were not going to update directory and file names. |
|
Could a maintainer add the |
workers-devprod
left a comment
There was a problem hiding this comment.
Codeowners reviews satisfied
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 CI workflow still references old fixture directory name, breaking Ubuntu test exclusion (.github/workflows/test-and-check.yml:153)
The CI workflow at .github/workflows/test-and-check.yml:153 uses --filter="!./fixtures/browser-rendering" to exclude the browser fixture tests on Ubuntu runners due to known AppArmor/Puppeteer issues (as noted in the comment on line 152). Since the fixture directory was renamed from fixtures/browser-rendering to fixtures/browser-run, this filter no longer matches anything, meaning the browser tests will now run on Ubuntu CI runners where they are expected to fail.
View 3 additional findings in Devin Review.
The Ubuntu exclusion filter still referenced the old fixtures/browser-rendering path, which would silently stop matching after the rename and cause browser tests to run on Ubuntu where they fail due to AppArmor/Puppeteer issues. beep-boop-🤖
yep focus of last PR was just what was customer facing for blog launch yesterday but make sense to clean up the rest now 👍🏾 |
Follow-up cleanup from #13516 per review feedback from @petebacondarwin.
Changes
fixtures/browser-rendering/→fixtures/browser-run/(directory + package.json name)packages/wrangler/src/__tests__/browser-rendering.test.ts→browser-run.test.tspnpm-lock.yamlupdated to match new fixture pathbeep-boop-🤖