Attempt to migrate to Node20

[mikaelarguedas]

Workflows complains that action-ros-ci is on EOL node 16: https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/

Depends on ros-tooling/setup-ros#679

Closes #861
Closes #856
Closes #854
Closes #853
Closes #850

[christophebedard]

It feels like I bumped to Node.js 16 just a few months ago, but it was over a year ago 😆 #778

[christophebedard]

Thanks for the PR! This looks great to me! Could you just update the version mentioned here to v20?

action-ros-ci/DEVELOPING.md

Line 7 in 6b8f219

We are currently using Node.js v16.

You'll also need to update to node-version: "20.x" under actions/setup-node everywhere.

[codecov]

Codecov Report

Attention: Patch coverage is 62.50000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 27.39%. Comparing base (8cf3d2d) to head (3bf3c7e).

❗ Current head 3bf3c7e differs from pull request most recent head 2490c3c. Consider uploading reports for the commit 2490c3c to get more accurate results

Files	Patch %	Lines
src/action-ros-ci.ts	50.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master     #870       +/-   ##
===========================================
- Coverage   44.55%   27.39%   -17.17%     
===========================================
  Files           2        2               
  Lines         303      303               
  Branches       81       81               
===========================================
- Hits          135       83       -52     
- Misses        168      220       +52     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mikaelarguedas]

will likely need to wait for ros-tooling/setup-ros#679 to then rerun CI as 9b03fde made all the jobs fail

[christophebedard]

It seems like this part of the tests might make CI fail now: https://github.com/ros-tooling/action-ros-ci/actions/runs/9050675407/job/24866327260?pr=870#step:5:86

[mikaelarguedas]

Yeah weird, this comes from @actions/exec that is the only @action lib that was already at the latest version 🤷

[christophebedard]

I found this issue: actions/toolkit#1625

but I just removed the test and it still fails, so it must be something else 🤔

[christophebedard]

I'm able to run tests locally by defining some GITHUB_* env vars:

$ GITHUB_REPOSITORY="owner/repo" GITHUB_WORKSPACE="/ws" npm test

I started commenting out a bunch of stuff here and there and found out two things:

1. Having the run(); line in action-ros-ci.ts and importing it in the test file executes that line, which results in the full action being run. We definitely did not intend to run the full action as part of these unit tests. Looking at setup-ros, its run() function is called in setup-ros.ts, which is never imported in its tests: https://github.com/ros-tooling/setup-ros/blob/8147828f7d9cb47d5807984e89f05c268c0cf0fb/src/setup-ros.ts#L28. Therefore, I moved the run(); line to a separate file that we don't import in the test file.
2. Even with the above fix, the issue persists. What really makes the test command return with a non-zero return code is the call to core.setFailed(), which sets the process' return code to 1 ❗ https://github.com/actions/toolkit/blob/main/packages/core/src/core.ts#L221. Therefore, I just reset the process' exit code to 0 in the test. I tried mocking core.setFailed() instead, but couldn't get it to work easily, and I don't have much more time for this for now.

[christophebedard]

by defining some GITHUB_* env vars:

This isn't necessary with the changes related to item 1 above.

[mikaelarguedas]

Nice thanks @christophebedard for picking this up!

Looks like now CI failures are the same as on master ? or is there more work to do on this one to get it over the finish line ?

[christophebedard]

No, I think this is good. I'll merge this and then create new releases of both setup-ros and action-ros-ci.

[christophebedard]

I released this as 0.3.13/v0.3.