tools: add WPT updater for specific subsystems
a WPT updater that specifically targets the url subsystem for more controlled and focused updates.
Review requested:
- [ ] @nodejs/actions
thanks for your suggestions
By the way, I'm trying to update the WPT files in #54468.
IMO this could be done via
git node wpt <...>
Because the README.md file also needs to be updated.
By the way, I'm trying to update the WPT files in #54468.
IMO this could be done via
git node wpt <...>Because the README.md file also needs to be updated.
Do you have a suggestion, would that be enough for this?
- Name: Update README.md file
Run: |
# Update the README.md file with the new WPT version
# Assuming you have a script or command to do this
./scripts/update-readme.sh ‘${{{ env.NEW_VERSION }}’
- Name: Commit to Changes
Run: |
git config --global user.name ‘Node.js GitHub Bot’
git config --global user.email ‘[email protected]’
git add test/fixtures/wpt README.md
git commit -m ‘test: Update WPT to ${{{ env.NEW_VERSION }} and update README.md’
In my opinion, the entire WPT update process could be moved to a dep-updater script, using git node for each section. But, there are a lot of cases where updating these tests cause problems (just look at the errors in my PR), so maybe keeping it manual is good?
I'm not sure.
cc @nodejs/web-standards
yeah, we had discussion at https://github.com/nodejs/node/pull/50567.
I'm closing this place, thank you very much for the comments
@panva What do you think about adding a WPT updater but for specific tests? Ada repository has a WPT updater for url and it works perfectly. I'd be interested if we have it for url, and I'm more than happy to intervene and fix the pull-request in case something breaks.
@anonrig That's pretty much a version of what I think we should be doing, only scoped to a particular WPT suite.
In my opinion a sensible way to update WPTs would be to pull a WPT subsystem from upstream (using git node wpt ...), run it, if it passes - open a PR, if it fails - have a keepalive issue where pending WPT updates are tracked.
Perfect @mertcanaltin seems interested in pursuing this. Mertcan can you add a WPT updater but only for url? Make sure to have a list for the scopes, so we can add them depending on people's interest.
This looks great, I'm gonna do it
Will you test this in https://github.com/nodejs/node-auto-test?
https://github.com/mertcanaltin/node-auto-test/actions/runs/10972255885/job/30468306292 I did a test run here and it seems to give an error to the github token
mertcanaltin/node-auto-test/actions/runs/10972255885/job/30468306292 I did a test run here and it seems to give an error to the github token
I think it seems to suggest the script is ending prematurely. i.e. not syntactically correct.
Passed 🎉 https://github.com/mertcanaltin/node-auto-test/actions/runs/10972357345/job/30468531837
The issue was that the if block wasn’t properly closed. I just added the missing fi at the end, and now everything works smoothly.
Passed 🎉 mertcanaltin/node-auto-test/actions/runs/10972357345/job/30468531837
The issue was that the if block wasn’t properly closed. I just added the missing fi at the end, and now everything works smoothly.
Can you run dispatch with two subsystems? url and WebCryptoAPI. The latter should open a PR.
https://github.com/mertcanaltin/node-auto-test/actions/runs/10972538059 I ran for both of them
https://github.com/mertcanaltin/node-auto-test/actions/runs/10972538059/job/30468925665#step:4:7
It would appear @node-core/utils needs its config file with credentials to run this. Maybe an empty config file would do? I dunno, I'm short on time to look into this.
You could also move all this logic into a dependency updater script, and call it like the rest of the updaters.
You could also move all this logic into a dependency updater script, and call it like the rest of the updaters.
ı applied the latest updates, thank you
You could also move all this logic into a dependency updater script, and call it like the rest of the updaters.
node/.github/workflows/update-openssl.yml
I looked here as an example
Look at .github/workflows/auto-start-ci.yml for Setup @node-core/utils
I believe the last 2 points weren't addressed. ncu is not configured and the new version variable is used before it is set
I believe the last 2 points weren't addressed. ncu is not configured and the new version variable is used before it is set
Thanks a lot for your detailed feedback! I’ve now addressed the last two points you mentioned:
I've added a step to configure ncu (node-core-utils) with the necessary GitHub token. I moved the calculation of the new_version earlier in the workflow to ensure it's set before being referenced.
@nodejs/actions please review
~~FYI I was looking at PRs and I saw #50567 cc @legendecas~~ I'm really late to the party, sorry
https://github.com/nodejs/node/pull/54460#issuecomment-2314933326
Landed in 4f62ab5af8e98c73dcb56399fa913bf52e0c0c01
🎉
https://github.com/nodejs/node/actions/runs/12031931489 https://github.com/nodejs/node/pull/55999