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

test: migrate jest -> node:test #6129

Closed
wants to merge 3 commits into from

Conversation

AugustinMauroy
Copy link
Contributor

Description

Migrate jest to nodejs internal test runer

Validation

all test should pass

Related Issues

#6123

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npx turbo lint to ensure the code follows the style guide. And run npx turbo lint:fix to fix the style errors if necessary.
  • I have run npx turbo format to ensure the code follows the style guide.
  • I have run npx turbo test to check if all tests are passing.
  • NA I've covered new added functionality with unit tests if necessary.

@AugustinMauroy AugustinMauroy requested a review from a team as a code owner November 17, 2023 20:17
Copy link

vercel bot commented Nov 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 25, 2023 10:22pm

@AugustinMauroy
Copy link
Contributor Author

AugustinMauroy commented Nov 17, 2023

hummmm something strange I din't get any error for other test (not migrated yet).
I mean node didn't read-it
but on junit.xml it's output error for these tests 🤔

@ovflowd
Copy link
Member

ovflowd commented Nov 17, 2023

hummmm something strange I din't get any error for other test (not migrated yet).

I mean node didn't read-it

but on junit.xml it's output error for these tests 🤔

Wdym? Is it not generating junit.xml when passing the right args?

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@@ -91,10 +92,6 @@
"@storybook/addon-themes": "^7.5.3",
"@storybook/addon-viewport": "~7.5.3",
"@storybook/nextjs": "~7.5.3",
"@testing-library/jest-dom": "~6.1.4",
"@testing-library/react": "~14.0.0",
"@testing-library/user-event": "~14.5.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why removed testing-library/user-event, are we not using them?

Afaik testing-library is used for react testing... and we need it to create the jsdom environment also...

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 have remove everything to start and re-add with we will need

Copy link
Member

Choose a reason for hiding this comment

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

Not sure that's a good idea, but hey you do you.

@AugustinMauroy
Copy link
Contributor Author

Wdym? Is it not generating junit.xml when passing the right args?

on terminal it's doesn't output any error but on junit.xml it's write error. Alors if I remove **/__tests__/*.test.mjs there any issue it's display correctly

@AugustinMauroy
Copy link
Contributor Author

AugustinMauroy commented Nov 18, 2023

huston we go a problem. TSX loader doesn't support jsx REF
I'm deep diving @educandu/node-jsx-loader a jsx loader

@ovflowd
Copy link
Member

ovflowd commented Nov 19, 2023

huston we go a problem. TSX loader doesn't support jsx REF I'm deep diving @educandu/node-jsx-loader a jsx loader

I'm deep diving @educandu/node-jsx-loader a jsx loader

Afaik it does... Why do you think it doesn't? It should support React natively :)

I even have seen users using it with React in the past (I also used it with React in the past, so I wonder what errors you're hitting?)

@AugustinMauroy
Copy link
Contributor Author

@ovflowd I think there's been a misunderstanding. tsx and tsc are loaders that allow you to interpret typescript. But neither of them understands JSX. That's why I looked at @educandu/node-jsx-loader. It supports JSX but not TSX. So I'm still looking for what/how. Also if a component imports style with CSS module node will try to load it.

@ovflowd
Copy link
Member

ovflowd commented Nov 20, 2023

@ovflowd I think there's been a misunderstanding. tsx and tsc are loaders that allow you to interpret typescript. But neither of them understands JSX. That's why I looked at @educandu/node-jsx-loader. It supports JSX but not TSX. So I'm still looking for what/how. Also if a component imports style with CSS module node will try to load it.

Again, you're wrong. tsx is a loader that supports JSX. tsc/esm is a loader that does not support JSX. tsx uses esbuild and supports jsx. You can even see that the codebase supports those extensions: https://github.com/search?q=repo%3Aprivatenumber%2Ftsx%20jsx&type=code

@AugustinMauroy
Copy link
Contributor Author

gotcha !!!

@ovflowd
Copy link
Member

ovflowd commented Nov 20, 2023

gotcha !!!

If you're still struggling, please feel free to share errors. We're here to help.

@ovflowd
Copy link
Member

ovflowd commented Nov 25, 2023

Any progress over here? 👀

@AugustinMauroy
Copy link
Contributor Author

I'ill come back in now I have time 😀

@AugustinMauroy
Copy link
Contributor Author

I'm getting trouble with react test.
with module css nodejs can't load file

@ovflowd
Copy link
Member

ovflowd commented Nov 25, 2023

I'm getting trouble with react test.

with module css nodejs can't load file

Can you share the errors?

@ovflowd
Copy link
Member

ovflowd commented Nov 25, 2023

I think we truly dont care about css imports; So probably we should stub/mock these imports... I know esbuild supports css and probably css modules by default, but I doubt it is something we really want to import.

@ovflowd
Copy link
Member

ovflowd commented Nov 26, 2023

@nodejs/loaders does anyone here know how we can stub/mock part of the module resolution process?

More specifically some of our files import .css files but we definitely dont want to load them with the Node.js Test Runner (node:test) and we're using "--import=tsx" that adds a loader for TypeScript and React (JSX).

Should we make another loader just to stub these CSS imports or is there something else we can do here? Like a builtin functionality to simply stub these imports?

@ovflowd
Copy link
Member

ovflowd commented Nov 26, 2023

I think I found a solution for my own question. https://github.com/coderaiser/mock-import

👆 this might help you, Augustin.

@ovflowd
Copy link
Member

ovflowd commented Nov 26, 2023

(Im also assuming https://www.npmjs.com/package/sinon can do the trick) as the state of node:test mocking and spying is kinda supbar atm (the whole thing is still experimental. We could definitely raise a feature request on node core

cc @nodejs/test_runner)

@JakobJingleheimer
Copy link
Contributor

I use Testdouble to do this in a pure ESM environment.

@cclauss
Copy link
Contributor

cclauss commented Jan 23, 2024

Lots of git conflicts are listed below...

@AugustinMauroy
Copy link
Contributor Author

Closing because it's too old. And I can't rebase GitHub desktop just crash.
an new branch is WIP https://github.com/nodejs/nodejs.org/tree/test-node if someone with write access can help. But let me know.

@AugustinMauroy AugustinMauroy deleted the test(node) branch April 4, 2024 10:27
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.

None yet

4 participants