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

Move from Jest to uvu #1665

Merged
merged 11 commits into from
Nov 20, 2021
Merged

Move from Jest to uvu #1665

merged 11 commits into from
Nov 20, 2021

Conversation

kimoofey
Copy link
Contributor

@kimoofey kimoofey commented Nov 15, 2021

Move from Jest to uvu #1661

package.json Outdated Show resolved Hide resolved
test/browser.test.ts Outdated Show resolved Hide resolved
@ai
Copy link
Member

ai commented Nov 15, 2021

@ai
Copy link
Member

ai commented Nov 15, 2021

We need to fix:

  1. Coverage
  2. Windows support (should we add uvu issue?)
  3. Node.js 10 support, when there is not ESM. I am OK to create some simple-test.js script which will parse simple CSS if ESM support is not available (but maybe we can hack Node.js with custom loader?).

@kimoofey
Copy link
Contributor Author

  1. Do we need to increase coverage with tests or reduce this number in c8 rule in package.json (I've seen uncovered code and all of them with precommented line // istanbul ignore next)?
  2. As a alternative, to rewrite yarn unit command to uvu -r tsm test (it will works on both platforms, but need to replace version.js and integration.js, so they don't run twice on yarn test)

@ai
Copy link
Member

ai commented Nov 15, 2021

I've seen uncovered code and all of them with precommented line // istanbul ignore next

Seems like we forget to convert istanbul ignore comments to c8 ignore comments

As a alternative, to rewrite yarn unit command to uvu -r tsm test

How will it help?

We already has uvu -r tsm test in yarn unit script. As I understand, the problem is with shell script.

@kimoofey
Copy link
Contributor Author

How will it help?

As in uvu docs, we can specify only target folder (without matching pattern, that different on some platforms).

@ai
Copy link
Member

ai commented Nov 15, 2021

As in uvu docs, we can specify only target folder (without matching pattern, that different on some platforms).

I afraid it will run all JS files. It is not our case. We need to find a way to run only specific files.

Solution:

  1. Use a different command for Windows
  2. Ask uvu team for some cross-platform API

.github/workflows/test.yml Outdated Show resolved Hide resolved
@ai
Copy link
Member

ai commented Nov 17, 2021

@kimoofey sorry, you will need to update PR because I merged another PR with test changes

@ai
Copy link
Member

ai commented Nov 17, 2021

@kimoofey all other 8.3 PRs was merged. I hope we will not have more conflicts.

@kimoofey
Copy link
Contributor Author

Everything is fine, except Node 10 support
I was looking for additional loader (for example this one ), but still have not success with it

@ai
Copy link
Member

ai commented Nov 17, 2021

Everything is fine

We also have Windows issue.

except Node 10 support

I asked uvu author lukeed/uvu#162

If there is no way we can write a simple smoke test. A simple Node.js script (without any test framework) which will parse CSS and accept some basic plugin.

@ai
Copy link
Member

ai commented Nov 17, 2021

@kimoofey uvu’s author recommends uvu -r esm tests to run on Node.js 10

@ai
Copy link
Member

ai commented Nov 19, 2021

Great. Now only coverage is a blocker.

package.json Outdated
@@ -54,7 +54,8 @@
"transpiler"
],
"scripts": {
"test": "jest --coverage && eslint . && node ./test/version.js && check-dts && node ./test/integration.js && size-limit"
"test": "c8 yarn unit && eslint . && node ./test/version.js && check-dts && node ./test/integration.js && size-limit",
"unit": "ts-node -T node_modules/uvu/bin.js -r esm test '\\.test\\.(ts|js)$'"
Copy link
Contributor

@43081j 43081j Nov 20, 2021

Choose a reason for hiding this comment

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

you can probably:

uvu -r esm -r ts-node/register/transpile-only test '\\.test\\.(ts|js)$'

i'm also confused. if you're passing it through ts-node and your tsconfig has module: "commonjs", why do you need the esm package? surely the output of ts-node will be commonjs in this case, no esm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you right, ts-node will be enough here. will remove esm

@ai ai merged commit 1bbd920 into postcss:main Nov 20, 2021
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

3 participants