Go back to using explicit node verisons in CI #51965
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
Well, that's disconcerting; our tests fail on Node 19. I guess it's good we didn't get upgraded...? |
|
Okay, so, Node 19 (probably v8) changed the formatting of Dates; instead of just |
You probably already found this issue but linking it in case you did not spot it, nodejs/node#45171 |
|
Yikes, I guess it's an icu change, not even a node change. This is just a testing bug, so I'll just change the helper to fix the character. This sort of thing could happen to any version of Node, I'm guessing. Cross linking: |
|
|
There's no major need to do this; all of the tasks which don't pin a version of Node don't care about the particular version other than that it's new enough. |
| - "19" | ||
| - "18" | ||
| - "16" | ||
| - "14" | ||
| bundle: | ||
| - "true" | ||
| include: |
There was a problem hiding this comment.
I think we can take out L27 here?
There was a problem hiding this comment.
No node-version means "use whatever random version is installed on the runner on $PATH", and is used if you want to set up other Node stuff like proxies or auth, but keep the version active on the runner. I don't think that's what we want.
There was a problem hiding this comment.
L27 re-introduces the duplication problem from the original issue. There are now Node * and Node 18 jobs, which are duplicates.
There was a problem hiding this comment.
This is an entirely different build with --bundle=false. It's not a duplicate.
There was a problem hiding this comment.
Ah, I missed that, never mind! Thanks for jumping on my bug report so quickly.
Fixes #51943
We were using "*" and "lts" assuming that one would be the most recent node release, and the other be LTS. This turns out to be wrong because "*" means "whatever is newest in GHA's cache", not the actual latest.
We could switch to say "current" or "latest", which would ensure we always get the actual current Node version. However, the current setup has another problem; it assumes that there are only two LTS versions out at one time. This isn't accurate; right now, Node 14, 16, and 18 are all LTS, and we are now suddenly not testing Node 14, which is scary.
I don't see a way to solve this, because at some points of the year, there will be two LTS releases, and sometimes, there will be three.
Just go back to explicitly listing the versions we test with. This will make us definitely run on all of our intended targets, but also mean old release branches won't suddenly test on the wrong versions, which is good.
We'll have to remember to update these, but I am personally confident that is fine. I also imagine that we don't want to drop Node 14 anytime soon, because it's the last version with stack restarting in debugging, a feature that many on the team use (including me).