feat(config): add read initial options helper #13356

nicojs · 2022-10-02T11:24:46Z

Summary

Add readInitialOptions function to the jest-config package. This helper function can be useful for consumers of the jest programmatic API. It exposes the functionality that jest uses to read config from different sources.

Test plan

I've added integration tests to the jest-config package. I couldn't add tests for a jest.config.mjs file, because it kept being read as a cjs file. I think this has to do with babel transforming.

To add a test for a jest.config.ts file, I had to enable --experimental-vm-modules to run jest on jest. If you see problems with this, I can also remove the jest.config.ts test altogether (or we can fix it somehow?)

I've tested it on the integration test suite in StrykerJS, and it works as expected.

Closes #13348

Add `readInitialOptions` function to the jest-config package. This is a helper function that can be useful for consumers of the jest programmatic api. It exposes the functionality that jest uses to read config from different sources.

nicojs · 2022-10-02T11:33:42Z

@SimenB I think readInitialOptions is a better name than readRawOptions. Do you agree?

I've decided to make the API a bit more precise; instead of 5 positional arguments, we now have 2 of which the second argument is an "options" parameter, where you are forced to explicitly name each option you want to override.

nicojs · 2022-10-02T11:38:11Z

I don't know why the circleci tests are failing 😟

SimenB · 2022-10-02T11:52:25Z

I've added integration tests to the jest-config package.

Integration test should be in e2e/ directory. That way there's no need for experimental flags either

SimenB

Exciting!

CHANGELOG.md

packages/jest-config/src/index.ts

SimenB · 2022-10-02T11:54:53Z

packages/jest-config/src/index.ts

+     * Indicates whether or not to ignore the error of jest finding multiple config files.
+     * (default is false)
+     */
+    skipMultipleConfigError?: boolean;


Does this need to be an option? Seems one should always fail of that's the case

It was already supported by readConfig. Since readConfig is using readInitialOptions internally I thought I would add this feature to readInitialOptions as well.

Removing this feature altogether might be a breaking change, right?

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

nicojs · 2022-10-02T12:07:55Z

@SimenB thanks for the fast review!

With regards to --experimental-vm-modules, reading from a jest.config.ts file without it doesn't seem to work in e2e directory either. I've pushed it now so we can see the error in CI.

I get this weird Segmentation fault error right before the test process crashes:

nicojs@NicoJ01 ~/github/jest (feat/read-initial-options)$  cd /home/nicojs/github/jest ; /usr/bin/env 'NODE_OPTIONS=--require /home/nicojs/.vscode-server/bin/74b1f979648cc44d385a2286793c226e611f59e7/extensions/ms-vscode.js-debug/src/bootloader.bundle.js --inspect-publish-uid=http' 'VSCODE_INSPECTOR_OPTIONS={"inspectorIpc":"/tmp/node-cdp.943-83.sock","deferredMode":false,"waitForDebugger":"","execPath":"/home/nicojs/.nvm/versions/node/v16.15.0/bin/node","onlyEntrypoint":false,"autoAttachMode":"always","mandatePortTracking":true,"fileCallback":"/tmp/node-debug-callback-45176a3390f07d11"}' /home/nicojs/.nvm/versions/node/v16.15.0/bin/node ./node_modules/.bin/jest readInitialOptions.test 
Debugger attached.

 RUNS  e2e/__tests__/readInitialOptions.test.ts
Segmentation fault

UPDATE: I think I still had an error in my implementation. I get this clear error now:

● readInitialOptions › should read a jest.config.ts file

    Jest: Failed to parse the TypeScript config file /home/nicojs/github/jest/e2e/read-initial-options/ts-config/jest.config.ts
      Error: You need to run with a version of node that supports ES Modules in the VM API. See https://jestjs.io/docs/ecmascript-modules

      38 |     const configFile = resolveFixture('ts-config', 'jest.config.ts');
      39 |     const rootDir = resolveFixture('ts-config');
    > 40 |     const {config, configPath} = await readInitialOptions(configFile);
         |                                  ^
      41 |     expect(config).toEqual({jestConfig: 'jest.config.ts', rootDir});
      42 |     expect(configPath).toEqual(configFile);
      43 |   });

      at readConfigFileAndSetRootDir (packages/jest-config/build/readConfigFileAndSetRootDir.js:136:13)
      at Object.<anonymous> (e2e/__tests__/readInitialOptions.test.ts:40:34)

packages/jest-config/src/__tests__/readInitialOptions.test.ts

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

packages/jest-config/src/__tests__/readInitialOptions.test.ts

mrazauskas · 2022-10-02T13:01:41Z

The e2e test for reading jest.config.ts lives here. Perhaps there is some clue?

Just an idea – would you consider using the pattern from the mentioned file for the e2e test in this PR? It works well with simple fixtures as in this case.

nicojs · 2022-10-02T13:20:07Z

