Skip to content

Commit fb2048b

Browse files
crisbetojelbourn
authored andcommitted
refactor(checkbox): switch to fakeAsync tests (#8639)
Moves all of the checkbox tests away from the `async` zone for improved reliability.
1 parent a1b5e75 commit fb2048b

File tree

1 file changed

+77
-60
lines changed

1 file changed

+77
-60
lines changed

src/lib/checkbox/checkbox.spec.ts

Lines changed: 77 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,18 @@
1-
import {
2-
async,
3-
ComponentFixture,
4-
fakeAsync,
5-
flushMicrotasks,
6-
TestBed,
7-
tick,
8-
} from '@angular/core/testing';
1+
import {ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
92
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
103
import {Component, DebugElement} from '@angular/core';
114
import {By} from '@angular/platform-browser';
125
import {dispatchFakeEvent} from '@angular/cdk/testing';
136
import {MatCheckbox, MatCheckboxChange, MatCheckboxModule} from './index';
147
import {RIPPLE_FADE_IN_DURATION, RIPPLE_FADE_OUT_DURATION} from '@angular/material/core';
158
import {MAT_CHECKBOX_CLICK_ACTION} from './checkbox-config';
9+
import {MutationObserverFactory} from '@angular/cdk/observers';
1610

1711

1812
describe('MatCheckbox', () => {
1913
let fixture: ComponentFixture<any>;
2014

21-
beforeEach(async(() => {
15+
beforeEach(fakeAsync(() => {
2216
TestBed.configureTestingModule({
2317
imports: [MatCheckboxModule, FormsModule, ReactiveFormsModule],
2418
declarations: [
@@ -116,7 +110,7 @@ describe('MatCheckbox', () => {
116110
fixture.detectChanges();
117111

118112
// Flush the microtasks because the forms module updates the model state asynchronously.
119-
flushMicrotasks();
113+
flush();
120114

121115
// The checked property has been updated from the model and now the view needs
122116
// to reflect the state change.
@@ -141,7 +135,7 @@ describe('MatCheckbox', () => {
141135
fixture.detectChanges();
142136

143137
// Flush the microtasks because the forms module updates the model state asynchronously.
144-
flushMicrotasks();
138+
flush();
145139

146140
// The checked property has been updated from the model and now the view needs
147141
// to reflect the state change.
@@ -153,7 +147,7 @@ describe('MatCheckbox', () => {
153147
expect(testComponent.isIndeterminate).toBe(false);
154148
}));
155149

156-
it('should not set indeterminate to false when checked is set programmatically', async(() => {
150+
it('should not set indeterminate to false when checked is set programmatically', () => {
157151
testComponent.isIndeterminate = true;
158152
fixture.detectChanges();
159153

@@ -176,7 +170,7 @@ describe('MatCheckbox', () => {
176170
expect(inputElement.indeterminate).toBe(true);
177171
expect(inputElement.checked).toBe(false);
178172
expect(testComponent.isIndeterminate).toBe(true);
179-
}));
173+
});
180174

181175
it('should change native element checked when check programmatically', () => {
182176
expect(inputElement.checked).toBe(false);
@@ -212,7 +206,7 @@ describe('MatCheckbox', () => {
212206
checkboxInstance._onInputClick(<Event>{stopPropagation: () => {}});
213207

214208
// Flush the microtasks because the indeterminate state will be updated in the next tick.
215-
flushMicrotasks();
209+
flush();
216210

217211
expect(checkboxInstance.checked).toBe(true);
218212
expect(checkboxInstance.indeterminate).toBe(false);
@@ -262,7 +256,7 @@ describe('MatCheckbox', () => {
262256
fixture.detectChanges();
263257

264258
// Flush the microtasks because the indeterminate state will be updated in the next tick.
265-
flushMicrotasks();
259+
flush();
266260

267261
expect(checkboxInstance.checked).toBe(true);
268262
expect(checkboxInstance.indeterminate).toBe(false);
@@ -317,7 +311,7 @@ describe('MatCheckbox', () => {
317311
expect(testComponent.onCheckboxClick).toHaveBeenCalledTimes(1);
318312
});
319313

320-
it('should trigger a change event when the native input does', async(() => {
314+
it('should trigger a change event when the native input does', fakeAsync(() => {
321315
spyOn(testComponent, 'onCheckboxChange');
322316

323317
expect(inputElement.checked).toBe(false);
@@ -329,16 +323,15 @@ describe('MatCheckbox', () => {
329323
expect(inputElement.checked).toBe(true);
330324
expect(checkboxNativeElement.classList).toContain('mat-checkbox-checked');
331325

332-
// Wait for the fixture to become stable, because the EventEmitter for the change event,
333-
// will only fire after the zone async change detection has finished.
334-
fixture.whenStable().then(() => {
335-
// The change event shouldn't fire, because the value change was not caused
336-
// by any interaction.
337-
expect(testComponent.onCheckboxChange).toHaveBeenCalledTimes(1);
338-
});
326+
fixture.detectChanges();
327+
flush();
328+
329+
// The change event shouldn't fire, because the value change was not caused
330+
// by any interaction.
331+
expect(testComponent.onCheckboxChange).toHaveBeenCalledTimes(1);
339332
}));
340333

341-
it('should not trigger the change event by changing the native value', async(() => {
334+
it('should not trigger the change event by changing the native value', fakeAsync(() => {
342335
spyOn(testComponent, 'onCheckboxChange');
343336

344337
expect(inputElement.checked).toBe(false);
@@ -350,14 +343,12 @@ describe('MatCheckbox', () => {
350343
expect(inputElement.checked).toBe(true);
351344
expect(checkboxNativeElement.classList).toContain('mat-checkbox-checked');
352345

353-
// Wait for the fixture to become stable, because the EventEmitter for the change event,
354-
// will only fire after the zone async change detection has finished.
355-
fixture.whenStable().then(() => {
356-
// The change event shouldn't fire, because the value change was not caused
357-
// by any interaction.
358-
expect(testComponent.onCheckboxChange).not.toHaveBeenCalled();
359-
});
346+
fixture.detectChanges();
347+
flush();
360348

349+
// The change event shouldn't fire, because the value change was not caused
350+
// by any interaction.
351+
expect(testComponent.onCheckboxChange).not.toHaveBeenCalled();
361352
}));
362353

363354
it('should forward the required attribute', () => {
@@ -576,7 +567,7 @@ describe('MatCheckbox', () => {
576567
inputElement.click();
577568

578569
fixture.detectChanges();
579-
flushMicrotasks();
570+
flush();
580571
fixture.detectChanges();
581572
expect(inputElement.checked).toBe(true);
582573
expect(checkboxNativeElement.classList).toContain('mat-checkbox-checked');
@@ -614,7 +605,7 @@ describe('MatCheckbox', () => {
614605
inputElement.click();
615606

616607
fixture.detectChanges();
617-
flushMicrotasks();
608+
flush();
618609
fixture.detectChanges();
619610

620611
expect(inputElement.checked).toBe(false);
@@ -630,7 +621,7 @@ describe('MatCheckbox', () => {
630621
inputElement.click();
631622

632623
fixture.detectChanges();
633-
flushMicrotasks();
624+
flush();
634625
fixture.detectChanges();
635626

636627
expect(inputElement.checked).toBe(true);
@@ -642,7 +633,7 @@ describe('MatCheckbox', () => {
642633
inputElement.click();
643634

644635
fixture.detectChanges();
645-
flushMicrotasks();
636+
flush();
646637
fixture.detectChanges();
647638

648639
expect(inputElement.checked).toBe(false);
@@ -689,22 +680,20 @@ describe('MatCheckbox', () => {
689680
expect(changeSpy).toHaveBeenCalledTimes(1);
690681
});
691682

692-
it('should not emit a DOM event to the change output', async(() => {
683+
it('should not emit a DOM event to the change output', fakeAsync(() => {
693684
fixture.detectChanges();
694685
expect(testComponent.lastEvent).toBeUndefined();
695686

696687
// Trigger the click on the inputElement, because the input will probably
697688
// emit a DOM event to the change output.
698689
inputElement.click();
699690
fixture.detectChanges();
691+
flush();
700692

701-
fixture.whenStable().then(() => {
702-
// We're checking the arguments type / emitted value to be a boolean, because sometimes the
703-
// emitted value can be a DOM Event, which is not valid.
704-
// See angular/angular#4059
705-
expect(testComponent.lastEvent.checked).toBe(true);
706-
});
707-
693+
// We're checking the arguments type / emitted value to be a boolean, because sometimes the
694+
// emitted value can be a DOM Event, which is not valid.
695+
// See angular/angular#4059
696+
expect(testComponent.lastEvent.checked).toBe(true);
708697
}));
709698
});
710699

@@ -789,7 +778,7 @@ describe('MatCheckbox', () => {
789778

790779
describe('with native tabindex attribute', () => {
791780

792-
it('should properly detect native tabindex attribute', async(() => {
781+
it('should properly detect native tabindex attribute', fakeAsync(() => {
793782
fixture = TestBed.createComponent(CheckboxWithTabindexAttr);
794783
fixture.detectChanges();
795784

@@ -835,7 +824,7 @@ describe('MatCheckbox', () => {
835824
});
836825

837826
it('should be in pristine, untouched, and valid states initially', fakeAsync(() => {
838-
flushMicrotasks();
827+
flush();
839828

840829
let checkboxElement = fixture.debugElement.query(By.directive(MatCheckbox));
841830
let ngModel = checkboxElement.injector.get<NgModel>(NgModel);
@@ -968,35 +957,63 @@ describe('MatCheckbox', () => {
968957
.toContain('mat-checkbox-inner-container-no-side-margin');
969958
});
970959

971-
it('should not remove margin if initial label is set through binding', async(() => {
960+
it('should not remove margin if initial label is set through binding', () => {
972961
testComponent.label = 'Some content';
973962
fixture.detectChanges();
974963

975964
expect(checkboxInnerContainer.classList)
976965
.not.toContain('mat-checkbox-inner-container-no-side-margin');
977-
}));
966+
});
967+
968+
it('should re-add margin if label is added asynchronously', () => {
969+
fixture.destroy();
970+
971+
const mutationCallbacks: Function[] = [];
972+
973+
TestBed
974+
.resetTestingModule()
975+
.configureTestingModule({
976+
imports: [MatCheckboxModule, FormsModule, ReactiveFormsModule],
977+
declarations: [CheckboxWithoutLabel],
978+
providers: [{
979+
provide: MutationObserverFactory,
980+
useValue: {
981+
// Stub out the factory that creates mutation observers for the underlying directive
982+
// to allows us to flush out the callbacks asynchronously.
983+
create: (callback: Function) => {
984+
mutationCallbacks.push(callback);
985+
986+
return {
987+
observe: () => {},
988+
disconnect: () => {}
989+
};
990+
}
991+
}
992+
}]
993+
})
994+
.compileComponents();
995+
996+
fixture = TestBed.createComponent(CheckboxWithoutLabel);
997+
checkboxInnerContainer = fixture.debugElement
998+
.query(By.css('.mat-checkbox-inner-container')).nativeElement;
978999

979-
it('should re-add margin if label is added asynchronously', async(() => {
9801000
fixture.detectChanges();
9811001

9821002
expect(checkboxInnerContainer.classList)
9831003
.toContain('mat-checkbox-inner-container-no-side-margin');
9841004

985-
testComponent.label = 'Some content';
1005+
fixture.componentInstance.label = 'Some content';
9861006
fixture.detectChanges();
1007+
mutationCallbacks.forEach(callback => callback());
9871008

988-
// Wait for the MutationObserver to detect the content change and for the cdkObserveContent
989-
// to emit the change event to the checkbox.
990-
setTimeout(() => {
991-
// The MutationObserver from the cdkObserveContent directive detected the content change
992-
// and notified the checkbox component. The checkbox then marks the component as dirty
993-
// by calling `markForCheck()`. This needs to be reflected by the component template then.
994-
fixture.detectChanges();
1009+
// The MutationObserver from the cdkObserveContent directive detected the content change
1010+
// and notified the checkbox component. The checkbox then marks the component as dirty
1011+
// by calling `markForCheck()`. This needs to be reflected by the component template then.
1012+
fixture.detectChanges();
9951013

996-
expect(checkboxInnerContainer.classList)
997-
.not.toContain('mat-checkbox-inner-container-no-side-margin');
998-
}, 1);
999-
}));
1014+
expect(checkboxInnerContainer.classList)
1015+
.not.toContain('mat-checkbox-inner-container-no-side-margin');
1016+
});
10001017

10011018
it('should not add the "name" attribute if it is not passed in', () => {
10021019
fixture.detectChanges();

0 commit comments

Comments
 (0)