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

[Question] global expect overrides jest expect — is that okay? #19798

Closed
Diokuz opened this issue Dec 30, 2022 · 6 comments
Closed

[Question] global expect overrides jest expect — is that okay? #19798

Diokuz opened this issue Dec 30, 2022 · 6 comments

Comments

@Diokuz
Copy link

Diokuz commented Dec 30, 2022

We spend a day on this)

So, we have some jest + puppeteer tests. We want to migrate to playwright-test, but, after some difficulties, as a first step we decided to replace just puppeteer with playwright. So, our first target – jest + playwright.

We replaced puppeteer with playwright and got this test failing in CI:

import { initPlaywright } from '@my-local-namespace/playwright-utils'

it.only('server HTML snapshot', async () => {
  ....

  expect(application).toMatchSnapshot();
});

it.skip('skip', async () => {
  console.log('initPlaywright', initPlaywright)
});
 FAIL  examples/assets/__integration__/application.test.ts (6.272 s)
  ● assets › server HTML snapshot

    toMatchSnapshot() must be called during the test



      at Object.toMatchSnapshot (node_modules/@playwright/test/lib/matchers/toMatchSnapshot.js:229:24)
      at __EXTERNAL_MATCHER_TRAP__ (node_modules/@jest/expect/node_modules/expect/build/index.js:349:30)
      at Object.throwingMatcher (node_modules/@jest/expect/node_modules/expect/build/index.js:350:15)
      at Object.<anonymous> (examples/assets/__integration__/application.test.ts:58:29)
      at asyncGeneratorStep (examples/assets/__integration__/application.test.ts:10:28)
      at _next (examples/assets/__integration__/application.test.ts:28:17)

WAT? @playwright/test matchers? :)

Test failed in CI, but not locally. We spent few hours to reproduce that locally. The trick was – during local debug we just removed the second test instead of marking it as .only. Because of that, typescript just removed unused '@my-local-namespace/playwright-utils' import, which contains imports from @playwright/test under the hood. So, in CI there were transitive imports from @playwright/test, and there were no imports from @playwright/test locally.

We found out that any import from @playwright/test replaces

  1. global.expect
  2. import {expect} from '@jest/globals';

Solution: use expect from

import { jestExpect as expect } from '@jest/expect'

The question: what do you think about not monkey patching global object when playwright test was not launched explicitly?

@aslushnikov
Copy link
Collaborator

aslushnikov commented Dec 30, 2022

@Diokuz first of all, I'm sorry you spent a day debugging this nasty behavior!

We found out that any import from @playwright/test replaces global.expect

So my naive attempt to reproduce this failed:

// a.mjs
import { chromium } from '@playwright/test';
console.log(global.expect);

This prints undefined to me - which is expected behavior. Could you please help me with a similar short repro to demonstrate the issue?

@pavelfeldman
Copy link
Member

pavelfeldman commented Dec 31, 2022

Unrelated: A word of advice, don't do jest + playwright - we've seen people misusing it and ending up with memory leaks, inefficient setups, poor parallelization.

Should you choose to use jest though, you can't use @playwright/test, you should import playwright and use it as a library.

@Diokuz
Copy link
Author

Diokuz commented Jan 4, 2023

Thanks for response)

@aslushnikov

Ill try to go deeper in a week, but what I can say right now is that removing import solved my problem:

fails:

export { test } from '@playwright/test'

works well:

// export { test } from '@playwright/test'

@pavelfeldman I agree with you for 100%, that was just a temporal solution during migration from jest to Playwright)

@Diokuz
Copy link
Author

Diokuz commented Jan 4, 2023

I found it! https://github.com/microsoft/playwright/blob/main/packages/playwright-test/src/expect.ts#L290

expect does not changed, but expect().toMatchSnapshot does.

@aslushnikov
Copy link
Collaborator

@Diokuz Ah! So the workaround would be to avoid importing test. Seems to be good-enough to me! :)

@Filipoliko
Copy link

Hi, we just ran into the same issue and spent some good time debugging it, before we noticed that playwright is actually overriding the jest expect method.

We have an npm package, which contains all our testing utilities that we share across our teams. This includes some utilities for jest unit/integration tests and playwright e2e tests. We just added shared playwright fixtures and we are importing test from @plawright/test there. Our npm package is exporting all test utilities from the central main.js file, so it is accessible under import { myUtil } from 'my-package. This leads to the very same error mentioned above.

We were also able to workaround it by adding exports field to package.json file and we defined /e2e as a separate entry point. In the end it actually seems as a better solution. The only problems are eslint not working well with exports field and we had to use different export/import pattern than any other package we have, which is a little bit unfortunate.

It would be nice if playwright didn't affect other packages just by importing it.

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

No branches or pull requests

4 participants