test: avoid leaking some LViews in tests (#57546)

crisbeto · alxhub · commit df42d2be163f · 2024-08-27T13:29:09.000-07:00
We had some tests that were leaking LViews, because we were testing things like `createComponent`, but not destroying them afterwards. These changes clean up most of them, although there are a handful still left that I didn't have time to fully track down. PR Close #57546
diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts
@@ -1105,6 +1105,7 @@ describe('change detection for transplanted views', () => {
     // goes and marks the root view dirty, which then starts the process all over again by
     // checking the declaration.
     expect(() => appRef.tick()).not.toThrow();
+    app.destroy();
   });
   it('does not cause infinite loops with exhaustive checkNoChanges', async () => {
     TestBed.configureTestingModule({
diff --git a/packages/core/test/acceptance/component_spec.ts b/packages/core/test/acceptance/component_spec.ts
@@ -738,6 +738,7 @@ describe('component', () => {
       componentRef.instance.name = 'ZoneJS';
       componentRef.changeDetectorRef.detectChanges();
       expect(hostElement.textContent).toBe('Hello ZoneJS!');
+      componentRef.destroy();
     });
 
     it('should create an instance of an NgModule-based component', () => {
@@ -797,6 +798,7 @@ describe('component', () => {
 
       componentRef.changeDetectorRef.detectChanges();
       expect(hostElement.innerHTML.replace(/\s*/g, '')).toBe('<p>1</p>|<p>2</p>|<p>3</p>');
+      componentRef.destroy();
     });
 
     it('should be able to inject tokens from EnvironmentInjector', () => {
@@ -817,6 +819,7 @@ describe('component', () => {
       componentRef.changeDetectorRef.detectChanges();
 
       expect(hostElement.textContent).toBe('Token: EnvironmentInjector(A)');
+      componentRef.destroy();
     });
 
     it('should be able to use NodeInjector from the node hierarchy', () => {
@@ -890,6 +893,7 @@ describe('component', () => {
       expect(hostElement.tagName.toLowerCase()).toBe(selector);
 
       expect(hostElement.textContent).toBe('Hello Angular!');
+      componentRef.destroy();
     });
 
     it(
@@ -917,6 +921,7 @@ describe('component', () => {
         expect(hostElement.tagName.toLowerCase()).toBe('div');
 
         expect(hostElement.textContent).toBe('Hello Angular!');
+        componentRef.destroy();
       },
     );
 
diff --git a/packages/core/test/acceptance/content_spec.ts b/packages/core/test/acceptance/content_spec.ts
@@ -1668,6 +1668,7 @@ describe('projection', () => {
       componentRef.changeDetectorRef.detectChanges();
 
       expect(getElementHtml(hostElement)).toContain('<p>override</p>|Two fallback|Three fallback');
+      componentRef.destroy();
     });
 
     it('should render fallback content when ng-content is inside an ng-template', () => {
diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts
@@ -144,14 +144,15 @@ describe('ViewContainerRef', () => {
 
       // Add a test component to the view container ref to ensure that
       // the "ng-container" comment was used as marker for the insertion.
-      vcref.createComponent(HelloComp);
+      const ref = vcref.createComponent(HelloComp);
       fixture.detectChanges();
 
       expect(testParent.textContent).toBe('hello');
       expect(testParent.childNodes.length).toBe(2);
       expect(testParent.childNodes[0].nodeType).toBe(Node.ELEMENT_NODE);
       expect(testParent.childNodes[0].textContent).toBe('hello');
       expect(testParent.childNodes[1].nodeType).toBe(Node.COMMENT_NODE);
+      ref.destroy();
     });
 
     it('should support attribute selectors in dynamically created components', () => {
@@ -167,7 +168,7 @@ describe('ViewContainerRef', () => {
         @ViewChild('container', {read: ViewContainerRef}) vcRef!: ViewContainerRef;
 
         createComponent() {
-          this.vcRef.createComponent(HelloComp);
+          return this.vcRef.createComponent(HelloComp);
         }
       }
 
@@ -176,9 +177,10 @@ describe('ViewContainerRef', () => {
       fixture.detectChanges();
       expect(fixture.debugElement.nativeElement.innerHTML).not.toContain('Hello');
 
-      fixture.componentInstance.createComponent();
+      const ref = fixture.componentInstance.createComponent();
       fixture.detectChanges();
       expect(fixture.debugElement.nativeElement.innerHTML).toContain('Hello');
+      ref.destroy();
     });
 
     it('should view queries in dynamically created components', () => {
@@ -294,11 +296,11 @@ describe('ViewContainerRef', () => {
         ) {}
 
         createComponentViaVCRef() {
-          this.vcRef.createComponent(HelloComp);
+          return this.vcRef.createComponent(HelloComp);
         }
 
         createComponentViaFactory() {
-          createComponent(HelloComp, {
+          return createComponent(HelloComp, {
             environmentInjector: this.injector,
             hostElement: this.elementRef.nativeElement.querySelector('#factory'),
           });
@@ -308,8 +310,8 @@ describe('ViewContainerRef', () => {
       TestBed.configureTestingModule({declarations: [TestComp, HelloComp]});
       const fixture = TestBed.createComponent(TestComp);
       fixture.detectChanges();
-      fixture.componentInstance.createComponentViaVCRef();
-      fixture.componentInstance.createComponentViaFactory();
+      const firstRef = fixture.componentInstance.createComponentViaVCRef();
+      const secondRef = fixture.componentInstance.createComponentViaFactory();
       fixture.detectChanges();
 
       // Verify host element for a component created via  `vcRef.createComponent` method
@@ -336,6 +338,8 @@ describe('ViewContainerRef', () => {
       // Make sure selector-based attrs and classes were not added to the host element
       expect(factoryHostElement.classList.contains('class-a')).toBe(false);
       expect(factoryHostElement.getAttribute('attr-c')).toBe(null);
+      firstRef.destroy();
+      secondRef.destroy();
     });
   });
 
@@ -1534,6 +1538,7 @@ describe('ViewContainerRef', () => {
         fixture.componentInstance.viewContainerRef.createComponent(DynamicComponent);
       const element = componentRef.location.nativeElement;
       expect((element.namespaceURI || '').toLowerCase()).not.toContain('svg');
+      componentRef.destroy();
     });
 
     it('should be compatible with componentRef generated via TestBed.createComponent in component factory', () => {
diff --git a/packages/core/test/linker/regression_integration_spec.ts b/packages/core/test/linker/regression_integration_spec.ts
@@ -365,6 +365,7 @@ describe('regressions', () => {
     });
 
     expect(compRef.location.nativeElement.hasAttribute('ng-version')).toBe(false);
+    compRef.destroy();
   });
 });
 
diff --git a/packages/core/test/render3/reactivity_spec.ts b/packages/core/test/render3/reactivity_spec.ts
@@ -100,6 +100,7 @@ describe('effects', () => {
       ),
     );
     await expectAsync(p).toBeResolvedTo([false, true]);
+    componentRef.destroy();
   });
 
   it('should propagate errors to the ErrorHandler', () => {
@@ -543,7 +544,7 @@ describe('effects', () => {
       const fixture = TestBed.createComponent(DriverCmp);
       fixture.detectChanges();
 
-      fixture.componentInstance.vcr.createComponent(TestCmp);
+      const ref = fixture.componentInstance.vcr.createComponent(TestCmp);
 
       // Verify that simply creating the component didn't schedule the effect.
       TestBed.flushEffects();
@@ -552,6 +553,7 @@ describe('effects', () => {
       // Running change detection should schedule and run the effect.
       fixture.detectChanges();
       expect(log).toEqual(['init', 'effect']);
+      ref.destroy();
     });
 
     it('when created in a service provided in a component', () => {
