Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): zone.js/testing + karma + esbuild
Browse files Browse the repository at this point in the history
Previously, the testing module was split into its own entrypoint but
then never loaded. Now it's just left in the overall polyfill bundle.

The bug wasn't caught by the existing test coverage, so this adds a
new test that ensures that fakeAsync works.

Cleaning up the Karma `files` list also removes the noisy "no file
matched the pattern worker-*.js" warnings that were previously generated
for test suites that don't include web worker sources.
  • Loading branch information
jkrems committed Sep 27, 2024
1 parent 87a90af commit 9d7613d
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,19 @@ async function getProjectSourceRoot(context: BuilderContext): Promise<string> {
return path.join(context.workspaceRoot, sourceRoot);
}

function normalizePolyfills(polyfills: string | string[] | undefined): string[] {
if (typeof polyfills === 'string') {
return [polyfills];
}

return polyfills ?? [];
}

async function collectEntrypoints(
options: KarmaBuilderOptions,
context: BuilderContext,
projectSourceRoot: string,
): Promise<[Set<string>, string[]]> {
): Promise<Set<string>> {
// Glob for files to test.
const testFiles = await findTests(
options.include ?? [],
Expand All @@ -102,13 +110,8 @@ async function collectEntrypoints(
...testFiles,
'@angular-devkit/build-angular/src/builders/karma/init_test_bed.js',
]);
// Extract `zone.js/testing` to a separate entry point because it needs to be loaded after Jasmine.
const [polyfills, hasZoneTesting] = extractZoneTesting(options.polyfills);
if (hasZoneTesting) {
entryPoints.add('zone.js/testing');
}

return [entryPoints, polyfills];
return entryPoints;
}

async function initializeApplication(
Expand All @@ -129,7 +132,7 @@ async function initializeApplication(
const testDir = path.join(context.workspaceRoot, 'dist/test-out', randomUUID());
const projectSourceRoot = await getProjectSourceRoot(context);

const [karma, [entryPoints, polyfills]] = await Promise.all([
const [karma, entryPoints] = await Promise.all([
import('karma'),
collectEntrypoints(options, context, projectSourceRoot),
fs.rm(testDir, { recursive: true, force: true }),
Expand Down Expand Up @@ -162,7 +165,7 @@ async function initializeApplication(
},
instrumentForCoverage,
styles: options.styles,
polyfills,
polyfills: normalizePolyfills(options.polyfills),
webWorkerTsConfig: options.webWorkerTsConfig,
},
context,
Expand All @@ -184,11 +187,10 @@ async function initializeApplication(
// Serve polyfills first.
{ pattern: `${testDir}/polyfills.js`, type: 'module' },
// Allow loading of chunk-* files but don't include them all on load.
{ pattern: `${testDir}/chunk-*.js`, type: 'module', included: false },
// Allow loading of worker-* files but don't include them all on load.
{ pattern: `${testDir}/worker-*.js`, type: 'module', included: false },
// `zone.js/testing`, served but not included on page load.
{ pattern: `${testDir}/testing.js`, type: 'module', included: false },
{ pattern: `${testDir}/{chunk,worker}-*.js`, type: 'module', included: false },
);

karmaOptions.files.push(
// Serve remaining JS on page load, these are the test entrypoints.
{ pattern: `${testDir}/*.js`, type: 'module' },
);
Expand Down Expand Up @@ -266,22 +268,6 @@ export async function writeTestFiles(files: Record<string, ResultFile>, testDir:
});
}

function extractZoneTesting(
polyfills: readonly string[] | string | undefined,
): [polyfills: string[], hasZoneTesting: boolean] {
if (typeof polyfills === 'string') {
polyfills = [polyfills];
}
polyfills ??= [];

const polyfillsWithoutZoneTesting = polyfills.filter(
(polyfill) => polyfill !== 'zone.js/testing',
);
const hasZoneTesting = polyfills.length !== polyfillsWithoutZoneTesting.length;

return [polyfillsWithoutZoneTesting, hasZoneTesting];
}

/** Returns the first item yielded by the given generator and cancels the execution. */
async function first<T>(generator: AsyncIterable<T>): Promise<T> {
for await (const value of generator) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/**
* @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.dev/license
*/

import { execute } from '../../index';
import { BASE_OPTIONS, KARMA_BUILDER_INFO, describeKarmaBuilder } from '../setup';

describeKarmaBuilder(execute, KARMA_BUILDER_INFO, (harness, setupTarget, isApp) => {
describe('Behavior: "fakeAsync"', () => {
beforeEach(async () => {
await setupTarget(harness);
});

it('loads zone.js/testing at the right time', async () => {
await harness.writeFiles({
'./src/app/app.component.ts': `
import { Component } from '@angular/core';
@Component({
selector: 'app-root',
template: '<button (click)="changeMessage()" class="change">{{ message }}</button>',
})
export class AppComponent {
message = 'Initial';
changeMessage() {
setTimeout(() => {
this.message = 'Changed';
}, 1000);
}
}`,
'./src/app/app.component.spec.ts': `
import { TestBed, fakeAsync, tick } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { AppComponent } from './app.component';
describe('AppComponent', () => {
beforeEach(() => TestBed.configureTestingModule({
declarations: [AppComponent]
}));
it('allows terrible things that break the most basic assumptions', fakeAsync(() => {
const fixture = TestBed.createComponent(AppComponent);
const btn = fixture.debugElement
.query(By.css('button.change'));
fixture.detectChanges();
expect(btn.nativeElement.innerText).toBe('Initial');
btn.triggerEventHandler('click', null);
// Pre-tick: Still the old value.
fixture.detectChanges();
expect(btn.nativeElement.innerText).toBe('Initial');
tick(1500);
fixture.detectChanges();
expect(btn.nativeElement.innerText).toBe('Changed');
}));
});`,
});

harness.useTarget('test', {
...BASE_OPTIONS,
});

const { result } = await harness.executeOnce();
expect(result?.success).toBeTrue();
});
});
});

0 comments on commit 9d7613d

Please sign in to comment.