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

fix(jest-config): normalize reporters option defined in presets #12769

Merged
merged 5 commits into from Apr 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintrc.cjs
Expand Up @@ -383,7 +383,7 @@ module.exports = {
'no-dupe-args': 'error',
'no-dupe-class-members': 'error',
'no-dupe-keys': 'error',
'no-duplicate-case': 'warn',
'no-duplicate-case': 'error',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps? I just missed that warning, better if it errors?

Copy link
Member

Choose a reason for hiding this comment

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

agreed, that's always an error I think

'no-else-return': 'off',
'no-empty': 'off',
'no-empty-character-class': 'warn',
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

### Fixes

- `[jest-config]` Normalize `reporters` option defined in presets ([#12769](https://github.com/facebook/jest/pull/12769))
- `[@jest/reporters]` Fix trailing slash in matching `coverageThreshold` key ([#12714](https://github.com/facebook/jest/pull/12714))
- `[@jest/transform]` Throw better error if an invalid return value if encountered ([#12764](https://github.com/facebook/jest/pull/12764))

Expand Down
49 changes: 49 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.ts
Expand Up @@ -1416,6 +1416,55 @@ describe.each(['setupFiles', 'setupFilesAfterEnv'])(
},
);

describe("preset with 'reporters' option", () => {
beforeEach(() => {
const Resolver = require('jest-resolve').default;
Resolver.findNodeModule = jest.fn(name => {
if (name === 'with-reporters/jest-preset') {
return '/node_modules/with-reporters/jest-preset.json';
}

return `/node_modules/${name}`;
});
jest.doMock(
'/node_modules/with-reporters/jest-preset.json',
() => ({
reporters: ['default'],
}),
{virtual: true},
);
});

afterEach(() => {
jest.dontMock('/node_modules/with-reporters/jest-preset.json');
});

test("normalizes 'reporters' option defined in preset", async () => {
const {options} = await normalize(
{
preset: 'with-reporters',
rootDir: '/root/',
},
{} as Config.Argv,
);

expect(options.reporters).toEqual([['default', {}]]);
});

test("overrides 'reporters' option defined in preset", async () => {
const {options} = await normalize(
{
preset: 'with-reporters',
reporters: ['summary'],
rootDir: '/root/',
},
{} as Config.Argv,
);

expect(options.reporters).toEqual([['summary', {}]]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm.. reporters from preset is discarded if config includes reporters key. Just thinking if this expected? Or it would be better to merge? Merging probably would make it impossible to disable reporters from a preset.

Edit: I just added a test, there is no change in behaviour. The test is passing on main as well.

Copy link
Member

Choose a reason for hiding this comment

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

not sure if ideal, but not something we should change here anyways

});
});

describe('runner', () => {
let Resolver;
beforeEach(() => {
Expand Down
33 changes: 18 additions & 15 deletions packages/jest-config/src/normalize.ts
Expand Up @@ -375,14 +375,19 @@ const normalizeRootDir = (
};
};

const normalizeReporters = (options: Config.InitialOptionsWithRootDir) => {
const reporters = options.reporters;
const normalizeReporters = ({
reporters,
rootDir,
}: Config.InitialOptionsWithRootDir):
| Array<Config.ReporterConfig>
| undefined => {
if (!reporters || !Array.isArray(reporters)) {
return options;
return undefined;
}

validateReporters(reporters);
options.reporters = reporters.map(reporterConfig => {

return reporters.map(reporterConfig => {
const normalizedReporterConfig: Config.ReporterConfig =
typeof reporterConfig === 'string'
? // if reporter config is a string, we wrap it in an array
Expand All @@ -392,13 +397,13 @@ const normalizeReporters = (options: Config.InitialOptionsWithRootDir) => {
: reporterConfig;

const reporterPath = replaceRootDirInPath(
options.rootDir,
rootDir,
normalizedReporterConfig[0],
);

if (!['default', 'github-actions', 'summary'].includes(reporterPath)) {
const reporter = Resolver.findNodeModule(reporterPath, {
basedir: options.rootDir,
basedir: rootDir,
});
if (!reporter) {
throw new Resolver.ModuleNotFoundError(
Expand All @@ -410,8 +415,6 @@ const normalizeReporters = (options: Config.InitialOptionsWithRootDir) => {
}
return normalizedReporterConfig;
});

return options;
};

const buildTestPathPattern = (argv: Config.Argv): string => {
Expand Down Expand Up @@ -536,12 +539,10 @@ export default async function normalize(
],
});

let options = normalizeReporters(
normalizeMissingOptions(
normalizeRootDir(setFromArgv(initialOptions, argv)),
configPath,
projectIndex,
),
let options = normalizeMissingOptions(
normalizeRootDir(setFromArgv(initialOptions, argv)),
configPath,
projectIndex,
);

if (options.preset) {
Expand Down Expand Up @@ -749,6 +750,9 @@ export default async function normalize(
];
});
break;
case 'reporters':
mrazauskas marked this conversation as resolved.
Show resolved Hide resolved
value = normalizeReporters(oldOptions);
break;
case 'coveragePathIgnorePatterns':
case 'modulePathIgnorePatterns':
case 'testPathIgnorePatterns':
Expand Down Expand Up @@ -938,7 +942,6 @@ export default async function normalize(
case 'outputFile':
case 'passWithNoTests':
case 'replname':
case 'reporters':
case 'resetMocks':
case 'resetModules':
case 'restoreMocks':
Expand Down