build: do not leak providers and component styles between tests

Angular's `TestBed` by default only removes a test component from the
document body and destructs individual component fixtures. It never
destroys the test module properly, but always re-creates it.

This means that providers are not necessarily cleaned up (could
leak in memory) and component styles are not cleaned up / deduped.
This results in thousands of style elements in the browsers. Ultimately
causing significantly increased memory consumption and CPU blocking.

This potentially leads to repeated crashes within browsers (as seen in
BrowserStack and Saucelabs in the past). Initial testing of this change
unveiled a reduction from 30min tests to 5min in BrowserStack. This is
a signficant improvement and we should consider moving this change
upstream with: angular/angular#31834.

Benefits are: reduced test time; increased test/browser stability;
isolated tests without leaking styles and services.

Author: devversion

src/cdk/drag-drop/directives/drag.spec.ts

@@ -2169,7 +2169,7 @@ describe('CdkDrag', () => {
       fixture.componentInstance.boundarySelector = '.cdk-drop-list';
       fixture.detectChanges();
 
-      const container: HTMLElement = fixture.nativeElement.querySelector('.container');
+      const container: HTMLElement = fixture.nativeElement.querySelector('.scroll-container');
       const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
       const list = fixture.componentInstance.dropInstance.element.nativeElement;
       const cleanup = makeScrollable('vertical', container);
@@ -3889,7 +3889,7 @@ describe('CdkDrag', () => {
       const fixture = createComponent(DraggableInScrollableParentContainer);
       fixture.detectChanges();
       const item = fixture.componentInstance.dragItems.first.element.nativeElement;
-      const container = fixture.nativeElement.querySelector('.container');
+      const container = fixture.nativeElement.querySelector('.scroll-container');
       const containerRect = container.getBoundingClientRect();
 
       expect(container.scrollTop).toBe(0);
@@ -5519,7 +5519,7 @@ class StandaloneDraggableWithMultipleHandles {
 const DROP_ZONE_FIXTURE_TEMPLATE = `
   <div
     cdkDropList
-    class="drop-list"
+    class="drop-list scroll-container"
     style="width: 100px; background: pink;"
     [id]="dropZoneId"
     [cdkDropListData]="items"
@@ -5539,7 +5539,7 @@ const DROP_ZONE_FIXTURE_TEMPLATE = `
 `;
 
 @Component({template: DROP_ZONE_FIXTURE_TEMPLATE})
-class DraggableInDropZone {
+class DraggableInDropZone implements AfterViewInit {
   @ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
   @ViewChild(CdkDropList) dropInstance: CdkDropList;
   items = [
@@ -5556,6 +5556,15 @@ class DraggableInDropZone {
     moveItemInArray(this.items, event.previousIndex, event.currentIndex);
   });
   startedSpy = jasmine.createSpy('started spy');
+
+  constructor(protected _elementRef: ElementRef) {}
+
+  ngAfterViewInit() {
+    // Firefox preserves the `scrollTop` value from previous similar containers. This
+    // could throw off test assertions and result in flaky results.
+    // See: https://bugzilla.mozilla.org/show_bug.cgi?id=959812.
+    this._elementRef.nativeElement.querySelector('.scroll-container').scrollTop = 0;
+  }
 }
 
 @Component({
@@ -5579,8 +5588,8 @@ class DraggableInOnPushDropZone extends DraggableInDropZone {}
   `]
 })
 class DraggableInScrollableVerticalDropZone extends DraggableInDropZone {
-  constructor() {
-    super();
+  constructor(elementRef: ElementRef) {
+    super(elementRef);
 
     for (let i = 0; i < 60; i++) {
       this.items.push({value: `Extra item ${i}`, height: ITEM_HEIGHT, margin: 0});
@@ -5589,21 +5598,21 @@ class DraggableInScrollableVerticalDropZone extends DraggableInDropZone {
 }
 
 @Component({
-  template: '<div class="container" cdkScrollable>' + DROP_ZONE_FIXTURE_TEMPLATE + '</div>',
+  template: '<div class="scroll-container" cdkScrollable>' + DROP_ZONE_FIXTURE_TEMPLATE + '</div>',
 
   // Note that it needs a margin to ensure that it's not flush against the viewport
   // edge which will cause the viewport to scroll, rather than the list.
   styles: [`
-    .container {
+    .scroll-container {
       max-height: 200px;
       overflow: auto;
       margin: 10vw 0 0 10vw;
     }
   `]
 })
 class DraggableInScrollableParentContainer extends DraggableInDropZone {
-  constructor() {
-    super();
+  constructor(elementRef: ElementRef) {
+    super(elementRef);
 
     for (let i = 0; i < 60; i++) {
       this.items.push({value: `Extra item ${i}`, height: ITEM_HEIGHT, margin: 0});
@@ -5617,7 +5626,7 @@ class DraggableInScrollableParentContainer extends DraggableInDropZone {
   template: `
     <div
       cdkDropList
-      class="drop-list"
+      class="drop-list scroll-container"
       style="width: 100px; background: pink;"
       [id]="dropZoneId"
       [cdkDropListData]="items"
@@ -5656,7 +5665,7 @@ const HORIZONTAL_FIXTURE_STYLES = `
 
 const HORIZONTAL_FIXTURE_TEMPLATE = `
   <div
-    class="drop-list"
+    class="drop-list scroll-container"
     cdkDropList
     cdkDropListOrientation="horizontal"
     [cdkDropListData]="items"
@@ -5675,7 +5684,7 @@ const HORIZONTAL_FIXTURE_TEMPLATE = `
   styles: [HORIZONTAL_FIXTURE_STYLES],
   template: HORIZONTAL_FIXTURE_TEMPLATE
 })
-class DraggableInHorizontalDropZone {
+class DraggableInHorizontalDropZone implements AfterViewInit {
   @ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
   @ViewChild(CdkDropList) dropInstance: CdkDropList;
   items = [
@@ -5688,6 +5697,15 @@ class DraggableInHorizontalDropZone {
   droppedSpy = jasmine.createSpy('dropped spy').and.callFake((event: CdkDragDrop<string[]>) => {
     moveItemInArray(this.items, event.previousIndex, event.currentIndex);
   });
+
+  constructor(protected _elementRef: ElementRef) {}
+
+  ngAfterViewInit() {
+    // Firefox preserves the `scrollLeft` value from previous similar containers. This
+    // could throw off test assertions and result in flaky results.
+    // See: https://bugzilla.mozilla.org/show_bug.cgi?id=959812.
+    this._elementRef.nativeElement.querySelector('.scroll-container').scrollLeft = 0;
+  }
 }
 
 
@@ -5706,8 +5724,8 @@ class DraggableInHorizontalDropZone {
   `]
 })
 class DraggableInScrollableHorizontalDropZone extends DraggableInHorizontalDropZone {
-  constructor() {
-    super();
+  constructor(elementRef: ElementRef) {
+    super(elementRef);
 
     for (let i = 0; i < 60; i++) {
       this.items.push({value: `Extra item ${i}`, width: ITEM_WIDTH, margin: 0});
@@ -6142,6 +6160,7 @@ class ConnectedWrappedDropZones {
 @Component({
   template: `
     <div
+      class="drop-list scroll-container"
       cdkDropList
       style="width: 100px; background: pink;"
       [id]="dropZoneId"
@@ -6162,11 +6181,12 @@ class ConnectedWrappedDropZones {
   `
 })
 class DraggableWithCanvasInDropZone extends DraggableInDropZone implements AfterViewInit {
-  constructor(private _elementRef: ElementRef<HTMLElement>) {
-    super();
+  constructor(elementRef: ElementRef<HTMLElement>) {
+    super(elementRef);
   }
 
   ngAfterViewInit() {
+    super.ngAfterViewInit();
     const canvases = this._elementRef.nativeElement.querySelectorAll('canvas');
 
     // Add a circle to all the canvases.
@@ -6183,6 +6203,7 @@ class DraggableWithCanvasInDropZone extends DraggableInDropZone implements After
 @Component({
   template: `
     <div
+      class="drop-list scroll-container"
       cdkDropList
       style="width: 100px; background: pink;"
       [id]="dropZoneId"
@@ -6413,6 +6434,7 @@ class DraggableWithAlternateRootAndSelfHandle {
   template: `
     <div
       cdkDropList
+      class="drop-list scroll-container"
       style="width: 100px; background: pink;"
       [id]="dropZoneId"
       [cdkDropListData]="items"

src/cdk/scrolling/scrollable.spec.ts

@@ -31,12 +31,16 @@ describe('CdkScrollable', () => {
 
   beforeEach(() => {
     fixture = TestBed.createComponent(ScrollableViewport);
+    fixture.detectChanges();
     testComponent = fixture.componentInstance;
+    // Firefox preserves the `scrollTop` value from previous similar containers. This
+    // could throw off test assertions and result in flaky results.
+    // See: https://bugzilla.mozilla.org/show_bug.cgi?id=959812.
+    testComponent.scrollContainer.nativeElement.scrollTop = 0;
   });
 
   describe('in LTR context', () => {
     beforeEach(() => {
-      fixture.detectChanges();
       maxOffset = testComponent.scrollContainer.nativeElement.scrollHeight -
           testComponent.scrollContainer.nativeElement.clientHeight;
     });

test/angular-test-init-spec.ts

@@ -1,3 +1,4 @@
+import {NgModuleRef} from '@angular/core';
 import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {
   BrowserDynamicTestingModule,
@@ -8,8 +9,8 @@ import {
  * Common setup / initialization for all unit tests in Angular Material and CDK.
  */
 
-const testBed = TestBed.initTestEnvironment(
-    [BrowserDynamicTestingModule], platformBrowserDynamicTesting());
+const testBed =
+    TestBed.initTestEnvironment([BrowserDynamicTestingModule], platformBrowserDynamicTesting());
 patchTestBedToDestroyFixturesAfterEveryTest(testBed);
 
 (window as any).module = {};
@@ -38,9 +39,24 @@ function patchTestBedToDestroyFixturesAfterEveryTest(testBedInstance: TestBed) {
   // Monkey-patch the resetTestingModule to destroy fixtures outside of a try/catch block.
   // With https://github.com/angular/angular/commit/2c5a67134198a090a24f6671dcdb7b102fea6eba
   // errors when destroying components are no longer causing Jasmine to fail.
-  testBedInstance.resetTestingModule = function(this: {_activeFixtures: ComponentFixture<any>[]}) {
+  testBedInstance.resetTestingModule = function(this: {
+    /** List of active fixtures in the current testing module. */
+    _activeFixtures: ComponentFixture<any>[],
+    /** Module Ref used in the Ivy TestBed for creating components. */
+    _testModuleRef?: NgModuleRef<unknown>|null,
+    /** Module Ref used in the View Engine TestBed for creating components. */
+    _moduleRef?: NgModuleRef<unknown>|null
+  }) {
     try {
+      const moduleRef = this._testModuleRef || this._moduleRef;
       this._activeFixtures.forEach((fixture: ComponentFixture<any>) => fixture.destroy());
+      // Destroy the TestBed `NgModule` reference to clear out shared styles that would
+      // otherwise remain in DOM and significantly increase memory consumption in browsers.
+      // This increased consumption then results in noticeable test instability and slow-down.
+      // See: https://github.com/angular/angular/issues/31834.
+      if (moduleRef != null) {
+        moduleRef.destroy();
+      }
     } finally {
       this._activeFixtures = [];
       // Regardless of errors or not, run the original reset testing module function.

test/karma-test-shim.js

@@ -79,7 +79,15 @@ function patchTestBedToDestroyFixturesAfterEveryTest(testBed) {
   // errors when destroying components are no longer causing Jasmine to fail.
   testBed.resetTestingModule = function() {
     try {
+      const moduleRef = this._testModuleRef || this._moduleRef;
       this._activeFixtures.forEach(function (fixture) { fixture.destroy(); });
+      // Destroy the TestBed `NgModule` reference to clear out shared styles that would
+      // otherwise remain in DOM and significantly increase memory consumption in browsers.
+      // This increased consumption then results in noticeable test instability and slow-down.
+      // See: https://github.com/angular/angular/issues/31834.
+      if (moduleRef != null) {
+        moduleRef.destroy();
+      }
     } finally {
       this._activeFixtures = [];
       // Regardless of errors or not, run the original reset testing module function.