From 223c49b0d62cab322f99728540533d0756f578f3 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 14 Nov 2017 17:22:58 +0100 Subject: [PATCH] chore(tooltip): unit test failures * Fixes a regression in the tooltip that was introduced by 0719c3866225874c2d900ece2554612675ce4d0b which caused the tests to fail. * Avoids issues in the future where failures in some tooltip tests can cause Jasmine to throw the browser into an infinite loop by trying to stringify a circular object. --- src/lib/tooltip/tooltip.spec.ts | 46 +++++++++++++++++++-------------- src/lib/tooltip/tooltip.ts | 3 ++- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/lib/tooltip/tooltip.spec.ts b/src/lib/tooltip/tooltip.spec.ts index 951a5d50baed..1c2b481eec63 100644 --- a/src/lib/tooltip/tooltip.spec.ts +++ b/src/lib/tooltip/tooltip.spec.ts @@ -53,7 +53,7 @@ describe('MatTooltip', () => { return {getContainerElement: () => overlayContainerElement}; }}, {provide: Directionality, useFactory: () => { - return dir = { value: 'ltr' }; + return dir = {value: 'ltr'}; }} ] }); @@ -80,7 +80,7 @@ describe('MatTooltip', () => { }); it('should show and hide the tooltip', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.show(); tick(0); // Tick for the show delay (default is 0) @@ -110,7 +110,7 @@ describe('MatTooltip', () => { // On animation complete, should expect that the tooltip has been detached. flushMicrotasks(); - expect(tooltipDirective._tooltipInstance).toBeNull(); + assertTooltipInstance(tooltipDirective, false); })); it('should be able to re-open a tooltip if it was closed by detaching the overlay', @@ -126,7 +126,7 @@ describe('MatTooltip', () => { fixture.detectChanges(); expect(tooltipDirective._isTooltipVisible()).toBe(false); flushMicrotasks(); - expect(tooltipDirective._tooltipInstance).toBeNull(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.show(); tick(0); @@ -134,7 +134,7 @@ describe('MatTooltip', () => { })); it('should show with delay', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); const tooltipDelay = 1000; tooltipDirective.show(tooltipDelay); @@ -192,7 +192,7 @@ describe('MatTooltip', () => { })); it('should not show if hide is called before delay finishes', async(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); const tooltipDelay = 1000; @@ -209,27 +209,27 @@ describe('MatTooltip', () => { })); it('should not show tooltip if message is not present or empty', () => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.message = undefined!; fixture.detectChanges(); tooltipDirective.show(); - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.message = null!; fixture.detectChanges(); tooltipDirective.show(); - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.message = ''; fixture.detectChanges(); tooltipDirective.show(); - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.message = ' '; fixture.detectChanges(); tooltipDirective.show(); - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); }); it('should not follow through with hide if show is called after', fakeAsync(() => { @@ -252,7 +252,7 @@ describe('MatTooltip', () => { const initialPosition: TooltipPosition = 'below'; const changedPosition: TooltipPosition = 'above'; - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.position = initialPosition; tooltipDirective.show(); @@ -264,12 +264,12 @@ describe('MatTooltip', () => { // Different position value should destroy the tooltip tooltipDirective.position = changedPosition; - expect(tooltipDirective._tooltipInstance).toBeNull(); + assertTooltipInstance(tooltipDirective, false); expect(tooltipDirective._overlayRef).toBeNull(); }); it('should be able to modify the tooltip message', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.show(); tick(0); // Tick for the show delay (default is 0) @@ -286,7 +286,7 @@ describe('MatTooltip', () => { })); it('should allow extra classes to be set on the tooltip', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.show(); tick(0); // Tick for the show delay (default is 0) @@ -510,7 +510,7 @@ describe('MatTooltip', () => { })); it('should not show the tooltip on progammatic focus', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); buttonElement.focus(); tick(0); @@ -585,7 +585,7 @@ describe('MatTooltip', () => { }); it('should hide tooltip if clipped after changing positions', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); // Show the tooltip and tick for the show delay (default is 0) tooltipDirective.show(); @@ -626,7 +626,7 @@ describe('MatTooltip', () => { }); it('should show and hide the tooltip', fakeAsync(() => { - expect(tooltipDirective._tooltipInstance).toBeUndefined(); + assertTooltipInstance(tooltipDirective, false); tooltipDirective.show(); tick(0); // Tick for the show delay (default is 0) @@ -654,7 +654,7 @@ describe('MatTooltip', () => { // On animation complete, should expect that the tooltip has been detached. flushMicrotasks(); - expect(tooltipDirective._tooltipInstance).toBeNull(); + assertTooltipInstance(tooltipDirective, false); })); it('should have rendered the tooltip text on init', fakeAsync(() => { @@ -751,3 +751,11 @@ class DynamicTooltipsDemo { return this._elementRef.nativeElement.querySelectorAll('button'); } } + +/** Asserts whether a tooltip directive has a tooltip instance. */ +function assertTooltipInstance(tooltip: MatTooltip, shouldExist: boolean): void { + // Note that we have to cast this to a boolean, because Jasmine will go into an infinite loop + // if it tries to stringify the `_tooltipInstance` when an assertion fails. The infinite loop + // happens due to the `_tooltipInstance` having a circular structure. + expect(!!tooltip._tooltipInstance).toBe(shouldExist); +} diff --git a/src/lib/tooltip/tooltip.ts b/src/lib/tooltip/tooltip.ts index 4e37dbacc4f3..d1c243b532a3 100644 --- a/src/lib/tooltip/tooltip.ts +++ b/src/lib/tooltip/tooltip.ts @@ -25,6 +25,7 @@ import { import {Platform} from '@angular/cdk/platform'; import {ComponentPortal} from '@angular/cdk/portal'; import {first} from 'rxjs/operators/first'; +import {merge} from 'rxjs/observable/merge'; import {ScrollDispatcher} from '@angular/cdk/scrolling'; import { ChangeDetectionStrategy, @@ -261,7 +262,7 @@ export class MatTooltip implements OnDestroy { this._tooltipInstance = overlayRef.attach(portal).instance; // Dispose of the tooltip when the overlay is detached. - overlayRef.detachments().subscribe(() => { + merge(this._tooltipInstance!.afterHidden(), overlayRef.detachments()).subscribe(() => { // Check first if the tooltip has already been removed through this components destroy. if (this._tooltipInstance) { this._disposeTooltip();