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

refactor: migrate to node test runner #1129

Merged
merged 12 commits into from Feb 1, 2024
Merged

refactor: migrate to node test runner #1129

merged 12 commits into from Feb 1, 2024

Conversation

barelyhuman
Copy link
Contributor

Refactors the following to use node test runner instead of Tap

  • Examples
  • Slide Dec

Note

The timeout issue still exists, as it's a problem with node v20 itself and not tap / node test runner.

Workaround for the infinite loop and getting stuck is to use Node 21, which doesn't sit well with the lts/* rule we have.

References

closes: #1128

@barelyhuman barelyhuman requested review from simoneb and luke88jones and removed request for simoneb February 1, 2024 05:13
@barelyhuman barelyhuman requested review from simoneb and removed request for luke88jones February 1, 2024 05:20
Copy link
Contributor

@luke88jones luke88jones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The switch to node test runner looks fine, just a couple of small comments.

Regarding the bug with node 20, is this a known issue? Is there an existing bug issue to track it?

src/step-06-testing/test/login.test.js Outdated Show resolved Hide resolved
src/step-11-hooks/test/unit/authenticate.test.js Outdated Show resolved Hide resolved
src/step-11-hooks/test/unit/users.test.js Show resolved Hide resolved
src/step-14-typescript/test/index.test.ts Outdated Show resolved Hide resolved
@barelyhuman
Copy link
Contributor Author

Regarding the bug with node 20, is this a known issue? Is there an existing bug issue to track it?

haven't found one, the one in references is one of the few duplicates about the same error. The workaround of using a newer node version seems to be working based on the following action

https://github.com/nearform/the-fastify-workshop/actions/runs/7736673623/job/21094398580

The original action in the repository seems to go on and on and so the workaround works but I do not have any idea about the root cause

Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good job

package.json Show resolved Hide resolved
@barelyhuman barelyhuman merged commit d09d3e2 into master Feb 1, 2024
3 checks passed
@barelyhuman barelyhuman deleted the refactor/1128 branch February 1, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to Node's test runner
3 participants