Skip to content

Commit 0e83fe1

Browse files
clydinalan-agius4
clydin
authored and
alan-agius4
committed
fix(@schematics/angular): add warnings and improve Karma config generation
This commit enhances the `config` schematic to provide better guidance and correctness when generating a Karma configuration file. Changes include: - The `addKarmaConfig` function now correctly handles projects using the `@angular/build:unit-test` builder. - When the `unit-test` builder is in use, the schematic checks the `runner` option in both the main `options` and all `configurations` of the `test` target. - Warnings are issued if the `runner` is not explicitly set to `karma` (defaulting to Vitest) or if it's set to a different runner, informing the user that the generated `karma.conf.js` might not be used. - Corresponding unit tests have been added to `packages/schematics/angular/config/index_spec.ts` to validate this new warning behavior and ensure correct interaction with the `unit-test` builder. (cherry picked from commit b5f4192)
1 parent b91fa31 commit 0e83fe1

File tree

2 files changed

+161
-40
lines changed

2 files changed

+161
-40
lines changed

‎packages/schematics/angular/config/index.ts‎

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -47,49 +47,88 @@ async function addBrowserslistConfig(projectRoot: string): Promise<Rule> {
4747
}
4848

4949
function addKarmaConfig(options: ConfigOptions): Rule {
50-
return updateWorkspace((workspace) => {
51-
const project = workspace.projects.get(options.project);
52-
if (!project) {
53-
throw new SchematicsException(`Project name "${options.project}" doesn't not exist.`);
54-
}
50+
return (_, context) =>
51+
updateWorkspace((workspace) => {
52+
const project = workspace.projects.get(options.project);
53+
if (!project) {
54+
throw new SchematicsException(`Project name "${options.project}" doesn't not exist.`);
55+
}
5556

56-
const testTarget = project.targets.get('test');
57-
if (!testTarget) {
58-
throw new SchematicsException(
59-
`No "test" target found for project "${options.project}".` +
60-
' A "test" target is required to generate a karma configuration.',
61-
);
62-
}
57+
const testTarget = project.targets.get('test');
58+
if (!testTarget) {
59+
throw new SchematicsException(
60+
`No "test" target found for project "${options.project}".` +
61+
' A "test" target is required to generate a karma configuration.',
62+
);
63+
}
6364

64-
if (
65-
testTarget.builder !== AngularBuilder.Karma &&
66-
testTarget.builder !== AngularBuilder.BuildKarma
67-
) {
68-
throw new SchematicsException(
69-
`Cannot add a karma configuration as builder for "test" target in project does not` +
70-
` use "${AngularBuilder.Karma}" or "${AngularBuilder.BuildKarma}".`,
71-
);
72-
}
65+
if (
66+
testTarget.builder !== AngularBuilder.Karma &&
67+
testTarget.builder !== AngularBuilder.BuildKarma &&
68+
testTarget.builder !== AngularBuilder.BuildUnitTest
69+
) {
70+
throw new SchematicsException(
71+
`Cannot add a karma configuration as builder for "test" target in project does not` +
72+
` use "${AngularBuilder.Karma}", "${AngularBuilder.BuildKarma}", or ${AngularBuilder.BuildUnitTest}.`,
73+
);
74+
}
75+
76+
testTarget.options ??= {};
77+
if (testTarget.builder !== AngularBuilder.BuildUnitTest) {
78+
testTarget.options.karmaConfig = path.join(project.root, 'karma.conf.js');
79+
} else {
80+
// `unit-test` uses the `runnerConfig` option which has configuration discovery if enabled
81+
testTarget.options.runnerConfig = true;
7382

74-
testTarget.options ??= {};
75-
testTarget.options.karmaConfig = path.join(project.root, 'karma.conf.js');
83+
let isKarmaRunnerConfigured = false;
84+
// Check runner option
85+
if (testTarget.options.runner) {
86+
if (testTarget.options.runner === 'karma') {
87+
isKarmaRunnerConfigured = true;
88+
} else {
89+
context.logger.warn(
90+
`The "test" target is configured to use a runner other than "karma" in the main options.` +
91+
' The generated "karma.conf.js" file may not be used.',
92+
);
93+
}
94+
}
7695

77-
// If scoped project (i.e. "@foo/bar"), convert dir to "foo/bar".
78-
let folderName = options.project.startsWith('@') ? options.project.slice(1) : options.project;
79-
if (/[A-Z]/.test(folderName)) {
80-
folderName = strings.dasherize(folderName);
81-
}
96+
for (const [name, config] of Object.entries(testTarget.configurations ?? {})) {
97+
if (config && typeof config === 'object' && 'runner' in config) {
98+
if (config.runner !== 'karma') {
99+
context.logger.warn(
100+
`The "test" target's "${name}" configuration is configured to use a runner other than "karma".` +
101+
' The generated "karma.conf.js" file may not be used for that configuration.',
102+
);
103+
} else {
104+
isKarmaRunnerConfigured = true;
105+
}
106+
}
107+
}
82108

83-
return mergeWith(
84-
apply(url('./files'), [
85-
filter((p) => p.endsWith('karma.conf.js.template')),
86-
applyTemplates({
87-
relativePathToWorkspaceRoot: relativePathToWorkspaceRoot(project.root),
88-
folderName,
89-
needDevkitPlugin: testTarget.builder === AngularBuilder.Karma,
90-
}),
91-
move(project.root),
92-
]),
93-
);
94-
});
109+
if (!isKarmaRunnerConfigured) {
110+
context.logger.warn(
111+
`The "test" target is not explicitly configured to use the "karma" runner.` +
112+
' The generated "karma.conf.js" file may not be used as the default runner is "vitest".',
113+
);
114+
}
115+
}
116+
// If scoped project (i.e. "@foo/bar"), convert dir to "foo/bar".
117+
let folderName = options.project.startsWith('@') ? options.project.slice(1) : options.project;
118+
if (/[A-Z]/.test(folderName)) {
119+
folderName = strings.dasherize(folderName);
120+
}
121+
122+
return mergeWith(
123+
apply(url('./files'), [
124+
filter((p) => p.endsWith('karma.conf.js.template')),
125+
applyTemplates({
126+
relativePathToWorkspaceRoot: relativePathToWorkspaceRoot(project.root),
127+
folderName,
128+
needDevkitPlugin: testTarget.builder === AngularBuilder.Karma,
129+
}),
130+
move(project.root),
131+
]),
132+
);
133+
});
95134
}

