Skip to content

chore(tooltip): unit test failures #8385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 14, 2017
Merged

chore(tooltip): unit test failures #8385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions src/lib/tooltip/tooltip.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe('MatTooltip', () => {
return {getContainerElement: () => overlayContainerElement};
}},
{provide: Directionality, useFactory: () => {
return dir = { value: 'ltr' };
return dir = {value: 'ltr'};
}}
]
});
Expand All @@ -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)
Expand Down Expand Up @@ -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',
Expand All @@ -126,15 +126,15 @@ describe('MatTooltip', () => {
fixture.detectChanges();
expect(tooltipDirective._isTooltipVisible()).toBe(false);
flushMicrotasks();
expect(tooltipDirective._tooltipInstance).toBeNull();
assertTooltipInstance(tooltipDirective, false);

tooltipDirective.show();
tick(0);
expect(tooltipDirective._isTooltipVisible()).toBe(true);
}));

it('should show with delay', fakeAsync(() => {
expect(tooltipDirective._tooltipInstance).toBeUndefined();
assertTooltipInstance(tooltipDirective, false);

const tooltipDelay = 1000;
tooltipDirective.show(tooltipDelay);
Expand Down Expand Up @@ -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;

Expand All @@ -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(() => {
Expand All @@ -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();
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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);
}
3 changes: 2 additions & 1 deletion src/lib/tooltip/tooltip.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the ! up to the assignment for tooltipInstance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TS isn't smart enough to figure it out when it's being assigned to a property, it only works for variables.

// Check first if the tooltip has already been removed through this components destroy.
if (this._tooltipInstance) {
this._disposeTooltip();
Expand Down