search

actions / toolkit

The GitHub ToolKit for developing GitHub Actions.
https://github.com/features/actions
MIT License
5.02k stars 1.45k forks source link

Extend Node version test coverage #1843

Closed joshmgross closed 1 week ago

joshmgross commented 1 month ago

1841 could have been caught in unit tests.

As long as we have users of the toolkit with deprecated actions (Node 16) and running with the default Node version on hosted runners (Node 18), we should avoid breaking the toolkit by utilizing Node 20 features.

In the future, we may want to be more clear about what versions of Node we support in the toolkit.

joshmgross commented 1 month ago

This is failing as expected due to #1841, so I've confirmed that this would catch the reference error 🎉

ReferenceError: crypto is not defined https://github.com/actions/toolkit/actions/runs/11186675222/job/31102183371?pr=1843

CI should be passing once #1842 is merged.

joshmgross commented 1 month ago

Removed Node 16 as the Artifact tests were not passing

FAIL packages/github/__tests__/github.proxy.test.ts
  ● @actions/github › should only use default agent if one is not provided

    fetch is not set. Please pass a fetch implementation as new Octokit({ request: { fetch }}). Learn more at https://github.com/octokit/octokit.js/#fetch-missing

      at fetchWrapper (packages/github/node_modules/@octokit/request/dist-node/index.js:57:11)
      at request2 (packages/github/node_modules/@octokit/request/dist-node/index.js:180:14)
      at hook (packages/github/node_modules/@octokit/auth-token/dist-node/index.js:58:10)
      at packages/github/node_modules/before-after-hook/lib/register.js:25:15

That's not a difficult thing to fix, but the purpose of this PR is not to extend which versions of Node we support, only to recognize which versions users already depend on.