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/use node native test runners #14

Conversation

thiagomini
Copy link
Contributor

@thiagomini thiagomini commented Feb 14, 2024

Closes #9

Removes Jest in favor of node's native test runner. The main difference here is that we'll need to use node's internal assert library instead of Jest's "expect" API, but that's a reasonable trade-off IMHO.

PS: I'll manage the merge conflicts soon

package.json Outdated Show resolved Hide resolved
@thiagomini
Copy link
Contributor Author

Hey @oskardudycz , this is ready for review, let me know if you have any more thoughts on that!

@oskardudycz
Copy link
Collaborator

@thiagomini I fixed failing CI by:

  • updating GitHub Actions config,
  • adjusting nvm settings,
  • added pattern to also run tests in the docs folder,
  • Refactored the naming pattern to not use dash but dot (e.g. use e2e.spec.ts instead of e2e-spec.ts)

I also tried configuring tests debugging in VSCode, but it didn't work as the breakpoint was not caught. Could you check that? That's the last blocker for me atm.

- updating GitHub runner,
- adjusting nvm settings,
- added pattern to also run tests in docs folder,
- Refactored the naming pattern to not use dash but dot (e.g. use e2e.spec.ts instead of e2e-spec.ts)

Also tried to configure tests debugging in VSCode but didn't work as the breakpoint is not caught.
@oskardudycz oskardudycz force-pushed the refactor/use-node-native-test-runners branch from 2a7305e to 0293f3a Compare February 16, 2024 17:57
@thiagomini
Copy link
Contributor Author

thiagomini commented Feb 16, 2024

Hey @oskardudycz , the configuration "Debug Current File" already works for test files. You can just open the test file you want to debug, run that configuration, and Node will stop at the breakpoint. We don't need special configurations since these are interpreted by Node directly - or is it something else you'd like to add?

@oskardudycz
Copy link
Collaborator

@thiagomini in my case tests were run, but the vscode didn't stop on breakpoint. I was running that on Windows, will try tomorrow on the Linux box.

@oskardudycz
Copy link
Collaborator

@thiagomini, I checked on my Linux box, and unfortunately, it's the same result. Is it working on your side? Might be that some ts config or workspace configuration is messing with it, but I'm not sure of the reason.

@thiagomini
Copy link
Contributor Author

Hey, @oskardudycz , sorry about the late reply - yes, It's working for me when I go to a specific test file and press F5 to run "Debug Current File" configuration, as you can see in the image below. However, I noticed that running npm test in the debugger terminal isn't attaching the debugger 🤔 I'll investigate it further.

image

Copy link
Contributor Author

@thiagomini thiagomini left a comment

Choose a reason for hiding this comment

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

@oskardudycz I realized that Node version 21 was causing some issues with the debugger. We were using that mainly to get the Node's test runner glob pattern capabilities, but since I managed to fix that using the node-glob package, we can revert back to v20. Can you try out debugging again? Now it works for me from the debugger terminal and the debugger runner option

@@ -19,7 +19,7 @@ jobs:
# selected operation systems to run CI
os: [ubuntu-latest] #, windows-latest, macos-latest]
# selected node version to run CI
node-version: [20.10.x]
node-version: [20.11.1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to revert to LTS version. One of the issues with that version was that it didn't work with globstar patterns (see nodejs/node#44023). However, I found a solution by using the node-glob package, which is lightweight and constantly updated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice finding, thank you for doing the investigation 🙂

.vscode/launch.json Show resolved Hide resolved
node-glob when it's unable to resolve files is not passing anything to command.That causes all tests to be run without the pattern. Thus if you run `npm run build` and you have dist folder, script will fail.
@oskardudycz oskardudycz force-pushed the refactor/use-node-native-test-runners branch from 74e90b9 to 52c205b Compare February 19, 2024 08:59
Copy link
Collaborator

@oskardudycz oskardudycz left a comment

Choose a reason for hiding this comment

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

@thiagomini, thank you a lot for doing all this and doing the detective work!

Added placeholders for int and e2e tests

I added placeholders for int and e2e tests. It seems that node-glob, when it's unable to resolve files, is not passing anything to the command. That causes all tests to be run without the pattern. Thus, if you run npm run build and you have a dist folder, the script will fail. I'll remove those placeholders when I have the first int and e2e tests.

@oskardudycz oskardudycz merged commit 3c0b7e5 into event-driven-io:main Feb 19, 2024
1 check passed
@oskardudycz oskardudycz added enhancement New feature or request testing labels Feb 19, 2024
@oskardudycz oskardudycz added this to the 0.2.0 milestone Feb 19, 2024
@thiagomini
Copy link
Contributor Author

@oskardudycz sure, no problem, I'm happy to help! Thanks for the fast feedback and fixes as well! I'll be watching the repo and hopefully contribute further to it :)

@oskardudycz
Copy link
Collaborator

I'll be watching the repo and hopefully contribute further to it :)

Feel invited! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use Node's Native Test Runner instead of Jest
3 participants