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

jest: support custom testEnvironment #834

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

tryggvigy
Copy link
Contributor

This PR

Adds support for custom testEnvironment files like:

  testEnvironment:
    '<rootDir>/tests/framework/e2e/jest-environments/E2EEnvironment.ts',

I get a unit test failure on the cli version when running the tests locally. Maybe it'll pass in CI. Anyone know how to fix that?
image

Copy link

pkg-pr-new bot commented Nov 13, 2024

Open in Stackblitz

bun add https://pkg.pr.new/knip@834

commit: b80e392

@tryggvigy tryggvigy force-pushed the harden-jest-test-environment branch from 021d2c5 to 839f679 Compare November 13, 2024 16:11
@webpro
Copy link
Collaborator

webpro commented Nov 13, 2024

I get a unit test failure on the cli version when running the tests locally. Maybe it'll pass in CI. Anyone know how to fix that?

I think it's because of a previous build, you can run bun build and it should be alright (the cli tests use the built dist version).

Copy link
Collaborator

@webpro webpro left a comment

Choose a reason for hiding this comment

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

Nice, just one nitpick but happy to merge!

environments = ['jest-environment-jsdom'];
} else if (config.testEnvironment) {
environments = [config.testEnvironment];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could make it a const in one line? That's become a bit of a standard whenever possible, for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would require a nested ternary but I can do that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in b80e392

@webpro webpro merged commit 153a836 into webpro-nl:main Nov 13, 2024
23 checks passed
@webpro
Copy link
Collaborator

webpro commented Nov 13, 2024

Thanks @tryggvigy! Nested ternaries aren't marvellous, but heck.

@tryggvigy tryggvigy deleted the harden-jest-test-environment branch November 13, 2024 20:17
@webpro
Copy link
Collaborator

webpro commented Nov 14, 2024

🚀 This pull request is included in v5.37.0. See Release 5.37.0 for release notes.

Using Knip in a commercial project? Please consider becoming a sponsor.

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.

2 participants