‎packages/schematics/angular/config/index_spec.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,88 @@ describe('Config Schematic', () => {
9595
const { karmaConfig } = prj.architect.test.options;
9696
expect(karmaConfig).toBe('projects/foo/karma.conf.js');
9797
});
98+
99+
describe('with "unit-test" builder', () => {
100+
beforeEach(() => {
101+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
102+
const angularJson = applicationTree.readJson('angular.json') as any;
103+
angularJson.projects.foo.architect.test.builder = '@angular/build:unit-test';
104+
applicationTree.overwrite('angular.json', JSON.stringify(angularJson));
105+
});
106+
107+
it(`should not set 'karmaConfig' in test builder`, async () => {
108+
const tree = await runConfigSchematic(ConfigType.Karma);
109+
const config = JSON.parse(tree.readContent('/angular.json'));
110+
const prj = config.projects.foo;
111+
const { karmaConfig } = prj.architect.test.options;
112+
expect(karmaConfig).toBeUndefined();
113+
});
114+
115+
it(`should warn when 'runner' is not specified`, async () => {
116+
const logs: string[] = [];
117+
schematicRunner.logger.subscribe(({ message }) => logs.push(message));
118+
await runConfigSchematic(ConfigType.Karma);
119+
expect(
120+
logs.some((v) => v.includes('may not be used as the default runner is "vitest"')),
121+
).toBeTrue();
122+
});
123+
124+
it(`should warn when 'runner' is 'vitest' in options`, async () => {
125+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
126+
const angularJson = applicationTree.readJson('angular.json') as any;
127+
angularJson.projects.foo.architect.test.options ??= {};
128+
angularJson.projects.foo.architect.test.options.runner = 'vitest';
129+
applicationTree.overwrite('angular.json', JSON.stringify(angularJson));
130+
131+
const logs: string[] = [];
132+
schematicRunner.logger.subscribe(({ message }) => logs.push(message));
133+
await runConfigSchematic(ConfigType.Karma);
134+
expect(
135+
logs.some((v) => v.includes('runner other than "karma" in the main options')),
136+
).toBeTrue();
137+
});
138+
139+
it(`should warn when 'runner' is 'vitest' in a configuration`, async () => {
140+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
141+
const angularJson = applicationTree.readJson('angular.json') as any;
142+
angularJson.projects.foo.architect.test.configurations ??= {};
143+
angularJson.projects.foo.architect.test.configurations.ci = { runner: 'vitest' };
144+
applicationTree.overwrite('angular.json', JSON.stringify(angularJson));
145+
146+
const logs: string[] = [];
147+
schematicRunner.logger.subscribe(({ message }) => logs.push(message));
148+
await runConfigSchematic(ConfigType.Karma);
149+
expect(
150+
logs.some((v) => v.includes(`"ci" configuration is configured to use a runner`)),
151+
).toBeTrue();
152+
});
153+
154+
it(`should not warn when 'runner' is 'karma' in options`, async () => {
155+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
156+
const angularJson = applicationTree.readJson('angular.json') as any;
157+
angularJson.projects.foo.architect.test.options ??= {};
158+
angularJson.projects.foo.architect.test.options.runner = 'karma';
159+
applicationTree.overwrite('angular.json', JSON.stringify(angularJson));
160+
161+
const logs: string[] = [];
162+
schematicRunner.logger.subscribe(({ message }) => logs.push(message));
163+
await runConfigSchematic(ConfigType.Karma);
164+
expect(logs.length).toBe(0);
165+
});
166+
167+
it(`should not warn when 'runner' is 'karma' in a configuration`, async () => {
168+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
169+
const angularJson = applicationTree.readJson('angular.json') as any;
170+
angularJson.projects.foo.architect.test.configurations ??= {};
171+
angularJson.projects.foo.architect.test.configurations.ci = { runner: 'karma' };
172+
applicationTree.overwrite('angular.json', JSON.stringify(angularJson));
173+
174+
const logs: string[] = [];
175+
schematicRunner.logger.subscribe(({ message }) => logs.push(message));
176+
await runConfigSchematic(ConfigType.Karma);
177+
expect(logs.length).toBe(0);
178+
});
179+
});
98180
});
99181

100182
describe(`when 'type' is 'browserslist'`, () => {

0 commit comments

Comments
 (0)