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

feat(jest-core): add support for globalSetup and globalTeardown written in ESM #11267

Merged
merged 8 commits into from
Apr 15, 2021

Conversation

aledalgrande
Copy link
Contributor

Summary

Adds support for globalSetup when using Jest in ESM mode. Potentially adds support for globalTeardown too, but will leave tests for that in a separate PR.

Works towards completing #11167.

I did not use requireOrImportModule because it seemed the flow here was a bit different from the other cases, but let me know if you want that changed. Also left formatting to Prettier/ESLint, hope that worked well.

Test plan

Tests are attached. I took inspiration from other PRs/existing code, hope it makes sense.

@aledalgrande
Copy link
Contributor Author

Not sure why the snapshot test file has been affected here.

@aledalgrande
Copy link
Contributor Author

Only Windows latest failing at the installation step now.

@aledalgrande
Copy link
Contributor Author

@SimenB do you know why the Windows / Node 15 would fail?

@SimenB
Copy link
Member

SimenB commented Apr 14, 2021

@aledalgrande sorry about the slow response here. Windows is failing due to an install issue (#11229) and can be safely ignored 🙂

@codecov-io
Copy link

Codecov Report

Merging #11267 (90ef5c2) into master (4926564) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11267      +/-   ##
==========================================
- Coverage   64.23%   64.20%   -0.03%     
==========================================
  Files         308      308              
  Lines       13512    13521       +9     
  Branches     3292     3295       +3     
==========================================
+ Hits         8679     8681       +2     
- Misses       4120     4128       +8     
+ Partials      713      712       -1     
Impacted Files Coverage Δ
packages/jest-core/src/runGlobalHook.ts 15.62% <0.00%> (-6.12%) ⬇️
packages/expect/src/utils.ts 96.12% <0.00%> (+1.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4926564...90ef5c2. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Thanks @aledalgrande, this looks good! 👍

When you update runGlobalHook.ts you also add support for globalTeardown - could you add a test for that as well?

@aledalgrande aledalgrande changed the title feat(jest-core): add support for globalSetup written in ESM feat(jest-core): add support for globalSetup and globalTeardown written in ESM Apr 15, 2021
@aledalgrande
Copy link
Contributor Author

@SimenB added a simple test, but I'm getting weird failures. Some about timing and others about logs.

@SimenB SimenB merged commit 1be8d73 into jestjs:master Apr 15, 2021
@SimenB
Copy link
Member

SimenB commented Apr 15, 2021

Thanks @aledalgrande!

@aledalgrande aledalgrande deleted the esm-global-hooks branch April 15, 2021 15:02
@gilles-yvetot
Copy link
Contributor

@SimenB this is not in @next yet right?

@SimenB
Copy link
Member

SimenB commented May 4, 2021

Correct. I can make a new release later today so people can use it 👍

@SimenB
Copy link
Member

SimenB commented May 4, 2021

@gilles-yvetot found some time now, next.9 released

@gilles-yvetot
Copy link
Contributor

@SimenB you are the real MVP

@jakobrosenberg
Copy link

I'm trying out next.9, but getting

TypeError: virtualMocks.get is not a function

  at Resolver._getVirtualMockPath (node_modules/@jest/core/node_modules/jest-resolve/build/index.js:448:25) 

The function that errors:

  _getVirtualMockPath(virtualMocks, from, moduleName) {
    console.log({virtualMocks, from, moduleName})  //my console.log
    const virtualMockPath = this.getModulePath(from, moduleName);
    return virtualMocks.get(virtualMockPath)  // line 448
      ? virtualMockPath
      : moduleName
      ? this.resolveModule(from, moduleName)
      : from;
  }

console.logging the input gives me

{
  virtualMocks: {},
  from: '<my-project>\\node_modules\\source-map-support\\source-map-support.js',   
  moduleName: 'source-map-support'
}

It seems I'm only getting the error when using jest-playwright-preset.

@SimenB
Copy link
Member

SimenB commented May 7, 2021

@jakobrosenberg huh, interesting! Could you open up a new issue with a (minimal) reproduction?

@jakobrosenberg
Copy link

@SimenB I'll see if I can get one done. In the meantime, I found the same issue here playwright-community/jest-playwright#665

Alas the proposed solution did not work for me.

@SimenB
Copy link
Member

SimenB commented May 7, 2021

I'd assume it's an issue because they pull in v26 deps: https://github.com/playwright-community/jest-playwright/blob/f8ee23108bcac6801f073986a9bba56dfe461c2f/package.json#L48-L51

Should probably be peer deps so users can plug in their own versions.

@SimenB I'll see if I can get one done. In the meantime, I found the same issue here playwright-community/jest-playwright#665

Alas the proposed solution did not work for me.

Hah, right! Makes sense it's the outdated deps. You can try resolutions or something

@github-actions
Copy link

github-actions bot commented Jun 7, 2021

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants