Skip to content

Commit

Permalink
fix: ignore custom Jest configuration file and warn if one is found
Browse files Browse the repository at this point in the history
Currently with Jest we're experimenting with whether or not Angular CLI can meet user needs without exposing the underlying configuration file. See angular#25434 (comment) for more context.

Jest was incorrectly picking up custom configurations, even though they exist outside Jest's root directory. This commit fixes that behavior so Jest does not see user configurations and does not apply them. Ideally we would throw an error if a Jest configuration is found to be more clear that it isn't doing what developers expect. However they may use a Jest config to run other projects in the same workspace outside of Angular CLI and we don't want to prevent that. A warning seems like the best trade off of notifying users that a custom config isn't supported while also not preventing unrelated Jest usage in the same workspace.
  • Loading branch information
dgp1130 committed Jan 11, 2024
1 parent f2f0ac4 commit b1fb7fd
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 1 deletion.
38 changes: 37 additions & 1 deletion packages/angular_devkit/build_angular/src/builders/jest/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
import { execFile as execFileCb } from 'child_process';
import { promises as fs } from 'fs';
import { platform } from 'os';
import * as path from 'path';
import { promisify } from 'util';
import { colors } from '../../utils/color';
Expand All @@ -20,6 +22,8 @@ import { Schema as JestBuilderSchema } from './schema';

const execFile = promisify(execFileCb);

const USING_WINDOWS = platform() === 'win32';

/** Main execution function for the Jest builder. */
export default createBuilder(
async (schema: JestBuilderSchema, context: BuilderContext): Promise<BuilderOutput> => {
Expand Down Expand Up @@ -54,8 +58,25 @@ export default createBuilder(
};
}

const [testFiles, customConfig] = await Promise.all([
findTestFiles(options.include, options.exclude, context.workspaceRoot),
findCustomJestConfig(context.workspaceRoot),
]);

// Warn if a custom Jest configuration is found. We won't use it, so if a developer is trying to use a custom config, this hopefully
// makes a better experience than silently ignoring the configuration.
// Ideally, this would be a hard error. However a Jest config could exist for testing other files in the workspace outside of Angular
// CLI, so we likely can't produce a hard error in this situation without an opt-out.
if (customConfig) {
context.logger.warn(
'A custom Jest config was found, but this is not supported by `@angular-devkit/build-angular:jest` and will be' +
` ignored: ${customConfig}. This is an experiment to see if completely abstracting away Jest's configuration is viable. Please` +
` consider if your use case can be met without directly modifying the Jest config. If this is a major obstacle for your use` +
` case, please post it in this issue so we can collect feedback and evaluate: https://github.com/angular/angular-cli/issues/25434.`,
);
}

// Build all the test files.
const testFiles = await findTestFiles(options.include, options.exclude, context.workspaceRoot);
const jestGlobal = path.join(__dirname, 'jest-global.mjs');
const initTestBed = path.join(__dirname, 'init-test-bed.mjs');
const buildResult = await build(context, {
Expand Down Expand Up @@ -85,6 +106,7 @@ export default createBuilder(
jest,

`--rootDir="${path.join(testOut, 'browser')}"`,
`--config=${path.join(__dirname, 'jest.config.mjs')}`,
'--testEnvironment=jsdom',

// TODO(dgp1130): Enable cache once we have a mechanism for properly clearing / disabling it.
Expand Down Expand Up @@ -162,3 +184,17 @@ function resolveModule(module: string): string | undefined {
return undefined;
}
}

/** Returns whether or not the provided directory includes a Jest configuration file. */
async function findCustomJestConfig(dir: string): Promise<string | undefined> {
const entries = await fs.readdir(dir, { withFileTypes: true });

// Jest supports many file extensions (`js`, `ts`, `cjs`, `cts`, `json`, etc.) Just look
// for anything with that prefix.
const config = entries.find((entry) => entry.isFile() && entry.name.startsWith('jest.config.'));
if (!config) {
return undefined;
}

return path.join(dir, config.name);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

// Empty config file, everything is specified via CLI options right now.
// This file is used just so Jest doesn't accidentally inherit a custom user-specified Jest config.
export default {};
25 changes: 25 additions & 0 deletions tests/legacy-cli/e2e/tests/jest/custom-config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { writeFile } from '../../utils/fs';
import { applyJestBuilder } from '../../utils/jest';
import { ng } from '../../utils/process';

export default async function (): Promise<void> {
await applyJestBuilder();

// Users may incorrectly write a Jest config believing it to be used by Angular.
await writeFile(
'jest.config.mjs',
`
export default {
runner: 'does-not-exist',
};
`.trim(),
);

// Should not fail from the above (broken) configuration. Shouldn't use it at all.
const { stderr } = await ng('test');

// Should warn that a Jest configuration was found but not used.
if (!stderr.includes('A custom Jest config was found')) {
throw new Error(`No warning about custom Jest config:\nSTDERR:\n\n${stderr}`);
}
}

0 comments on commit b1fb7fd

Please sign in to comment.