From 873229f24be292ac9e909993e28c2ae2cef4033a Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 13 Jun 2021 10:32:57 +0200 Subject: [PATCH] feat(core): add opt-in test module teardown configuration (#42566) We currently have two long-standing issues related to how `TestBed` tests are torn down: 1. The dynamically-created test module isn't going to be destroyed, preventing the `ngOnDestroy` hooks on providers from running and keeping the component `style` nodes in the DOM. 2. The test root elements aren't going to be removed from the DOM. Instead, they will be removed whenever another test component is created. By themselves, these issues are easy to resolve, but given how long they've been around, there are a lot of unit tests out there that depend on the broken behavior. These changes address the issues by introducing APIs that allow users to opt into the correct test teardown behavior either at the application level via `TestBed.initTestEnvironment` or the test suite level via `TestBed.configureTestingModule`. At the moment, the new teardown behavior is opt-in, but the idea is that we'll eventually make it opt-out before removing the configuration altogether. Fixes #18831. PR Close #42566 --- goldens/public-api/core/testing/testing.d.ts | 16 ++ .../acceptance/view_container_ref_spec.ts | 5 + packages/core/test/test_bed_spec.ts | 199 +++++++++++++++++- packages/core/testing/src/r3_test_bed.ts | 111 +++++++++- packages/core/testing/src/test_bed.ts | 143 +++++++++++-- packages/core/testing/src/test_bed_common.ts | 31 +++ .../src/{before_each.ts => test_hooks.ts} | 24 ++- packages/core/testing/src/testing.ts | 4 +- .../src/dom_test_component_renderer.ts | 5 +- 9 files changed, 502 insertions(+), 36 deletions(-) rename packages/core/testing/src/{before_each.ts => test_hooks.ts} (61%) diff --git a/goldens/public-api/core/testing/testing.d.ts b/goldens/public-api/core/testing/testing.d.ts index 1ef670be54..350814d6c5 100644 --- a/goldens/public-api/core/testing/testing.d.ts +++ b/goldens/public-api/core/testing/testing.d.ts @@ -49,6 +49,11 @@ export declare type MetadataOverride = { set?: Partial; }; +export declare interface ModuleTeardownOptions { + destroyAfterEach: boolean; + rethrowErrors?: boolean; +} + export declare function resetFakeAsyncZone(): void; export declare interface TestBed { @@ -64,6 +69,7 @@ export declare interface TestBed { execute(tokens: any[], fn: Function, context?: any): any; /** @deprecated */ get(token: ProviderToken, notFoundValue?: T, flags?: InjectFlags): any; /** @deprecated */ get(token: any, notFoundValue?: any): any; + initTestEnvironment(ngModule: Type | Type[], platform: PlatformRef, options?: TestEnvironmentOptions): void; initTestEnvironment(ngModule: Type | Type[], platform: PlatformRef, aotSummaries?: () => any[]): void; inject(token: ProviderToken, notFoundValue?: T, flags?: InjectFlags): T; inject(token: ProviderToken, notFoundValue: null, flags?: InjectFlags): T | null; @@ -101,6 +107,9 @@ export declare interface TestBedStatic { createComponent(component: Type): ComponentFixture; /** @deprecated */ get(token: ProviderToken, notFoundValue?: T, flags?: InjectFlags): any; /** @deprecated */ get(token: any, notFoundValue?: any): any; + initTestEnvironment(ngModule: Type | Type[], platform: PlatformRef, options?: { + teardown?: ModuleTeardownOptions; + }): TestBed; initTestEnvironment(ngModule: Type | Type[], platform: PlatformRef, aotSummaries?: () => any[]): TestBed; inject(token: ProviderToken, notFoundValue?: T, flags?: InjectFlags): T; inject(token: ProviderToken, notFoundValue: null, flags?: InjectFlags): T | null; @@ -128,6 +137,12 @@ export declare interface TestBedStatic { export declare class TestComponentRenderer { insertRootElement(rootElementId: string): void; + removeAllRootElements?(): void; +} + +export declare interface TestEnvironmentOptions { + aotSummaries?: () => any[]; + teardown?: ModuleTeardownOptions; } export declare type TestModuleMetadata = { @@ -136,6 +151,7 @@ export declare type TestModuleMetadata = { imports?: any[]; schemas?: Array; aotSummaries?: () => any[]; + teardown?: ModuleTeardownOptions; }; export declare function tick(millis?: number, tickOptions?: { diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index bc97b91aa2..16b1940b6b 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -2278,6 +2278,11 @@ describe('ViewContainerRef', () => { containerEl = document.createElement('div'); document.body.appendChild(containerEl); containerEl!.appendChild(rootEl); + }, + removeAllRootElements() { + if (containerEl) { + containerEl.parentNode?.removeChild(containerEl); + } } }; } diff --git a/packages/core/test/test_bed_spec.ts b/packages/core/test/test_bed_spec.ts index 3031801161..6d360229a0 100644 --- a/packages/core/test/test_bed_spec.ts +++ b/packages/core/test/test_bed_spec.ts @@ -7,10 +7,11 @@ */ import {APP_INITIALIZER, ChangeDetectorRef, Compiler, Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, Optional, Pipe, Type, ViewChild, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵdefineInjector as defineInjector, ɵɵdefineNgModule as defineNgModule, ɵɵsetNgModuleScope as setNgModuleScope, ɵɵtext as text} from '@angular/core'; -import {getTestBed, TestBed} from '@angular/core/testing/src/test_bed'; +import {getTestBed, TestBed, TestBedViewEngine} from '@angular/core/testing/src/test_bed'; import {By} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {onlyInIvy} from '@angular/private/testing'; +import {TestBedRender3} from '../testing/src/r3_test_bed'; const NAME = new InjectionToken('name'); @@ -1284,3 +1285,199 @@ describe('TestBed', () => { expect(fixture!.nativeElement.textContent).toContain('changed'); }); }); + + +describe('TestBed module teardown', () => { + // Cast the `TestBed` to the internal data type since we're testing private APIs. + let TestBed: TestBedRender3|TestBedViewEngine; + + beforeEach(() => { + TestBed = getTestBed() as unknown as (TestBedRender3 | TestBedViewEngine); + TestBed.resetTestingModule(); + }); + + it('should not tear down the test module by default', () => { + expect(TestBed.shouldTearDownTestingModule()).toBe(false); + }); + + it('should be able to configure the teardown behavior', () => { + TestBed.configureTestingModule({teardown: {destroyAfterEach: true}}); + expect(TestBed.shouldTearDownTestingModule()).toBe(true); + }); + + it('should reset the teardown behavior back to the default when TestBed is reset', () => { + TestBed.configureTestingModule({teardown: {destroyAfterEach: true}}); + expect(TestBed.shouldTearDownTestingModule()).toBe(true); + TestBed.resetTestingModule(); + expect(TestBed.shouldTearDownTestingModule()).toBe(false); + }); + + it('should destroy test module providers when test module teardown is enabled', () => { + SimpleService.ngOnDestroyCalls = 0; + TestBed.configureTestingModule({ + providers: [SimpleService], + declarations: [GreetingCmp], + teardown: {destroyAfterEach: true} + }); + TestBed.createComponent(GreetingCmp); + + expect(SimpleService.ngOnDestroyCalls).toBe(0); + TestBed.resetTestingModule(); + expect(SimpleService.ngOnDestroyCalls).toBe(1); + }); + + it('should remove the fixture root element from the DOM when module teardown is enabled', () => { + TestBed.configureTestingModule({ + declarations: [SimpleCmp], + teardown: {destroyAfterEach: true}, + }); + const fixture = TestBed.createComponent(SimpleCmp); + const fixtureDocument = fixture.nativeElement.ownerDocument; + + expect(fixtureDocument.body.contains(fixture.nativeElement)).toBe(true); + TestBed.resetTestingModule(); + expect(fixtureDocument.body.contains(fixture.nativeElement)).toBe(false); + }); + + it('should re-throw errors that were thrown during fixture cleanup', () => { + @Component({template: ''}) + class ThrowsOnDestroy { + ngOnDestroy() { + throw Error('oh no'); + } + } + + TestBed.configureTestingModule({ + declarations: [ThrowsOnDestroy], + teardown: {destroyAfterEach: true}, + }); + TestBed.createComponent(ThrowsOnDestroy); + + const spy = spyOn(console, 'error'); + expect(() => TestBed.resetTestingModule()) + .toThrowError('1 component threw errors during cleanup'); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('should not interrupt fixture destruction if an error is thrown', () => { + @Component({template: ''}) + class ThrowsOnDestroy { + ngOnDestroy() { + throw Error('oh no'); + } + } + + TestBed.configureTestingModule({ + declarations: [ThrowsOnDestroy], + teardown: {destroyAfterEach: true}, + }); + + for (let i = 0; i < 3; i++) { + TestBed.createComponent(ThrowsOnDestroy); + } + + const spy = spyOn(console, 'error'); + expect(() => TestBed.resetTestingModule()) + .toThrowError('3 components threw errors during cleanup'); + expect(spy).toHaveBeenCalledTimes(3); + }); + + it('should re-throw errors that were thrown during module teardown by default', () => { + @Injectable() + class ThrowsOnDestroy { + ngOnDestroy() { + throw Error('oh no'); + } + } + + @Component({template: ''}) + class App { + constructor(_service: ThrowsOnDestroy) {} + } + + TestBed.configureTestingModule({ + providers: [ThrowsOnDestroy], + declarations: [App], + teardown: {destroyAfterEach: true}, + }); + TestBed.createComponent(App); + + expect(() => TestBed.resetTestingModule()).toThrowError('oh no'); + }); + + it('should be able to opt out of rethrowing of errors coming from module teardown', () => { + @Injectable() + class ThrowsOnDestroy { + ngOnDestroy() { + throw Error('oh no'); + } + } + + @Component({template: ''}) + class App { + constructor(_service: ThrowsOnDestroy) {} + } + + TestBed.configureTestingModule({ + providers: [ThrowsOnDestroy], + declarations: [App], + teardown: {destroyAfterEach: true, rethrowErrors: false}, + }); + TestBed.createComponent(App); + + const spy = spyOn(console, 'error'); + expect(() => TestBed.resetTestingModule()).not.toThrow(); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('should remove the styles associated with a test component when the test module is torn down', + () => { + @Component({ + template: 'Hello', + styles: [`span {color: hotpink;}`], + }) + class StyledComp1 { + } + + @Component({ + template: '
Hello
', + styles: [`div {color: red;}`], + }) + class StyledComp2 { + } + + TestBed.configureTestingModule({ + declarations: [StyledComp1, StyledComp2], + teardown: {destroyAfterEach: true}, + }); + + const fixtures = [ + TestBed.createComponent(StyledComp1), + TestBed.createComponent(StyledComp2), + ]; + const fixtureDocument = fixtures[0].nativeElement.ownerDocument; + const styleCountBefore = fixtureDocument.querySelectorAll('style').length; + + // Note that we can only assert that the behavior works as expected by checking that the + // number of stylesheets has decreased. We can't expect that they'll be zero, because there + // may by stylesheets leaking in from other tests that don't use the module teardown + // behavior. + expect(styleCountBefore).toBeGreaterThan(0); + TestBed.resetTestingModule(); + expect(fixtureDocument.querySelectorAll('style').length).toBeLessThan(styleCountBefore); + }); + + + it('should remove the fixture root element from the DOM when module teardown is enabled', () => { + TestBed.configureTestingModule({ + declarations: [SimpleCmp], + teardown: {destroyAfterEach: true}, + }); + const fixture = TestBed.createComponent(SimpleCmp); + const fixtureDocument = fixture.nativeElement.ownerDocument; + + expect(fixtureDocument.body.contains(fixture.nativeElement)).toBe(true); + TestBed.resetTestingModule(); + expect(fixtureDocument.body.contains(fixture.nativeElement)).toBe(false); + }); +}); diff --git a/packages/core/testing/src/r3_test_bed.ts b/packages/core/testing/src/r3_test_bed.ts index f7d6487a26..8cc09a541a 100644 --- a/packages/core/testing/src/r3_test_bed.ts +++ b/packages/core/testing/src/r3_test_bed.ts @@ -36,7 +36,7 @@ import {ComponentFixture} from './component_fixture'; import {MetadataOverride} from './metadata_override'; import {R3TestBedCompiler} from './r3_test_bed_compiler'; import {TestBed} from './test_bed'; -import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, TestBedStatic, TestComponentRenderer, TestModuleMetadata} from './test_bed_common'; +import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, ModuleTeardownOptions, TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, TestBedStatic, TestComponentRenderer, TestEnvironmentOptions, TestModuleMetadata} from './test_bed_common'; let _nextRootElementId = 0; @@ -52,6 +52,18 @@ let _nextRootElementId = 0; * according to the compiler used. */ export class TestBedRender3 implements TestBed { + /** + * Teardown options that have been configured at the environment level. + * Used as a fallback if no instance-level options have been provided. + */ + private static _environmentTeardownOptions: ModuleTeardownOptions|undefined; + + /** + * Teardown options that have been configured at the `TestBed` instance level. + * These options take precedence over the environemnt-level ones. + */ + private _instanceTeardownOptions: ModuleTeardownOptions|undefined; + /** * Initialize the environment for testing with a compiler factory, a PlatformRef, and an * angular module. These are common to every test in the suite. @@ -66,9 +78,10 @@ export class TestBedRender3 implements TestBed { * @publicApi */ static initTestEnvironment( - ngModule: Type|Type[], platform: PlatformRef, aotSummaries?: () => any[]): TestBed { + ngModule: Type|Type[], platform: PlatformRef, + summariesOrOptions?: TestEnvironmentOptions|(() => any[])): TestBed { const testBed = _getTestBedRender3(); - testBed.initTestEnvironment(ngModule, platform, aotSummaries); + testBed.initTestEnvironment(ngModule, platform, summariesOrOptions); return testBed; } @@ -182,6 +195,14 @@ export class TestBedRender3 implements TestBed { return TestBedRender3 as any as TestBedStatic; } + static shouldTearDownTestingModule(): boolean { + return _getTestBedRender3().shouldTearDownTestingModule(); + } + + static tearDownTestingModule(): void { + _getTestBedRender3().tearDownTestingModule(); + } + // Properties platform: PlatformRef = null!; @@ -206,11 +227,18 @@ export class TestBedRender3 implements TestBed { * * @publicApi */ - initTestEnvironment( - ngModule: Type|Type[], platform: PlatformRef, aotSummaries?: () => any[]): void { + initTestEnvironment(ngModule: Type|Type[], platform: PlatformRef, summariesOrOptions?: { + teardown?: ModuleTeardownOptions + }|(() => any[])): void { if (this.platform || this.ngModule) { throw new Error('Cannot set base providers because it has already been called'); } + + // If `summariesOrOptions` is a function, it means that it's + // an AOT summaries factory which Ivy doesn't support. + TestBedRender3._environmentTeardownOptions = + typeof summariesOrOptions === 'function' ? undefined : summariesOrOptions?.teardown; + this.platform = platform; this.ngModule = ngModule; this._compiler = new R3TestBedCompiler(this.platform, this.ngModule); @@ -226,6 +254,7 @@ export class TestBedRender3 implements TestBed { this._compiler = null; this.platform = null!; this.ngModule = null!; + TestBedRender3._environmentTeardownOptions = undefined; } resetTestingModule(): void { @@ -235,8 +264,22 @@ export class TestBedRender3 implements TestBed { this.compiler.restoreOriginalState(); } this._compiler = new R3TestBedCompiler(this.platform, this.ngModule); - this._testModuleRef = null; - this.destroyActiveFixtures(); + + // We have to chain a couple of try/finally blocks, because each step can + // throw errors and we don't want it to interrupt the next step and we also + // want an error to be thrown at the end. + try { + this.destroyActiveFixtures(); + } finally { + try { + if (this.shouldTearDownTestingModule()) { + this.tearDownTestingModule(); + } + } finally { + this._testModuleRef = null; + this._instanceTeardownOptions = undefined; + } + } } configureCompiler(config: {providers?: any[]; useJit?: boolean;}): void { @@ -251,6 +294,9 @@ export class TestBedRender3 implements TestBed { configureTestingModule(moduleDef: TestModuleMetadata): void { this.assertNotInstantiated('R3TestBed.configureTestingModule', 'configure the test module'); + // Always re-assign the teardown options, even if they're undefined. + // This ensures that we don't carry the options between tests. + this._instanceTeardownOptions = moduleDef.teardown; this.compiler.configureTestingModule(moduleDef); } @@ -402,10 +448,12 @@ export class TestBedRender3 implements TestBed { } private destroyActiveFixtures(): void { + let errorCount = 0; this._activeFixtures.forEach((fixture) => { try { fixture.destroy(); } catch (e) { + errorCount++; console.error('Error during cleanup of component', { component: fixture.componentInstance, stacktrace: e, @@ -413,6 +461,55 @@ export class TestBedRender3 implements TestBed { } }); this._activeFixtures = []; + + if (errorCount > 0 && this.shouldRethrowTeardownErrors()) { + throw Error( + `${errorCount} ${(errorCount === 1 ? 'component' : 'components')} ` + + `threw errors during cleanup`); + } + } + + private shouldRethrowTeardownErrors() { + const instanceOptions = this._instanceTeardownOptions; + const environmentOptions = TestBedRender3._environmentTeardownOptions; + + // If the new teardown behavior hasn't been configured, preserve the old behavior. + if (!instanceOptions && !environmentOptions) { + return false; + } + + // Otherwise use the configured behavior or default to rethrowing. + return instanceOptions?.rethrowErrors ?? environmentOptions?.rethrowErrors ?? true; + } + + shouldTearDownTestingModule(): boolean { + return this._instanceTeardownOptions?.destroyAfterEach ?? + TestBedRender3._environmentTeardownOptions?.destroyAfterEach ?? + TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT; + } + + tearDownTestingModule() { + // If the module ref has already been destroyed, we won't be able to get a test renderer. + if (this._testModuleRef === null) { + return; + } + // Resolve the renderer ahead of time, because we want to remove the root elements as the very + // last step, but the injector will be destroyed as a part of the module ref destruction. + const testRenderer = this.inject(TestComponentRenderer); + try { + this._testModuleRef.destroy(); + } catch (e) { + if (this.shouldRethrowTeardownErrors()) { + throw e; + } else { + console.error('Error during cleanup of a testing module', { + component: this._testModuleRef.instance, + stacktrace: e, + }); + } + } finally { + testRenderer.removeAllRootElements?.(); + } } } diff --git a/packages/core/testing/src/test_bed.ts b/packages/core/testing/src/test_bed.ts index 9859681c07..627489c3e0 100644 --- a/packages/core/testing/src/test_bed.ts +++ b/packages/core/testing/src/test_bed.ts @@ -11,7 +11,7 @@ import {ApplicationInitStatus, CompilerOptions, Component, Directive, InjectFlag import {ComponentFixture} from './component_fixture'; import {MetadataOverride} from './metadata_override'; import {_getTestBedRender3, TestBedRender3} from './r3_test_bed'; -import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, TestBedStatic, TestComponentRenderer, TestModuleMetadata} from './test_bed_common'; +import {ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, ModuleTeardownOptions, TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT, TestBedStatic, TestComponentRenderer, TestEnvironmentOptions, TestModuleMetadata} from './test_bed_common'; import {TestingCompiler, TestingCompilerFactory} from './test_compiler'; @@ -36,6 +36,9 @@ export interface TestBed { * Test modules and platforms for individual platforms are available from * '@angular//testing'. */ + initTestEnvironment( + ngModule: Type|Type[], platform: PlatformRef, + options?: TestEnvironmentOptions): void; initTestEnvironment( ngModule: Type|Type[], platform: PlatformRef, aotSummaries?: () => any[]): void; @@ -97,6 +100,18 @@ export interface TestBed { * according to the compiler used. */ export class TestBedViewEngine implements TestBed { + /** + * Teardown options that have been configured at the environment level. + * Used as a fallback if no instance-level options have been provided. + */ + private static _environmentTeardownOptions: ModuleTeardownOptions|undefined; + + /** + * Teardown options that have been configured at the `TestBed` instance level. + * These options take precedence over the environemnt-level ones. + */ + private _instanceTeardownOptions: ModuleTeardownOptions|undefined; + /** * Initialize the environment for testing with a compiler factory, a PlatformRef, and an * angular module. These are common to every test in the suite. @@ -110,9 +125,9 @@ export class TestBedViewEngine implements TestBed { */ static initTestEnvironment( ngModule: Type|Type[], platform: PlatformRef, - aotSummaries?: () => any[]): TestBedViewEngine { + summariesOrOptions?: TestEnvironmentOptions|(() => any[])): TestBedViewEngine { const testBed = _getTestBedViewEngine(); - testBed.initTestEnvironment(ngModule, platform, aotSummaries); + testBed.initTestEnvironment(ngModule, platform, summariesOrOptions); return testBed; } @@ -236,10 +251,18 @@ export class TestBedViewEngine implements TestBed { return _getTestBedViewEngine().createComponent(component); } + static shouldTearDownTestingModule(): boolean { + return _getTestBedViewEngine().shouldTearDownTestingModule(); + } + + static tearDownTestingModule(): void { + _getTestBedViewEngine().tearDownTestingModule(); + } + private _instantiated: boolean = false; private _compiler: TestingCompiler = null!; - private _moduleRef: NgModuleRef = null!; + private _moduleRef: NgModuleRef|null = null; private _moduleFactory: NgModuleFactory = null!; private _compilerOptions: CompilerOptions[] = []; @@ -278,14 +301,19 @@ export class TestBedViewEngine implements TestBed { * '@angular//testing'. */ initTestEnvironment( - ngModule: Type|Type[], platform: PlatformRef, aotSummaries?: () => any[]): void { + ngModule: Type|Type[], platform: PlatformRef, + summariesOrOptions?: TestEnvironmentOptions|(() => any[])): void { if (this.platform || this.ngModule) { throw new Error('Cannot set base providers because it has already been called'); } this.platform = platform; this.ngModule = ngModule; - if (aotSummaries) { - this._testEnvAotSummaries = aotSummaries; + if (typeof summariesOrOptions === 'function') { + this._testEnvAotSummaries = summariesOrOptions; + TestBedViewEngine._environmentTeardownOptions = undefined; + } else { + this._testEnvAotSummaries = (summariesOrOptions?.aotSummaries) || (() => []); + TestBedViewEngine._environmentTeardownOptions = summariesOrOptions?.teardown; } } @@ -297,6 +325,7 @@ export class TestBedViewEngine implements TestBed { this.platform = null!; this.ngModule = null!; this._testEnvAotSummaries = () => []; + TestBedViewEngine._environmentTeardownOptions = undefined; } resetTestingModule(): void { @@ -312,25 +341,29 @@ export class TestBedViewEngine implements TestBed { this._isRoot = true; this._rootProviderOverrides = []; - this._moduleRef = null!; this._moduleFactory = null!; this._compilerOptions = []; this._providers = []; this._declarations = []; this._imports = []; this._schemas = []; - this._instantiated = false; - this._activeFixtures.forEach((fixture) => { + + // We have to chain a couple of try/finally blocks, because each step can + // throw errors and we don't want it to interrupt the next step and we also + // want an error to be thrown at the end. + try { + this.destroyActiveFixtures(); + } finally { try { - fixture.destroy(); - } catch (e) { - console.error('Error during cleanup of component', { - component: fixture.componentInstance, - stacktrace: e, - }); + if (this.shouldTearDownTestingModule()) { + this.tearDownTestingModule(); + } + } finally { + this._moduleRef = null; + this._instanceTeardownOptions = undefined; + this._instantiated = false; } - }); - this._activeFixtures = []; + } } configureCompiler(config: {providers?: any[], useJit?: boolean}): void { @@ -355,6 +388,9 @@ export class TestBedViewEngine implements TestBed { if (moduleDef.aotSummaries) { this._aotSummaries.push(moduleDef.aotSummaries); } + // Always re-assign the teardown options, even if they're undefined. + // This ensures that we don't carry the options between tests. + this._instanceTeardownOptions = moduleDef.teardown; } compileComponents(): Promise { @@ -470,7 +506,7 @@ export class TestBedViewEngine implements TestBed { // Tests can inject things from the ng module and from the compiler, // but the ng module can't inject things from the compiler and vice versa. const UNDEFINED = {}; - const result = this._moduleRef.injector.get(token, UNDEFINED, flags); + const result = this._moduleRef!.injector.get(token, UNDEFINED, flags); return result === UNDEFINED ? this._compiler.injector.get(token, notFoundValue, flags) as any : result; } @@ -602,7 +638,7 @@ export class TestBedViewEngine implements TestBed { const initComponent = () => { const componentRef = - componentFactory.create(Injector.NULL, [], `#${rootElId}`, this._moduleRef); + componentFactory.create(Injector.NULL, [], `#${rootElId}`, this._moduleRef!); return new ComponentFixture(componentRef, ngZone, autoDetect); }; @@ -610,6 +646,73 @@ export class TestBedViewEngine implements TestBed { this._activeFixtures.push(fixture); return fixture; } + + private destroyActiveFixtures(): void { + let errorCount = 0; + this._activeFixtures.forEach((fixture) => { + try { + fixture.destroy(); + } catch (e) { + errorCount++; + console.error('Error during cleanup of component', { + component: fixture.componentInstance, + stacktrace: e, + }); + } + }); + this._activeFixtures = []; + + if (errorCount > 0 && this.shouldRethrowTeardownErrors()) { + throw Error( + `${errorCount} ${(errorCount === 1 ? 'component' : 'components')} ` + + `threw errors during cleanup`); + } + } + + private shouldRethrowTeardownErrors() { + const instanceOptions = this._instanceTeardownOptions; + const environmentOptions = TestBedViewEngine._environmentTeardownOptions; + + // If the new teardown behavior hasn't been configured, preserve the old behavior. + if (!instanceOptions && !environmentOptions) { + return false; + } + + // Otherwise use the configured behavior or default to rethrowing. + return instanceOptions?.rethrowErrors ?? environmentOptions?.rethrowErrors ?? true; + } + + shouldTearDownTestingModule(): boolean { + return this._instanceTeardownOptions?.destroyAfterEach ?? + TestBedViewEngine._environmentTeardownOptions?.destroyAfterEach ?? + TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT; + } + + tearDownTestingModule() { + // If the module ref has already been destroyed, we won't be able to get a test renderer. + if (this._moduleRef === null) { + return; + } + + // Resolve the renderer ahead of time, because we want to remove the root elements as the very + // last step, but the injector will be destroyed as a part of the module ref destruction. + const testRenderer = this.inject(TestComponentRenderer); + try { + this._moduleRef.destroy(); + } catch (e) { + if (this._instanceTeardownOptions?.rethrowErrors ?? + TestBedViewEngine._environmentTeardownOptions?.rethrowErrors ?? true) { + throw e; + } else { + console.error('Error during cleanup of a testing module', { + component: this._moduleRef.instance, + stacktrace: e, + }); + } + } finally { + testRenderer?.removeAllRootElements?.(); + } + } } /** diff --git a/packages/core/testing/src/test_bed_common.ts b/packages/core/testing/src/test_bed_common.ts index 76163253aa..46ff0be28d 100644 --- a/packages/core/testing/src/test_bed_common.ts +++ b/packages/core/testing/src/test_bed_common.ts @@ -12,6 +12,12 @@ import {ComponentFixture} from './component_fixture'; import {MetadataOverride} from './metadata_override'; import {TestBed} from './test_bed'; +/** + * Whether test modules should be torn down by default. + * Currently disabled for backwards-compatibility reasons. + */ +export const TEARDOWN_TESTING_MODULE_ON_DESTROY_DEFAULT = false; + /** * An abstract class for inserting the root test component element in a platform independent way. * @@ -19,6 +25,7 @@ import {TestBed} from './test_bed'; */ export class TestComponentRenderer { insertRootElement(rootElementId: string) {} + removeAllRootElements?() {} } /** @@ -41,8 +48,29 @@ export type TestModuleMetadata = { imports?: any[], schemas?: Array, aotSummaries?: () => any[], + teardown?: ModuleTeardownOptions; }; +/** + * @publicApi + */ +export interface TestEnvironmentOptions { + aotSummaries?: () => any[]; + teardown?: ModuleTeardownOptions; +} + +/** + * Object used to configure the test module teardown behavior in `TestBed`. + * @publicApi + */ +export interface ModuleTeardownOptions { + /** Whether the test module should be destroyed after every test. */ + destroyAfterEach: boolean; + + /** Whether errors during test module destruction should be re-thrown. Defaults to `true`. */ + rethrowErrors?: boolean; +} + /** * Static methods implemented by the `TestBedViewEngine` and `TestBedRender3` * @@ -51,6 +79,9 @@ export type TestModuleMetadata = { export interface TestBedStatic { new(...args: any[]): TestBed; + initTestEnvironment(ngModule: Type|Type[], platform: PlatformRef, options?: { + teardown?: ModuleTeardownOptions + }): TestBed; initTestEnvironment( ngModule: Type|Type[], platform: PlatformRef, aotSummaries?: () => any[]): TestBed; diff --git a/packages/core/testing/src/before_each.ts b/packages/core/testing/src/test_hooks.ts similarity index 61% rename from packages/core/testing/src/before_each.ts rename to packages/core/testing/src/test_hooks.ts index fe9427c87f..c76c17e385 100644 --- a/packages/core/testing/src/before_each.ts +++ b/packages/core/testing/src/test_hooks.ts @@ -13,7 +13,7 @@ */ import {resetFakeAsyncZone} from './fake_async'; -import {TestBed} from './test_bed'; +import {TestBed, TestBedViewEngine as TestBedInternal} from './test_bed'; declare var global: any; @@ -21,10 +21,24 @@ const _global = (typeof window === 'undefined' ? global : window); // Reset the test providers and the fake async zone before each test. if (_global.beforeEach) { - _global.beforeEach(() => { - TestBed.resetTestingModule(); - resetFakeAsyncZone(); - }); + _global.beforeEach(getCleanupHook(false)); +} + +// We provide both a `beforeEach` and `afterEach`, because the updated behavior for +// tearing down the module is supposed to run after the test so that we can associate +// teardown errors with the correct test. +if (_global.afterEach) { + _global.afterEach(getCleanupHook(true)); +} + +function getCleanupHook(expectedTeardownValue: boolean) { + return () => { + if ((TestBed as unknown as TestBedInternal).shouldTearDownTestingModule() === + expectedTeardownValue) { + TestBed.resetTestingModule(); + resetFakeAsyncZone(); + } + }; } /** diff --git a/packages/core/testing/src/testing.ts b/packages/core/testing/src/testing.ts index 220cbc9d43..c8ec6baae9 100644 --- a/packages/core/testing/src/testing.ts +++ b/packages/core/testing/src/testing.ts @@ -16,8 +16,8 @@ export * from './async'; export * from './component_fixture'; export * from './fake_async'; export {TestBed, getTestBed, inject, InjectSetupWrapper, withModule} from './test_bed'; -export * from './test_bed_common'; -export * from './before_each'; +export {TestComponentRenderer, ComponentFixtureAutoDetect, ComponentFixtureNoNgZone, TestModuleMetadata, TestEnvironmentOptions, ModuleTeardownOptions, TestBedStatic} from './test_bed_common'; +export * from './test_hooks'; export * from './metadata_override'; export {MetadataOverrider as ɵMetadataOverrider} from './metadata_overrider'; export * from './private_export_testing'; diff --git a/packages/platform-browser-dynamic/testing/src/dom_test_component_renderer.ts b/packages/platform-browser-dynamic/testing/src/dom_test_component_renderer.ts index e57f9dd290..6f321b7d90 100644 --- a/packages/platform-browser-dynamic/testing/src/dom_test_component_renderer.ts +++ b/packages/platform-browser-dynamic/testing/src/dom_test_component_renderer.ts @@ -20,14 +20,17 @@ export class DOMTestComponentRenderer extends TestComponentRenderer { } insertRootElement(rootElId: string) { + this.removeAllRootElements(); const rootElement = getDOM().getDefaultDocument().createElement('div'); rootElement.setAttribute('id', rootElId); + this._doc.body.appendChild(rootElement); + } + removeAllRootElements() { // TODO(juliemr): can/should this be optional? const oldRoots = this._doc.querySelectorAll('[id^=root]'); for (let i = 0; i < oldRoots.length; i++) { getDOM().remove(oldRoots[i]); } - this._doc.body.appendChild(rootElement); } }