feat(config): add read initial options helper #13356
Conversation
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.
|
@SimenB I think 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. |
|
I don't know why the circleci tests are failing 😟 |
Integration test should be in |
| * Indicates whether or not to ignore the error of jest finding multiple config files. | ||
| * (default is false) | ||
| */ | ||
| skipMultipleConfigError?: boolean; |
There was a problem hiding this comment.
Does this need to be an option? Seems one should always fail of that's the case
There was a problem hiding this comment.
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?
|
@SimenB thanks for the fast review! With regards to I get this weird UPDATE: I think I still had an error in my implementation. I get this clear error now: |
Co-authored-by: Tom Mrazauskas <[email protected]>
|
The e2e test for reading 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. |
|
The difference between The problem lies in this line of code:
I don't know why a |
Co-authored-by: Tom Mrazauskas <[email protected]>
Co-authored-by: Tom Mrazauskas <[email protected]>
Co-authored-by: Tom Mrazauskas <[email protected]>
|
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'");
}); |
|
Yeah, exactly. This is what I was thinking later as well. That approach will also work for a I will update this PR tomorrow morning. |
|
I've fixed my test 🎉 Don't know why some pipelines are failing 😢 |
| // 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, | ||
| }); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
True, using execa is much better. Fixed with the latest push
| // Otherwise just try to find config in the current rootDir. | ||
| configPath = resolveConfigPath( | ||
| const {config: initialOptions, configPath} = await readInitialOptions( | ||
| argv?.config, |
There was a problem hiding this comment.
| argv?.config, | |
| argv.config, |
There was a problem hiding this comment.
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',
);
});There was a problem hiding this comment.
that's a bug in the test - argv is always called according to the types
There was a problem hiding this comment.
Ok, updated the tests and removed this ?
Co-authored-by: Simen Bekkhus <[email protected]>
| // 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` |
There was a problem hiding this comment.
This comment should be moved two lines down. Or?
Co-authored-by: Simen Bekkhus <[email protected]>
|
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. |
Summary
Add
readInitialOptionsfunction 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.mjsfile, 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.tsfile, I had to enable--experimental-vm-modulesto run jest on jest. If you see problems with this, I can also remove thejest.config.tstest 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