The difference between jest.config.ts.test.ts and these new e2e tests is that I'm using the readInitialConfig API directly, while the other tests are spawning a new jest instance. It probably has to do with that.

The problem lies in this line of code:

https://github.com/nicojs/jest/blob/c2dffe995af845ccc0bd4affc936d8013f90bf19/packages/jest-config/src/readConfigFileAndSetRootDir.ts#L112

I don't know why a import expression would yield such an error 🤷‍♀️

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

mrazauskas · 2022-10-02T17:19:37Z

If I did not miss something, this e2e test is passing:

import * as path from 'path';
import execa = require('execa');
import {cleanup, writeFiles} from '../Utils';

const DIR = path.resolve(__dirname, '../read-initial-options');

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

test('reads a jest.config.ts file', async () => {
  writeFiles(DIR, {
    'jest.config.ts': "export default {jestConfig: 'jest.config.ts'};",
    'package.json': '{}',
    'readConfig.js': `
      const {readInitialOptions} = require('jest-config');
      async function readConfig() {
        console.log(await readInitialOptions());
      }
      readConfig();
    `,
  });

  const {stderr, stdout} = await execa('node', ['readConfig.js'], {cwd: DIR});

  expect(stderr).toBe('');
  expect(stdout).toMatch("jestConfig: 'jest.config.ts'");
});

nicojs · 2022-10-02T18:30:50Z

Yeah, exactly. This is what I was thinking later as well. That approach will also work for a jest.config.mjs file I think. Probably the root cause is jest running tests in a node VM (at least, that is my understanding now).

I will update this PR tomorrow morning.

nicojs · 2022-10-03T06:41:31Z

I've fixed my test 🎉

Don't know why some pipelines are failing 😢

mrazauskas · 2022-10-03T07:06:16Z

e2e/__tests__/readInitialOptions.test.ts

+    // Read by proxy, because we're running in a VM and are not allowed to `import` directly
+    const {stdout} = await execa('node', ['../readOptions.js'], {
+      cwd: rootDir,
+    });


For my understanding this approach should be used in all test in this file. It is based on real world implementation and that is how I like e2e tests to be written.

In contrary, importing readInitialOptions to a test file and mocking process.cwd still feels like a unit test. Of course, that just an approach or opinion. The same thing is tested either way.

True, using execa is much better. Fixed with the latest push

e2e/__tests__/readInitialOptions.test.ts

SimenB

looking good!

SimenB · 2022-10-03T08:12:01Z

packages/jest-config/src/index.ts

