-
Notifications
You must be signed in to change notification settings - Fork 7
feat: dropping support for node 16 + esm compat #198
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
Conversation
| test(`should return the expected response for \`${fixture}\``, () => { | ||
| const basePath = path.join('src', 'targets', targetId, clientId, 'fixtures', `${fixture}${extname(targetId)}`); | ||
| const fixtureExtension = extname(targetId, clientId); | ||
| const basePath = path.join('src', 'targets', targetId, clientId, 'fixtures', `${fixture}${fixtureExtension}`); |
Check warning
Code scanning / CodeQL
Unsafe shell command constructed from library input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marked this as invalid because this is a unit test.
| context: . | ||
| dockerfile: integrations/python.Dockerfile | ||
| command: 'npx jest src/integration.test.ts' | ||
| command: 'npx vitest run src/integration.test.ts' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have to use the run command here because Vitest doesn't know in this Docker container that it's being run in a CI environment and it'll sit there in watch mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to rename these all to be .cjs because they the snippet that's being generated here uses require().
🧰 Changes
.jsand.cjsto every file import as well as shifting around how clients and targetsextnameis setup as our Node snippets are generating snippets withrequire()and thus need to have.cjsextensions now. Without this work all integration tests for these were failing as you can't userequire()in a.jsfile whentype: moduleis present.