From d7c82f5c0f35953f80ee8bc6c055161707d4e363 Mon Sep 17 00:00:00 2001 From: Marc Laval Date: Thu, 25 Aug 2016 23:37:46 +0200 Subject: [PATCH] test: fix memory leak when running test campaign (#11072) --- .../core/testing/component_fixture.ts | 36 ++++---- modules/@angular/core/testing/test_bed.ts | 8 +- .../abstract_form_group_directive.ts | 8 +- .../reactive_directives/form_control_name.ts | 9 +- .../reactive_directives/form_group_name.ts | 10 ++- .../forms/test/template_integration_spec.ts | 88 ++++++++++--------- 6 files changed, 92 insertions(+), 67 deletions(-) diff --git a/modules/@angular/core/testing/component_fixture.ts b/modules/@angular/core/testing/component_fixture.ts index 9ebadc3701..d926d6d502 100644 --- a/modules/@angular/core/testing/component_fixture.ts +++ b/modules/@angular/core/testing/component_fixture.ts @@ -57,6 +57,7 @@ export class ComponentFixture { private _autoDetect: boolean; private _isStable: boolean = true; + private _isDestroyed: boolean = false; private _resolve: (result: any) => void; private _promise: Promise = null; private _onUnstableSubscription: any /** TODO #9100 */ = null; @@ -178,22 +179,25 @@ export class ComponentFixture { * Trigger component destruction. */ destroy(): void { - this.componentRef.destroy(); - if (this._onUnstableSubscription != null) { - this._onUnstableSubscription.unsubscribe(); - this._onUnstableSubscription = null; - } - if (this._onStableSubscription != null) { - this._onStableSubscription.unsubscribe(); - this._onStableSubscription = null; - } - if (this._onMicrotaskEmptySubscription != null) { - this._onMicrotaskEmptySubscription.unsubscribe(); - this._onMicrotaskEmptySubscription = null; - } - if (this._onErrorSubscription != null) { - this._onErrorSubscription.unsubscribe(); - this._onErrorSubscription = null; + if (!this._isDestroyed) { + this.componentRef.destroy(); + if (this._onUnstableSubscription != null) { + this._onUnstableSubscription.unsubscribe(); + this._onUnstableSubscription = null; + } + if (this._onStableSubscription != null) { + this._onStableSubscription.unsubscribe(); + this._onStableSubscription = null; + } + if (this._onMicrotaskEmptySubscription != null) { + this._onMicrotaskEmptySubscription.unsubscribe(); + this._onMicrotaskEmptySubscription = null; + } + if (this._onErrorSubscription != null) { + this._onErrorSubscription.unsubscribe(); + this._onErrorSubscription = null; + } + this._isDestroyed = true; } } } diff --git a/modules/@angular/core/testing/test_bed.ts b/modules/@angular/core/testing/test_bed.ts index 95715343e4..9169309191 100644 --- a/modules/@angular/core/testing/test_bed.ts +++ b/modules/@angular/core/testing/test_bed.ts @@ -11,6 +11,7 @@ import {ListWrapper} from '../src/facade/collection'; import {BaseException} from '../src/facade/exceptions'; import {FunctionWrapper, stringify} from '../src/facade/lang'; import {Type} from '../src/type'; + import {AsyncTestCompleter} from './async_test_completer'; import {ComponentFixture} from './component_fixture'; import {MetadataOverride} from './metadata_override'; @@ -155,6 +156,7 @@ export class TestBed implements Injector { private _declarations: Array|any[]|any> = []; private _imports: Array|any[]|any> = []; private _schemas: Array = []; + private _activeFixtures: ComponentFixture[] = []; /** * Initialize the environment for testing with a compiler factory, a PlatformRef, and an @@ -203,6 +205,8 @@ export class TestBed implements Injector { this._imports = []; this._schemas = []; this._instantiated = false; + this._activeFixtures.forEach((fixture) => fixture.destroy()); + this._activeFixtures = []; } platform: PlatformRef = null; @@ -355,7 +359,9 @@ export class TestBed implements Injector { return new ComponentFixture(componentRef, ngZone, autoDetect); }; - return ngZone == null ? initComponent() : ngZone.run(initComponent); + const fixture = ngZone == null ? initComponent() : ngZone.run(initComponent); + this._activeFixtures.push(fixture); + return fixture; } } diff --git a/modules/@angular/forms/src/directives/abstract_form_group_directive.ts b/modules/@angular/forms/src/directives/abstract_form_group_directive.ts index 085805a61f..19661ec147 100644 --- a/modules/@angular/forms/src/directives/abstract_form_group_directive.ts +++ b/modules/@angular/forms/src/directives/abstract_form_group_directive.ts @@ -37,7 +37,11 @@ export class AbstractFormGroupDirective extends ControlContainer implements OnIn this.formDirective.addFormGroup(this); } - ngOnDestroy(): void { this.formDirective.removeFormGroup(this); } + ngOnDestroy(): void { + if (this.formDirective) { + this.formDirective.removeFormGroup(this); + } + } /** * Get the {@link FormGroup} backing this binding. @@ -52,7 +56,7 @@ export class AbstractFormGroupDirective extends ControlContainer implements OnIn /** * Get the {@link Form} to which this group belongs. */ - get formDirective(): Form { return this._parent.formDirective; } + get formDirective(): Form { return this._parent ? this._parent.formDirective : null; } get validator(): ValidatorFn { return composeValidators(this._validators); } diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts b/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts index f4d6027b9c..53d7cc9629 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_control_name.ts @@ -105,7 +105,6 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { // TODO(kara): Replace ngModel with reactive API @Input('ngModel') model: any; @Output('ngModelChange') update = new EventEmitter(); - @Input('disabled') set disabled(isDisabled: boolean) { ReactiveErrors.disabledAttrWarning(); } @@ -133,7 +132,11 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { } } - ngOnDestroy(): void { this.formDirective.removeControl(this); } + ngOnDestroy(): void { + if (this.formDirective) { + this.formDirective.removeControl(this); + } + } viewToModelUpdate(newValue: any): void { this.viewModel = newValue; @@ -142,7 +145,7 @@ export class FormControlName extends NgControl implements OnChanges, OnDestroy { get path(): string[] { return controlPath(this.name, this._parent); } - get formDirective(): any { return this._parent.formDirective; } + get formDirective(): any { return this._parent ? this._parent.formDirective : null; } get validator(): ValidatorFn { return composeValidators(this._validators); } diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts b/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts index 0f86a71974..071d108979 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_group_name.ts @@ -161,11 +161,17 @@ export class FormArrayName extends ControlContainer implements OnInit, OnDestroy this.formDirective.addFormArray(this); } - ngOnDestroy(): void { this.formDirective.removeFormArray(this); } + ngOnDestroy(): void { + if (this.formDirective) { + this.formDirective.removeFormArray(this); + } + } get control(): FormArray { return this.formDirective.getFormArray(this); } - get formDirective(): FormGroupDirective { return this._parent.formDirective; } + get formDirective(): FormGroupDirective { + return this._parent ? this._parent.formDirective : null; + } get path(): string[] { return controlPath(this.name, this._parent); } diff --git a/modules/@angular/forms/test/template_integration_spec.ts b/modules/@angular/forms/test/template_integration_spec.ts index 4f205e9fbb..e50cfe81d1 100644 --- a/modules/@angular/forms/test/template_integration_spec.ts +++ b/modules/@angular/forms/test/template_integration_spec.ts @@ -135,61 +135,63 @@ export function main() { expect(form.value).toEqual({}); })); - it('should set status classes with ngModel', () => { - const fixture = TestBed.createComponent(NgModelForm); - fixture.debugElement.componentInstance.name = 'aa'; - fixture.detectChanges(); - fixture.whenStable().then(() => { - fixture.detectChanges(); + it('should set status classes with ngModel', async(() => { + const fixture = TestBed.createComponent(NgModelForm); + fixture.debugElement.componentInstance.name = 'aa'; + fixture.detectChanges(); + fixture.whenStable().then(() => { + fixture.detectChanges(); - const input = fixture.debugElement.query(By.css('input')).nativeElement; - const form = fixture.debugElement.children[0].injector.get(NgForm); - expect(sortedClassList(input)).toEqual(['ng-invalid', 'ng-pristine', 'ng-untouched']); + const input = fixture.debugElement.query(By.css('input')).nativeElement; + const form = fixture.debugElement.children[0].injector.get(NgForm); + expect(sortedClassList(input)).toEqual(['ng-invalid', 'ng-pristine', 'ng-untouched']); - dispatchEvent(input, 'blur'); - fixture.detectChanges(); + dispatchEvent(input, 'blur'); + fixture.detectChanges(); - expect(sortedClassList(input)).toEqual(['ng-invalid', 'ng-pristine', 'ng-touched']); + expect(sortedClassList(input)).toEqual(['ng-invalid', 'ng-pristine', 'ng-touched']); - input.value = 'updatedValue'; - dispatchEvent(input, 'input'); - fixture.detectChanges(); - expect(sortedClassList(input)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); - }); - }); + input.value = 'updatedValue'; + dispatchEvent(input, 'input'); + fixture.detectChanges(); + expect(sortedClassList(input)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); + }); + })); - it('should set status classes with ngModelGroup and ngForm', () => { - const fixture = TestBed.createComponent(NgModelGroupForm); - fixture.debugElement.componentInstance.first = ''; - fixture.detectChanges(); + it('should set status classes with ngModelGroup and ngForm', async(() => { + const fixture = TestBed.createComponent(NgModelGroupForm); + fixture.debugElement.componentInstance.first = ''; + fixture.detectChanges(); - const form = fixture.debugElement.query(By.css('form')).nativeElement; - const modelGroup = fixture.debugElement.query(By.css('[ngModelGroup]')).nativeElement; - const input = fixture.debugElement.query(By.css('input')).nativeElement; + const form = fixture.debugElement.query(By.css('form')).nativeElement; + const modelGroup = fixture.debugElement.query(By.css('[ngModelGroup]')).nativeElement; + const input = fixture.debugElement.query(By.css('input')).nativeElement; - // ngModelGroup creates its control asynchronously - fixture.whenStable().then(() => { - fixture.detectChanges(); - expect(sortedClassList(modelGroup)).toEqual([ - 'ng-invalid', 'ng-pristine', 'ng-untouched' - ]); + // ngModelGroup creates its control asynchronously + fixture.whenStable().then(() => { + fixture.detectChanges(); + expect(sortedClassList(modelGroup)).toEqual([ + 'ng-invalid', 'ng-pristine', 'ng-untouched' + ]); - expect(sortedClassList(form)).toEqual(['ng-invalid', 'ng-pristine', 'ng-untouched']); + expect(sortedClassList(form)).toEqual(['ng-invalid', 'ng-pristine', 'ng-untouched']); - dispatchEvent(input, 'blur'); - fixture.detectChanges(); + dispatchEvent(input, 'blur'); + fixture.detectChanges(); - expect(sortedClassList(modelGroup)).toEqual(['ng-invalid', 'ng-pristine', 'ng-touched']); - expect(sortedClassList(form)).toEqual(['ng-invalid', 'ng-pristine', 'ng-touched']); + expect(sortedClassList(modelGroup)).toEqual([ + 'ng-invalid', 'ng-pristine', 'ng-touched' + ]); + expect(sortedClassList(form)).toEqual(['ng-invalid', 'ng-pristine', 'ng-touched']); - input.value = 'updatedValue'; - dispatchEvent(input, 'input'); - fixture.detectChanges(); + input.value = 'updatedValue'; + dispatchEvent(input, 'input'); + fixture.detectChanges(); - expect(sortedClassList(modelGroup)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); - expect(sortedClassList(form)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); - }); - }); + expect(sortedClassList(modelGroup)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); + expect(sortedClassList(form)).toEqual(['ng-dirty', 'ng-touched', 'ng-valid']); + }); + })); it('should not create a template-driven form when ngNoForm is used', () => { const fixture = TestBed.createComponent(NgNoFormComp);