-    // Otherwise just try to find config in the current rootDir.
-    configPath = resolveConfigPath(
+  const {config: initialOptions, configPath} = await readInitialOptions(
+    argv?.config,


Suggested change

argv?.config,

argv.config,

This fails the readConfig test. I assumed you would want to keep supporting it.

See packages/jest-config/src/tests/readConfig.test.ts

test('readConfig() throws when an object is passed without a file path', async () => { await expect( readConfig( // @ts-expect-error null /* argv */, {} /* packageRootOrConfig */, false /* skipArgvConfigOption */, null /* parentConfigPath */, ), ).rejects.toThrowError( 'Jest: Cannot use configuration as an object without a file path', ); });

that's a bug in the test - argv is always called according to the types

Ok, updated the tests and removed this ?

packages/jest-config/src/index.ts

e2e/__tests__/readInitialOptions.test.ts

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

mrazauskas · 2022-10-03T08:39:22Z

packages/jest-config/src/index.ts

+    // A string passed to `--config`, which is either a direct path to the config
+    // or a path to directory containing `package.json`, `jest.config.js` or `jest.config.ts`


This comment should be moved two lines down. Or?

You are right! Fixed

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

SimenB

thanks!

SimenB · 2022-10-14T09:16:08Z

https://github.com/facebook/jest/releases/tag/v29.2.0

github-actions · 2022-11-14T00:10:00Z

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.

nicojs added 2 commits October 2, 2022 13:20

feat(config): add read initial options helper

8d598dd

Add `readInitialOptions` function to the jest-config package. This is a helper function that can be useful for consumers of the jest programmatic api. It exposes the functionality that jest uses to read config from different sources.

Enable experimental-vm-modules to run jest.

aabb8de

facebook-github-bot added the cla signed label Oct 2, 2022

nicojs added 2 commits October 2, 2022 13:28

Update changelog

e9b5c87

Add copyright headers

b6276fb

Update assert: also use forward slashes on windows.

d93a12c

SimenB reviewed Oct 2, 2022

View reviewed changes

nicojs and others added 4 commits October 2, 2022 13:56

Update packages/jest-config/src/index.ts

eadd24e

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

Update changelog sorting

947bfaa

Move integration tests to e2e

7793ff2

Remove --experimental-vm-modules

153af9b

nicojs added 2 commits October 2, 2022 14:10

Extract ReadJestConfigOptions interface

4294610

Finish rename readFromCwdInstead -> readFromCwd

6d347a6

mrazauskas reviewed Oct 2, 2022

View reviewed changes

packages/jest-config/src/__tests__/readInitialOptions.test.ts Outdated Show resolved Hide resolved

Import from @jest/types instead of jest-runner

c2dffe9

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

mrazauskas reviewed Oct 2, 2022

View reviewed changes

packages/jest-config/src/__tests__/readInitialOptions.test.ts Outdated Show resolved Hide resolved

packages/jest-config/src/__tests__/readInitialOptions.test.ts Outdated Show resolved Hide resolved

packages/jest-config/src/__tests__/readInitialOptions.test.ts Outdated Show resolved Hide resolved

nicojs and others added 4 commits October 2, 2022 15:23

Update packages/jest-config/src/__tests__/readInitialOptions.test.ts

b3a13cc

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

Update packages/jest-config/src/__tests__/readInitialOptions.test.ts

c41dbdf

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

Update packages/jest-config/src/__tests__/readInitialOptions.test.ts

8b479f5

Co-authored-by: Tom Mrazauskas <tom@mrazauskas.de>

Remove final .name

9f01d80

nicojs added 2 commits October 3, 2022 08:24

Read config by proxy for ts and mjs files

188b080

Add copyright header

5c54a6a

nicojs mentioned this pull request Oct 3, 2022

fix(jest): support more config file formats stryker-mutator/stryker-js#3761

Merged

mrazauskas reviewed Oct 3, 2022

View reviewed changes

SimenB reviewed Oct 3, 2022

View reviewed changes

e2e/__tests__/readInitialOptions.test.ts Outdated Show resolved Hide resolved

Read by proxy for every test

53b49ba

SimenB approved these changes Oct 3, 2022

View reviewed changes

nicojs and others added 2 commits October 3, 2022 10:22

Update e2e/__tests__/readInitialOptions.test.ts

34e1a32

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

Refactor if/else -> if

3cf1f21

mrazauskas reviewed Oct 3, 2022

View reviewed changes

nicojs and others added 3 commits October 3, 2022 13:28

refactor: move some comments around

3d535ba

Update e2e/__tests__/readInitialOptions.test.ts

5ab7237

Co-authored-by: Simen Bekkhus <sbekkhus91@gmail.com>

Require argv to be supplied

2dfe6ef

SimenB approved these changes Oct 3, 2022

View reviewed changes

SimenB merged commit d78baab into jestjs:main Oct 3, 2022

nicojs deleted the feat/read-initial-options branch October 3, 2022 16:17

ms-bot mentioned this pull request Nov 7, 2022

Bump 🧳 jest to ^29.3.1 mobsuccess-devops/github-actions-mobsuccess#35

Closed

github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): add read initial options helper #13356

feat(config): add read initial options helper #13356

nicojs commented Oct 2, 2022 •

edited

Loading

nicojs commented Oct 2, 2022 •

edited

Loading

nicojs commented Oct 2, 2022

SimenB commented Oct 2, 2022

SimenB left a comment

SimenB Oct 2, 2022

nicojs Oct 2, 2022

nicojs commented Oct 2, 2022 •

edited

Loading

mrazauskas commented Oct 2, 2022

nicojs commented Oct 2, 2022

mrazauskas commented Oct 2, 2022

nicojs commented Oct 2, 2022

nicojs commented Oct 3, 2022

mrazauskas Oct 3, 2022

nicojs Oct 3, 2022

SimenB left a comment

SimenB Oct 3, 2022

nicojs Oct 3, 2022 •

edited

Loading

SimenB Oct 3, 2022 •

edited

Loading

nicojs Oct 3, 2022

mrazauskas Oct 3, 2022

nicojs Oct 3, 2022

SimenB left a comment

SimenB commented Oct 14, 2022

github-actions bot commented Nov 14, 2022

		// A string passed to `--config`, which is either a direct path to the config
		// or a path to directory containing `package.json`, `jest.config.js` or `jest.config.ts`

feat(config): add read initial options helper #13356

feat(config): add read initial options helper #13356

Conversation

nicojs commented Oct 2, 2022 • edited Loading

Summary

Test plan

nicojs commented Oct 2, 2022 • edited Loading

nicojs commented Oct 2, 2022

SimenB commented Oct 2, 2022

SimenB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicojs commented Oct 2, 2022 • edited Loading

mrazauskas commented Oct 2, 2022

nicojs commented Oct 2, 2022

mrazauskas commented Oct 2, 2022

nicojs commented Oct 2, 2022

nicojs commented Oct 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicojs Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

SimenB Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB left a comment

Choose a reason for hiding this comment

SimenB commented Oct 14, 2022

github-actions bot commented Nov 14, 2022

nicojs commented Oct 2, 2022 •

edited

Loading

nicojs commented Oct 2, 2022 •

edited

Loading

nicojs commented Oct 2, 2022 •

edited

Loading

nicojs Oct 3, 2022 •

edited

Loading

SimenB Oct 3, 2022 •

edited

Loading