-
Notifications
You must be signed in to change notification settings - Fork 75
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
Support different jest environments #116
Conversation
Pull Request Test Coverage Report for Build 91500839
💛 - Coveralls |
I just made some rewriting here. And I wanna make jest-playwright-jsdom env to support browser as separate package, like jest-environment-jsdom and jest-environment-node |
src/PlaywrightEnvironment.ts
Outdated
if (page) { | ||
page.removeListener('pageerror', handleError) | ||
} | ||
if (context) { |
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.
maybe we can simplify the whole function in something like that. Its enough to only call browser.close().
const wait = []
if (browser) {
wait.push(browser.close())
}
if (!jestConfig.watch && !jestConfig.watchAll && teardownServer) {
wait.push(teardownServer())
}
await Promise.all(...wait)
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.
big diff, but seems ok if most of it is just indention changes.
It's only separating types from constants =) |
Some thoughts on #115
After it we can use it with custom testEnvironment:
Yeah! That's not perfect. I just thinking about using lerna for next releases, cause I suppose we'll need also rewrite jest-dev-server #104