Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refreshing Node.js supported versions #786

Closed
wants to merge 3 commits into from

Conversation

aaronpowell
Copy link
Contributor

Updating the supported node versions to match the current list, including previews.

Fixes #756

@aaronpowell
Copy link
Contributor Author

Oh right, tests probably need updating too 😅

@aaronpowell
Copy link
Contributor Author

I'm not sure how to fix the failing test. The test that fails is this one here and it fails because it "thinks" we have v3 of the core tools installed but we need v4 (since v3 didn't support Node18 but now v3 does).

The problem is that we now have some overlaps in the version support (v3 and v4 both support Node 18 and Node 20), resulting in it not being able to tell what to install.

Probably need an overhaul of that test but I'm not sure what it should look like.

@adrianhall
Copy link
Member

Hold off on merging this one for right now

Hey @aaronpowell - I'll get this merged once we have the big CVE/ESM move done.

@Timothyw0 Timothyw0 self-requested a review as a code owner July 22, 2024 15:42
@github-actions github-actions bot added the scope: core Issues happened a the ./src/core level label Jul 22, 2024
@Timothyw0
Copy link
Member

@aaronpowell Could you please change the cached version here that will fix the unit test? In src/core/func-core-tools.spec.ts line 227:
fct.setCachedInstalledSystemCoreToolsVersion(2);

This will force the binary to be out of sync with the version of Node and return the expected downloaded binary

@Timothyw0
Copy link
Member

Timothyw0 commented Jul 22, 2024

@aaronpowell Based on the timeline of our upcoming release, I've created a PR including your changes and the unit test fix at #869

So I will close this PR after the merge. Thank you for your help on this! :)

@Timothyw0 Timothyw0 closed this Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: core Issues happened a the ./src/core level
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node 20 support
3 participants