Skip to content

[vitest-pool-workers] Fix Workflow binding's script_name when it matches its own Worker name#10033

Merged
pombosilva merged 3 commits intomainfrom
fix/workflows-vitest-pool-workers-script-name
Jul 24, 2025
Merged

[vitest-pool-workers] Fix Workflow binding's script_name when it matches its own Worker name#10033
pombosilva merged 3 commits intomainfrom
fix/workflows-vitest-pool-workers-script-name

Conversation

@pombosilva
Copy link
Copy Markdown
Contributor

@pombosilva pombosilva commented Jul 21, 2025 •
edited
Loading

Fixes WOR-760.

A user can define a Workflow binding where the script_name is the same as the name of the Worker on which the binding is defined. Although redundant, it should be supported.

vitest-pool-workers breaks because it does not use the user's Worker as the test entrypoint. It uses its own runner worker, which has a different name generated before execution. This leads to a mismatch between the script_name in the binding and the Worker that will run.

To fix this, the code now checks it a workflow binding's script_name matches its own Worker's name. When it does, script_name is removed, making it default to the current worker.

  • Tests
    • Tests included
    • Tests not necessary because: bug not reproduced in fixtures for some unknown reason
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: only makes vitest-pool-workers not break in a specific case for Workflows
  • Wrangler V3 Backport
    • Wrangler PR:
    • Not necessary because: does not affect wrangler

@pombosilva pombosilva requested a review from a team as a code owner July 21, 2025 17:47
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jul 21, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: dbf9e1d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudflare/vitest-pool-workers Patch

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

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Jul 21, 2025
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Jul 21, 2025 •
edited
Loading

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@10033

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@10033

miniflare

npm i https://pkg.pr.new/miniflare@10033

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@10033

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@10033

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@10033

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@10033

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@10033

wrangler

npm i https://pkg.pr.new/wrangler@10033

commit: dbf9e1d

@pombosilva pombosilva requested a review from a team as a code owner July 22, 2025 08:45
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jul 22, 2025

Failed to automatically backport this PR's changes to Wrangler v3. Please manually create a PR targeting the v3-maintenance branch with your changes. Thank you for helping us keep Wrangler v3 supported!

Depending on your changes, running git rebase --onto v3-maintenance main fix/workflows-vitest-pool-workers-script-name might be a good starting point.

Notes:

  • your PR branch should be named v3-backport-10033
  • add the skip-v3-pr label to the current PR to stop this workflow from failing

@pombosilva pombosilva force-pushed the fix/workflows-vitest-pool-workers-script-name branch 2 times, most recently from e7a36a3 to f4d51dc Compare July 22, 2025 14:56
LuisDuarte1
LuisDuarte1 reviewed Jul 22, 2025
View reviewed changes
LuisDuarte1
LuisDuarte1 reviewed Jul 22, 2025
View reviewed changes
@pombosilva pombosilva force-pushed the fix/workflows-vitest-pool-workers-script-name branch from 7b1a2d8 to ae0a9c5 Compare July 22, 2025 16:57
LuisDuarte1
LuisDuarte1 approved these changes Jul 23, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@LuisDuarte1 LuisDuarte1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved from the workflows team.

petebacondarwin
petebacondarwin approved these changes Jul 23, 2025
View reviewed changes
@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jul 23, 2025
@emily-shen
Copy link
Copy Markdown
Contributor

emily-shen commented Jul 23, 2025

I think the name is available under project.project.name so you won't have to re-read the wrangler config

@emily-shen
Copy link
Copy Markdown
Contributor

emily-shen commented Jul 23, 2025

but if Pete's suggestion to strip out the name works, that'd be great as its much simpler

@petebacondarwin
Copy link
Copy Markdown
Contributor

petebacondarwin commented Jul 23, 2025

but if Pete's suggestion to strip out the name works, that'd be great as its much simpler

I think we'd still need to know the Worker name in case it is actually pointing to a different Worker not itself?

@emily-shen
Copy link
Copy Markdown
Contributor

emily-shen commented Jul 24, 2025

but if Pete's suggestion to strip out the name works, that'd be great as its much simpler

I think we'd still need to know the Worker name in case it is actually pointing to a different Worker not itself?

oh sorry fair enough - i guess check if it matches project.project.name, and strip it out if it does?

@pombosilva
Copy link
Copy Markdown
Contributor Author

pombosilva commented Jul 24, 2025

but if Pete's suggestion to strip out the name works, that'd be great as its much simpler

I think we'd still need to know the Worker name in case it is actually pointing to a different Worker not itself?

oh sorry fair enough - i guess check if it matches project.project.name, and strip it out if it does?

Yes, I'll do it, however I'm not able to get the worker name through project.project.name. It actually returns nothing

@pombosilva
fix: make vitest-pool-workers not break if Workflow binding's script_…
f50ed8f 
…name matches its own Worker name

removing try catch since error gets thrown either way

use userConfigPath instead of redirected one (#10005)

Revert "use userConfigPath instead of redirected one (#10005)"

This reverts commit 5c11fe7504a0b16a963c726cc63748426fd8bab6.

fix deep import

prettier run

fix: make vitest-pool-workers not break if Workflow binding's script_name matches its own Worker name
@pombosilva
apply suggestions from code review
8a3fdc8
@pombosilva
code review suggestion: delete script_name instead of replacing it
dbf9e1d
@pombosilva pombosilva force-pushed the fix/workflows-vitest-pool-workers-script-name branch from 3cdd607 to dbf9e1d Compare July 24, 2025 14:08
@pombosilva pombosilva merged commit f467415 into main Jul 24, 2025
36 of 37 checks passed
@pombosilva pombosilva deleted the fix/workflows-vitest-pool-workers-script-name branch July 24, 2025 14:34
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jul 24, 2025
@workers-devprod workers-devprod mentioned this pull request Jul 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petebacondarwin petebacondarwin petebacondarwin approved these changes

@LuisDuarte1 LuisDuarte1 LuisDuarte1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

workers-sdk
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pombosilva @emily-shen @petebacondarwin @LuisDuarte1 @workers-devprod