Repository navigation
Conversation
Migrates to node:test to reduce devDependencies significantly while keeping expect for now, to not have to rewrite every test. Changes: - uses describe, test and before/after from node:test - removes jest as devDependency and its config - adds expect=^30.5.2 as direct devDependency (from: 29.7.0) - adds scripts/register-alias.mjs to help node resolve the '@' alias in jsconfig.json - rewrites mocks to use node:test.mock - fixes stray require() in ots-mini tests - fixes stray __dirname into import.meta.dirname in ots-mini tests - rewrites test.each into a plain for loop in url tests - updates sndev to support both help and custom test insertion - removes and disables payIn tests as these were fully broken prior to this commit, and warrant a separate change
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| fi | ||
|
|
||
| docker__exec -t -u apprunner app npm run test $args | ||
| docker__exec -t -u apprunner app node --import ./scripts/register-alias.mjs --experimental-test-module-mocks --test $args |
There was a problem hiding this comment.
nit (because we don't have enough tests for this to be relevant and there are similar problems in other commands in this script) but unless we append the glob something like sndev test --test-name-pattern=pollSchema doesn't work because the validation tests end in *.spec.js and node --test does not find it.
There was a problem hiding this comment.
We could just standardize towards what all frameworks accept (.test.js), also since the .spec.js is not consistently there; both ots and the disabled payIn tests are using test.js extensions. It probably would make things easier to just rename the 3 files that use spec; less friction?
I'll see what that does.
2 participants
Description
Migrates to
node:testto reduce devDependencies significantly while keeping expect for now, to not have to rewrite every test. less jesting, more testingChanges:
expect=^30.5.2as direct devDependency (from: 29.7.0)scripts/register-alias.mjsto help node resolve the '@' alias in jsconfig.jsonrequire()in ots-mini tests__dirnameintoimport.meta.dirnamein ots-mini teststest.eachinto a plain for loop in url testssndevto support both help and custom test insertionAdditional Context
This removes 102 dependencies, which helps enormously with review pressure when dealing with 3rd party vulns that can only be fixed by bumping
jest. We're keeping some of the jest packages due toexpectbeing maintained by the jestjs org since a few years, and, because this is the easiest path for now, to not change every test body.Dependencies that were bumped instead of removed with the bump of expect:
NOTE:
api/payIn/__tests__/**/*.test.jsis completely broken and requires its own PR to not grow this insanely large - it cannot be ran regardless of this PR, since a couple of months. I have disabled both the runner and the tests so that under no circumstance are we triggering these (they were explicitly ignored in./jest.config.js)Checklist
Are your changes backward compatible? Please answer below:
The arguments that
./sndev testtakes are a breaking change, as they are the nodejs arguments, not the jest cli arguments. If anyone has custom test scripts, these will likely break. I'm open to suggestions as to if anything else is needed than the current bare minimum.On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
8/10:
ci-infotriggers terminal behavior based on opaque env variables (likeexport SOME_CI_ENV_VAR=1) but is not reachable for SN code todayjest-matcher-utilshas an open behavioral bug, seejestjs/jest#16461, not reachable for SN code todayFor frontend changes: Tested on mobile, light and dark mode? Please answer below:
N/A
Did you introduce any new environment variables? If so, call them out explicitly here:
No.
Did you use AI for this? If so, how much did it assist you?
scripts/register-alias.mjswas initially bot-written because I was stuck