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

browser field is not respected in concurrent mode w/ multi-project config #6887

Closed
lhorie opened this issue Aug 24, 2018 · 4 comments
Closed

Comments

@lhorie
Copy link

lhorie commented Aug 24, 2018

🐛 Bug Report

Jest does not honor browser fields field in concurrent tests correctly if nested in projects field.

This makes it very difficult to run node and jsdom tests concurrently.

To Reproduce

git clone https://github.com/lhorie/jest-bug
yarn
yarn test

Expected behavior

This repo configures jest with two projects (one has browser: true and tests *.browser.js files and the other has browser:false and tests *.node.js files.

// jest config
  {
    "projects": [
      {
        "displayName": "browser",
        "browser": true,
        "testEnvironment": "jsdom",
        "testPathIgnorePatterns": [
          ".*\\.node\\.js"
        ]
      },
      {
        "displayName": "node",
        "browser": false,
        "testEnvironment": "node",
        "testPathIgnorePatterns": [
          ".*\\.browser\\.js"
        ]
      }
    ]
  }

There are 22 tests (because more than 20 tests are required to trigger concurrent mode).

11 tests (named a.browser.js through k.browser.js) run in the browser project and look like this:

//browser tests
const isBrowser = require('is-browser');

test('browser', () => {
  expect(isBrowser).toEqual(true);
});

11 tests (named a.node.js through k.node.js) run in node project and look like this:

//node tests
const isBrowser = require('is-browser');

test('node', () => {
  expect(isBrowser).toEqual(false);
});

All tests should pass. Half of them fail. See repo for output.

Link to repl or repo (highly encouraged)

https://github.com/lhorie/jest-bug

Run npx envinfo --preset jest

Paste the results here:

npx envinfo --preset jest
npx: installed 1 in 2.844s

  System:
    OS: macOS Sierra 10.12.6
    CPU: x64 Intel(R) Core(TM) i7-4980HQ CPU @ 2.80GHz
  Binaries:
    Node: 8.11.3 - /usr/local/bin/node
    Yarn: 1.7.0 - /usr/local/bin/yarn
    npm: 5.6.0 - /usr/local/bin/npm
  npmPackages:
    jest: 23.5.0 => 23.5.0 

Other relevant information

  • Running require('clear-module').all() in a transformer as a workaround to clear require cache breaks if you have binary modules such as farmhash.
  • Running separate jest processes for each project allows tests to pass, but result in two separate coverage reports, which is not ideal
  • Running 20 tests (sequential mode) instead of 22 does not exhibit the bug, but being limited to less than 20 tests is not ideal
  • Running with --runInBand also does not exhibit the bug but doing so can increase CI time significantly
@lhorie lhorie changed the title browser field is not respected in concurrent mode browser field is not respected in concurrent mode w/ multi-project config Aug 24, 2018
lhorie added a commit to fusionjs/fusion-cli that referenced this issue Aug 24, 2018
- clearing require.cache causes `Module did not self-register` error from binary dependencies
- original issue was isolated in jestjs/jest#6887
- workaround: `fusion test --env node && fusion test --env jsdom`
- downsides: coverage for node and jsdom tests are not consolidated

Next steps:

- tell people to use the workaround
- wait for upstream fix in Jest
lhorie added a commit to fusionjs/fusion-cli that referenced this issue Aug 24, 2018
* Reverts require.cache busting

- clearing require.cache causes `Module did not self-register` error from binary dependencies
- original issue was isolated in jestjs/jest#6887
- workaround: `fusion test --env node && fusion test --env jsdom`
- downsides: coverage for node and jsdom tests are not consolidated

Next steps:

- tell people to use the workaround
- wait for upstream fix in Jest

* stop flow nag
@ranyitz
Copy link
Contributor

ranyitz commented Aug 30, 2018

@lhorie Thanks for opening the issue!

TL;DR
A quick fix would be to add name attribute to your projects to have a unique resolver for each one of them.

I've confirmed the bug and i'll try to explain why it happens:

  1. Jest is using workers to run tests in parallel. The bug occurs when the same worker was used from the node project and then from the browser project.

  2. Jest creates a unique resolver for each context (project)

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-cli/src/lib/create_context.js#L23

  1. Each resolver has its own cache.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-resolve/src/index.js#L132-L136

  1. When running tests serially (runInBand) every test runs with its own initialized resolver.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-runner/src/index.js#L76-L81

  1. For Workers, which don't run on the same process we can't use the unserializeable instance of the resolver, so we just hand over the config.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-runner/src/index.js#L117-L118

  1. And then create the the resolvers inside of the worker process.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-runner/src/test_worker.js#L79-L84

  1. The different resolvers are saved in memory for each worker and identify by name.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-runner/src/test_worker.js#L61-L68

  1. The name property is automatically set if missing, computed as a function of the rootDir which is in your case, identical between both projects.

https://github.com/facebook/jest/blob/8e72341c6df9af92a5e95e4a5784923baf5245de/packages/jest-config/src/normalize.js#L244-L250

  1. It means that you get the same resolver, hence you use the same resolver cache and get the same value for require('is-browser') (when you've used a different browser configuration).

Summary

A quick fix would be to create a different rootDir/name attribute for each project, and you'll have a different resolver.

A proposed solution would be to compute the name used for the resolver from the whole config, or something which is unique enough to not be similar between different projects.

@KevinGrandon
Copy link

Wow, thanks for the great write-up and investigation into this bug!

A proposed solution would be to compute the name used for the resolver from the whole config, or something which is unique enough to not be similar between different projects.

I think this would be great if you could compute the name from the entire config. +1 from me.

@SimenB
Copy link
Member

SimenB commented Feb 7, 2019

This should be solved in Jest 24 as we generate a name if it's missing when using projects. So I'm gonna close this, but if you can reproduce this issue in Jest 24, I'll reopen.

PR: #5862 (followed by #7746)

@SimenB SimenB closed this as completed Feb 7, 2019
@github-actions
Copy link

This issue 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 May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants