jest icon indicating copy to clipboard operation
jest copied to clipboard

Add `runnerConfig` option to configure runners

Open rogeliog opened this issue 8 years ago • 25 comments

Do you want to request a feature or report a bug?

Enhancement

What is the current behavior?

There is no way of configuring a test runner

What is the expected behavior?

It would be nice to have a way to configure runners. For example: runnerConfig which could be any arbitrary object that is relevant for the runner configuration.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

rogeliog avatar Aug 16 '17 02:08 rogeliog

@cpojer @aaronabramov If we do decide to move forward with this, what do you think should be the interface here? runnerConfig: Object?

rogeliog avatar Sep 01 '17 20:09 rogeliog

maybe something like what we do with reporters?

runner: 'some_runner.js'

// or

runner: ['some_runner.js', {hey: 'yo'}]

which translates to

// globalConfig.runner
{
  path: 'some_runner.js',
  options: {hey: 'yo'}
}

and then

const Runner = require(globalConfig.runner.path);
const runner = new Runner(globalConfig.runner.options);

boujeepossum avatar Sep 01 '17 20:09 boujeepossum

I actually had something quite different in mind because ProjectConfig is mostly local to the runner. The ideal system would be to have a base ProjectConfig (with a runner option) that can include both the runner configuration as well as any custom configuration for that runner. ProjectConfig is then both the config for the project and the configuration for the runner.

The challenge here is how to make this type safe and how to make the configuration system ultimately configurable.

Most things in Jest would be typed with BaseProjectConfig with the minimal set of options that any runner (even for non-test use cases) would share, which is threaded to most places in Jest. Then there would be a type JestProjectConfig which is threaded to everything from jest-runner onwards to jest-jasmine etc. I think this change is quite involved, but the end-state sounds enticing to me. What do you think?

cpojer avatar Sep 01 '17 20:09 cpojer

i'm a little worried about mixing Jest projectConfig with other custom configuration. I really don't like how it worked with global+project configs sharing the same JSON object, most of the bugs we discovered with MPR were super hard to debug and were caused by some values being in the wrong config. But i think if we have a separate typed projectConfig with a custom/possibly untyped projectConfig.runnerConfig it might work well

boujeepossum avatar Sep 01 '17 20:09 boujeepossum

The GlobalConfig + ProjectConfig issues were a one time cost that the split brought with it, and I think now (as of Jest 21), the system is working quite smoothly. The thing that worries me the most is adding an uncontrollable amount of config and cli options (it's already a bit of a mess) and making ProjectConfig something that is theoretically pluggable would counter that, as people could build their own runners that cut down on the number of configuration options. Also, to build runners for things like prettier/eslint etc., all the test related configuration options don't really make any sense.

cpojer avatar Sep 01 '17 20:09 cpojer

So is there still no way to pass custom options to a custom runner?

ljharb avatar Jan 31 '18 20:01 ljharb

Is anybody working on this?

borela avatar Jul 18 '18 18:07 borela

Nope, feel free to grab it :)

thymikee avatar Jul 18 '18 18:07 thymikee

I installed the dependencies and ran the tests, many of them break on windows, that needs to be solved first.

borela avatar Jul 18 '18 22:07 borela

We have appveyor running as CI, so developing on windows should be fine... You'll need mercurial installed for a few tests, but besides that you should be good What do you have issues with?

SimenB avatar Jul 18 '18 23:07 SimenB

It seams most tests are failing because somehow paths are being prepended with ../../../../../Alexandre/AppData/Local/Temp/.

 FAIL  packages/jest-resolve/src/__tests__/resolve.test.js
  ● resolveModule › is possible to resolve node modules by resolving their realpath

    Cannot find module '../../src/__mocks__/bar/node_modules/foo/index.js' from 'resolve.test.js'

      at Resolver.resolveModule (packages/jest-resolve/build/index.js:221:17)

 FAIL  e2e/__tests__/only_changed.test.js (45.124s)
  ● run only changed files

    expect(received).toMatch(expected)

    Expected value to match:
      /PASS __tests__(\/|\\)file1.test.js/
    Received:
      "PASS ../../../../../Alexandre/AppData/Local/Temp/jest_only_changed/__tests__/file1.test.js
      ✓ file1 (3ms)

    Test Suites: 1 passed, 1 total
    Tests:       1 passed, 1 total
    Snapshots:   0 total
    Time:        4.154s
    Ran all test suites related to changed files."

      43 |
      44 |   ({stderr} = runJest(DIR, ['-o', '--lastCommit']));
    > 45 |   expect(stderr).toMatch(/PASS __tests__(\/|\\)file1.test.js/);
         |                  ^
      46 |
      47 |   writeFiles(DIR, {
      48 |     '__tests__/file2.test.js': `require('../file2'); test('file2', () => {});`,

      at Object.<anonymous>.test (e2e/__tests__/only_changed.test.js:45:18)

  ● onlyChanged in config is overwritten by --all or testPathPattern

    expect(received).toMatch(expected)

    Expected value to match:
      /PASS __tests__(\/|\\)file1.test.js/
    Received:
      "PASS ../../../../../Alexandre/AppData/Local/Temp/jest_only_changed/__tests__/file1.test.js
      ✓ file1 (2ms)

    Test Suites: 1 passed, 1 total
    Tests:       1 passed, 1 total
    Snapshots:   0 total
    Time:        3.449s
    Ran all test suites related to changed files."

      143 |
      144 |   ({stderr} = runJest(DIR, ['--lastCommit']));
    > 145 |   expect(stderr).toMatch(/PASS __tests__(\/|\\)file1.test.js/);
          |                  ^
      146 |
      147 |   writeFiles(DIR, {
      148 |     '__tests__/file2.test.js': `require('../file2'); test('file2', () => {});`,

      at Object.<anonymous>.test (e2e/__tests__/only_changed.test.js:145:18)

  ● gets changed files for hg

    Error

      34 |       ERROR: ${result.error}
      35 |     `;
    > 36 |     throw new Error(message);
         |           ^
      37 |   }
      38 |
      39 |   return result;

      Error:
            ORIGINAL CMD: hg --config ui.username=jest_test init
            STDOUT:
            STDERR: 'hg' is not recognized as an internal or external command,
      operable program or batch file.
            STATUS: 1
            ERROR: undefined

      at run (e2e/Utils.js:36:11)
      at Object.<anonymous> (e2e/__tests__/only_changed.test.js:201:3)
      at step (e2e/__tests__/only_changed.test.js:26:191)
      at e2e/__tests__/only_changed.test.js:26:437
      at Object.<anonymous> (e2e/__tests__/only_changed.test.js:26:99)

  ● path on Windows is case-insensitive

    expect(received).toMatch(expected)

    Expected value to match:
      /PASS __tests__(\/|\\)file2.test.js/
    Received:
      "PASS ../../../../../../../Alexandre/AppData/Local/Temp/jest_only_changed/outer_dir/inner_dir/__tests__/file3.test.js
    PASS ../../../../../../../Alexandre/AppData/Local/Temp/jest_only_changed/outer_dir/inner_dir/__tests__/file2.test.js

    Test Suites: 2 passed, 2 total
    Tests:       2 passed, 2 total
    Snapshots:   0 total
    Time:        7.849s
    Ran all test suites related to changed files."

      267 |
      268 |   expect(stderr).not.toMatch(/PASS __tests__(\/|\\)file1.test.js/);
    > 269 |   expect(stderr).toMatch(/PASS __tests__(\/|\\)file2.test.js/);
          |                  ^
      270 |   expect(stderr).toMatch(/PASS __tests__(\/|\\)file3.test.js/);
      271 | });
      272 |

      at Object.<anonymous>.test (e2e/__tests__/only_changed.test.js:269:18)

 FAIL  e2e/__tests__/globals.test.js (35.212s)
  ● basic test constructs

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "basic test constructs 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "PASS __tests__/basic.test-constructs.test.js
    + "PASS ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/basic.test-constructs.test.js
        ✓ it
        ✓ test
        describe
          ✓ it
          ✓ test

      48 |
      49 |   const {summary, rest} = extractSummary(stderr);
    > 50 |   expect(rest).toMatchSnapshot();
         |                ^
      51 |   expect(summary).toMatchSnapshot();
      52 | });
      53 |

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:50:16)

  ● skips

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "skips 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "PASS __tests__/skips-constructs.test.js
    + "PASS ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/skips-constructs.test.js
        ✓ it
        ○ skipped 4 tests
        xdescribe
          ○ skipped 2 tests
        describe.skip

      78 |
      79 |   const {summary, rest} = extractSummary(stderr);
    > 80 |   expect(rest).toMatchSnapshot();
         |                ^
      81 |   expect(summary).toMatchSnapshot();
      82 |   expect(status).toBe(0);
      83 | });

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:80:16)

  ● only

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "only 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "PASS __tests__/only-constructs.test.js
    + "PASS ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/only-constructs.test.js
        ✓ test.only
        ✓ it.only
        ✓ fit
        ○ skipped 1 test
        fdescribe

      109 |
      110 |   const {summary, rest} = extractSummary(stderr);
    > 111 |   expect(rest).toMatchSnapshot();
          |                ^
      112 |   expect(summary).toMatchSnapshot();
      113 | });
      114 |

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:111:16)

  ● cannot test with no implementation

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "cannot test with no implementation 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "FAIL __tests__/only-constructs.test.js
    + "FAIL ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/only-constructs.test.js
        ● Test suite failed to run

          Missing second argument. It must be a callback function.

            1 |

      126 |
      127 |   const {summary} = extractSummary(stderr);
    > 128 |   expect(cleanStderr(stderr)).toMatchSnapshot();
          |                               ^
      129 |   expect(summary).toMatchSnapshot();
      130 | });
      131 |

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:128:31)

  ● skips with expand arg

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "skips with expand arg 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "PASS __tests__/skips-constructs.test.js
    + "PASS ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/skips-constructs.test.js
        ✓ it
        ○ xtest
        ○ xit
        ○ it.skip
        ○ test.skip

      157 |
      158 |   const {summary, rest} = extractSummary(stderr);
    > 159 |   expect(rest).toMatchSnapshot();
          |                ^
      160 |   expect(summary).toMatchSnapshot();
      161 | });
      162 |

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:159:16)

  ● only with expand arg

    expect(value).toMatchSnapshot()

    Received value does not match stored snapshot "only with expand arg 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - "PASS __tests__/only-constructs.test.js
    + "PASS ../../../../../Alexandre/AppData/Local/Temp/global-variables.test/__tests__/only-constructs.test.js
        ○ it
        ✓ test.only
        ✓ it.only
        ✓ fit
        fdescribe

      187 |
      188 |   const {summary, rest} = extractSummary(stderr);
    > 189 |   expect(rest).toMatchSnapshot();
          |                ^
      190 |   expect(summary).toMatchSnapshot();
      191 | });
      192 |

      at Object.<anonymous>.test (e2e/__tests__/globals.test.js:189:16)

borela avatar Jul 18 '18 23:07 borela

Most things in Jest would be typed with BaseProjectConfig with the minimal set of options that any runner (even for non-test use cases) would share, which is threaded to most places in Jest. Then there would be a type JestProjectConfig which is threaded to everything from jest-runner onwards to jest-jasmine etc. I think this change is quite involved, but the end-state sounds enticing to me. What do you think?

That definitely sounds like an ideal solution, but I'm not sure how people would differentiate between project and global config. Do you imagine everything still being in a single flat configuration? Would it be possible at all to check for typos in configuration in normalize then, when we can't limit the keys present? I suppose we could load all global config, then load runners and either query them for schemas to pass config through for validation. I do think that typing out the config might be confusing for consumers though.

Something like this would feel like boilerplate I think:

{
  "jest": {
    // a bunch of global config,
    "runners": [
      // jest config
      {
        "module": "jest-runner",
        "testMatch": ["blabla"]
      },
      // eslint config
      {
        "module": "jest-runner-eslint",
        "include": ["blablabla"]
      }
    ]
  }
}

But any other option feels like it'd just be confusing, which is worse.


I do think @aaronabramov's first suggestion should be the way we go, at least for a first iteration. I do think to properly separate config we'll have to do a breaking change anyways, so why not design something properly at that point, but still solve the very real need of custom runner config with a simple tuple similar to reporters for now? That's also how we ended up passing config to watch plugins. Using cosmoconfig shouldn't be necessary.

SimenB avatar Sep 15 '18 18:09 SimenB

Just tried to write a runner and was surprised to find that this doesn't exist. I guess ones like jest-runner-eslint just rely on eslint picking up its own config via the standard ways rather than getting it from Jest (e.g. package.json, .eslintrc.json, etc) and I guess that's the only way to go for now.

I don't entirely understand the interaction between the project config and global config, so can't really comment intelligently on @aaronabramov's original suggestion (it looks like it would only support it in the global config, which isn't very useful if you're using one runner in one project and another in another, e.g. the default runner for tests in one project and the eslint runner for linting in another).

Edit: added https://github.com/elyobo/jest-runner-oas-linter, just uses config in package.json or in a top level .oaslintrc.json.

elyobo avatar Jun 07 '19 14:06 elyobo

Still no solution? If anyway can guide me a bit from where we can start that would be great.

Otherwise, we should use hacky solutions. And yes, there is a way. And yes, this should land in runners implementations that you want to use, until the official jest support comes.

The solution is that runners should try to read from config.haste[runner-name]. Yes, it gives warnings, but the important thing is that it doesn't break the process.

Why in config.haste? Glad you asked. You can actually put whatever name you want in your jest config file (module.exports = { foobar: 123 }), but it won't appear in the jest runner (e.g. { config }, the config.foobar won't exist), but haste appears, always.

I've tested that solution and it's great for now. I probably will fork the babel and eslint runners for now if they don't accept my PRs.

tunnckoCore avatar Jun 17 '19 00:06 tunnckoCore

Please don't contribute hacks to the runners. Either apply the patch locally or maybe spend this time to actually implement the feature in Jest. You can start with normalize.ts file and see how other similar options are configured and pass the parameters down

thymikee avatar Jun 17 '19 06:06 thymikee

Yea, agree. Thanks for the starting point.

tunnckoCore avatar Jun 17 '19 06:06 tunnckoCore

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Feb 26 '22 10:02 github-actions[bot]

bump

ljharb avatar Feb 26 '22 15:02 ljharb

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Feb 26 '23 15:02 github-actions[bot]

bump

ljharb avatar Feb 26 '23 22:02 ljharb

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Feb 26 '24 22:02 github-actions[bot]

bump

ljharb avatar Feb 26 '24 22:02 ljharb

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Feb 26 '25 00:02 github-actions[bot]

bump

ljharb avatar Feb 26 '25 00:02 ljharb

We(jest-light-runner) need this feature too, for now we just added a different entry https://github.com/nicolo-ribaudo/jest-light-runner/blob/c47d1baefd7ce80c25b41290582a6a173b5e69ec/package.json#L10

fisker avatar Apr 18 '25 19:04 fisker