From 9d7613db9bf8b397d5896fcdf6c7b0efeaffa5d5 Mon Sep 17 00:00:00 2001 From: Jan Martin Date: Fri, 27 Sep 2024 12:45:39 -0700 Subject: [PATCH] fix(@angular-devkit/build-angular): zone.js/testing + karma + esbuild 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. --- .../src/builders/karma/application_builder.ts | 46 ++++------- .../karma/tests/behavior/fake-async_spec.ts | 77 +++++++++++++++++++ 2 files changed, 93 insertions(+), 30 deletions(-) create mode 100644 packages/angular_devkit/build_angular/src/builders/karma/tests/behavior/fake-async_spec.ts diff --git a/packages/angular_devkit/build_angular/src/builders/karma/application_builder.ts b/packages/angular_devkit/build_angular/src/builders/karma/application_builder.ts index 3661cfc256ef..2e6b487ab689 100644 --- a/packages/angular_devkit/build_angular/src/builders/karma/application_builder.ts +++ b/packages/angular_devkit/build_angular/src/builders/karma/application_builder.ts @@ -85,11 +85,19 @@ async function getProjectSourceRoot(context: BuilderContext): Promise { 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[]]> { +): Promise> { // Glob for files to test. const testFiles = await findTests( options.include ?? [], @@ -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( @@ -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 }), @@ -162,7 +165,7 @@ async function initializeApplication( }, instrumentForCoverage, styles: options.styles, - polyfills, + polyfills: normalizePolyfills(options.polyfills), webWorkerTsConfig: options.webWorkerTsConfig, }, context, @@ -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' }, ); @@ -266,22 +268,6 @@ export async function writeTestFiles(files: Record, 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(generator: AsyncIterable): Promise { for await (const value of generator) { diff --git a/packages/angular_devkit/build_angular/src/builders/karma/tests/behavior/fake-async_spec.ts b/packages/angular_devkit/build_angular/src/builders/karma/tests/behavior/fake-async_spec.ts new file mode 100644 index 000000000000..a6cde25eb435 --- /dev/null +++ b/packages/angular_devkit/build_angular/src/builders/karma/tests/behavior/fake-async_spec.ts @@ -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: '', + }) + 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(); + }); + }); +});