Skip to content

Commit 325533a

Browse files
authored
test(multiple): remove provideZoneChangeDetection for all menu tests (#29061)
This removes 'provideZoneChangeDetection' from menu tests. Change summary: * Many tests set inputs directly. This isn't really how it happens in applications - inputs set from the template automatically mark components for check. When tests set inputs directly like this, they need to call `markForCheck` manually (or something similar). * Similarly, test components need to be marked for check when setting values * One test seems to be calling a public API (`CdkContextMenuTrigger.open`) that failed to call `markForCheck` so that was added to the implementation * `fakeAsync` had an additional timer so I just added `flush` to make it work.
1 parent 0dc172a commit 325533a

8 files changed

+45
-47
lines changed

src/cdk/menu/context-menu-trigger.spec.ts

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,4 @@
1-
import {
2-
Component,
3-
ViewChild,
4-
ElementRef,
5-
Type,
6-
ViewChildren,
7-
QueryList,
8-
provideZoneChangeDetection,
9-
} from '@angular/core';
1+
import {Component, ViewChild, ElementRef, Type, ViewChildren, QueryList} from '@angular/core';
102
import {CdkMenuModule} from './menu-module';
113
import {TestBed, waitForAsync, ComponentFixture} from '@angular/core/testing';
124
import {CdkMenu} from './menu';
@@ -26,7 +18,6 @@ describe('CdkContextMenuTrigger', () => {
2618
TestBed.configureTestingModule({
2719
imports: [CdkMenuModule],
2820
declarations: [SimpleContextMenu],
29-
providers: [provideZoneChangeDetection()],
3021
}).compileComponents();
3122
}));
3223

@@ -160,7 +151,6 @@ describe('CdkContextMenuTrigger', () => {
160151
beforeEach(waitForAsync(() => {
161152
TestBed.configureTestingModule({
162153
imports: [CdkMenuModule],
163-
providers: [provideZoneChangeDetection()],
164154
declarations: [NestedContextMenu],
165155
}).compileComponents();
166156
}));
@@ -223,6 +213,7 @@ describe('CdkContextMenuTrigger', () => {
223213
' is disabled',
224214
() => {
225215
fixture.componentInstance.copyMenuDisabled = true;
216+
fixture.changeDetectorRef.markForCheck();
226217
fixture.detectChanges();
227218
openCopyContextMenu();
228219

@@ -410,7 +401,6 @@ describe('CdkContextMenuTrigger', () => {
410401
function createComponent<T>(componentClass: Type<T>) {
411402
TestBed.configureTestingModule({
412403
imports: [CdkMenuModule],
413-
providers: [provideZoneChangeDetection()],
414404
declarations: [componentClass],
415405
}).compileComponents();
416406

src/cdk/menu/context-menu-trigger.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {booleanAttribute, Directive, inject, Injectable, Input, OnDestroy} from '@angular/core';
9+
import {
10+
booleanAttribute,
11+
ChangeDetectorRef,
12+
Directive,
13+
inject,
14+
Injectable,
15+
Input,
16+
OnDestroy,
17+
} from '@angular/core';
1018
import {Directionality} from '@angular/cdk/bidi';
1119
import {
1220
FlexibleConnectedPositionStrategy,
@@ -83,6 +91,8 @@ export class CdkContextMenuTrigger extends CdkMenuTriggerBase implements OnDestr
8391
/** The app's context menu tracking registry */
8492
private readonly _contextMenuTracker = inject(ContextMenuTracker);
8593

94+
private readonly _changeDetectorRef = inject(ChangeDetectorRef);
95+
8696
/** Whether the context menu is disabled. */
8797
@Input({alias: 'cdkContextMenuDisabled', transform: booleanAttribute}) disabled: boolean = false;
8898

@@ -97,6 +107,7 @@ export class CdkContextMenuTrigger extends CdkMenuTriggerBase implements OnDestr
97107
*/
98108
open(coordinates: ContextMenuCoordinates) {
99109
this._open(null, coordinates);
110+
this._changeDetectorRef.markForCheck();
100111
}
101112

102113
/** Close the currently opened context menu. */

src/cdk/menu/menu-item-checkbox.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component, ElementRef, provideZoneChangeDetection} from '@angular/core';
1+
import {Component, ElementRef} from '@angular/core';
22
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
33
import {By} from '@angular/platform-browser';
44
import {CdkMenuModule} from './menu-module';
@@ -16,7 +16,6 @@ describe('MenuItemCheckbox', () => {
1616
TestBed.configureTestingModule({
1717
imports: [CdkMenuModule, SingleCheckboxButton],
1818
providers: [
19-
provideZoneChangeDetection(),
2019
{provide: CDK_MENU, useClass: CdkMenu},
2120
{provide: MENU_STACK, useClass: MenuStack},
2221
// View engine can't figure out the ElementRef to inject so we need to provide a fake
@@ -43,6 +42,7 @@ describe('MenuItemCheckbox', () => {
4342
expect(checkboxElement.getAttribute('aria-disabled')).toBeNull();
4443

4544
checkbox.disabled = true;
45+
fixture.changeDetectorRef.markForCheck();
4646
fixture.detectChanges();
4747

4848
expect(checkboxElement.getAttribute('aria-disabled')).toBe('true');

src/cdk/menu/menu-item-radio.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component, ElementRef, provideZoneChangeDetection} from '@angular/core';
1+
import {Component, ElementRef} from '@angular/core';
22
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
33
import {By} from '@angular/platform-browser';
44
import {UniqueSelectionDispatcher} from '@angular/cdk/collections';
@@ -19,7 +19,6 @@ describe('MenuItemRadio', () => {
1919
TestBed.configureTestingModule({
2020
imports: [CdkMenuModule, SimpleRadioButton],
2121
providers: [
22-
provideZoneChangeDetection(),
2322
{provide: UniqueSelectionDispatcher, useValue: selectionDispatcher},
2423
{provide: CDK_MENU, useClass: CdkMenu},
2524
{provide: MENU_STACK, useClass: MenuStack},
@@ -47,6 +46,7 @@ describe('MenuItemRadio', () => {
4746
expect(radioElement.getAttribute('aria-disabled')).toBeNull();
4847

4948
radioButton.disabled = true;
49+
fixture.componentRef.changeDetectorRef.markForCheck();
5050
fixture.detectChanges();
5151

5252
expect(radioElement.getAttribute('aria-disabled')).toBe('true');

src/cdk/menu/menu-item.spec.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component, Type, ElementRef, provideZoneChangeDetection} from '@angular/core';
1+
import {Component, Type, ElementRef} from '@angular/core';
22
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
33
import {dispatchKeyboardEvent} from '@angular/cdk/testing/private';
44
import {By} from '@angular/platform-browser';
@@ -20,7 +20,6 @@ describe('MenuItem', () => {
2020
imports: [CdkMenuModule],
2121
declarations: [SingleMenuItem],
2222
providers: [
23-
provideZoneChangeDetection(),
2423
{provide: CDK_MENU, useClass: CdkMenu},
2524
{provide: MENU_STACK, useClass: MenuStack},
2625
// View engine can't figure out the ElementRef to inject so we need to provide a fake
@@ -49,11 +48,13 @@ describe('MenuItem', () => {
4948
expect(nativeButton.hasAttribute('aria-disabled')).toBeFalse();
5049

5150
menuItem.disabled = true;
51+
fixture.changeDetectorRef.markForCheck();
5252
fixture.detectChanges();
5353

5454
expect(nativeButton.getAttribute('aria-disabled')).toBe('true');
5555

5656
menuItem.disabled = false;
57+
fixture.changeDetectorRef.markForCheck();
5758
fixture.detectChanges();
5859

5960
expect(nativeButton.hasAttribute('aria-disabled')).toBeFalse();
@@ -83,7 +84,6 @@ describe('MenuItem', () => {
8384
TestBed.configureTestingModule({
8485
imports: [CdkMenuModule, MatIcon],
8586
providers: [
86-
provideZoneChangeDetection(),
8787
{provide: CDK_MENU, useClass: CdkMenu},
8888
{provide: MENU_STACK, useClass: MenuStack},
8989
// View engine can't figure out the ElementRef to inject so we need to provide a fake
@@ -108,6 +108,7 @@ describe('MenuItem', () => {
108108
const fixture = createComponent(MenuItemWithIcon);
109109
expect(menuItem.getLabel()).toEqual('unicorn Click me!');
110110
fixture.componentInstance.typeahead = 'Click me!';
111+
fixture.changeDetectorRef.markForCheck();
111112
fixture.detectChanges();
112113
expect(menuItem.getLabel()).toEqual('Click me!');
113114
});
@@ -119,6 +120,7 @@ describe('MenuItem', () => {
119120
const fixture = createComponent(MenuItemWithIconClass);
120121
expect(menuItem.getLabel()).toEqual('unicorn Click me!');
121122
fixture.componentInstance.typeahead = 'Click me!';
123+
fixture.changeDetectorRef.markForCheck();
122124
fixture.detectChanges();
123125
expect(menuItem.getLabel()).toEqual('Click me!');
124126
},
@@ -136,6 +138,7 @@ describe('MenuItem', () => {
136138
const fixture = createComponent(MenuItemWithMultipleNestings);
137139
expect(menuItem.getLabel()).toEqual('unicorn Click menume!');
138140
fixture.componentInstance.typeahead = 'Click me!';
141+
fixture.changeDetectorRef.markForCheck();
139142
fixture.detectChanges();
140143
expect(menuItem.getLabel()).toEqual('Click me!');
141144
},

src/cdk/menu/menu-trigger.spec.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,4 @@
1-
import {
2-
Component,
3-
ViewChildren,
4-
QueryList,
5-
ElementRef,
6-
ViewChild,
7-
Type,
8-
provideZoneChangeDetection,
9-
} from '@angular/core';
1+
import {Component, ViewChildren, QueryList, ElementRef, ViewChild, Type} from '@angular/core';
102
import {ComponentFixture, TestBed, fakeAsync, tick, waitForAsync} from '@angular/core/testing';
113
import {By} from '@angular/platform-browser';
124
import {dispatchKeyboardEvent} from '../../cdk/testing/private';
@@ -27,7 +19,6 @@ describe('MenuTrigger', () => {
2719
TestBed.configureTestingModule({
2820
imports: [CdkMenuModule],
2921
declarations: [TriggerForEmptyMenu],
30-
providers: [provideZoneChangeDetection()],
3122
}).compileComponents();
3223
}));
3324

@@ -51,6 +42,7 @@ describe('MenuTrigger', () => {
5142
expect(menuItemElement.getAttribute('aria-disabled')).toBeNull();
5243

5344
menuItem.disabled = true;
45+
fixture.changeDetectorRef.markForCheck();
5446
fixture.detectChanges();
5547

5648
expect(menuItemElement.getAttribute('aria-disabled')).toBe('true');
@@ -60,6 +52,7 @@ describe('MenuTrigger', () => {
6052
expect(menuItemElement.getAttribute('aria-haspopup')).toEqual('menu');
6153

6254
fixture.componentInstance.trigger.menuTemplateRef = null;
55+
fixture.changeDetectorRef.markForCheck();
6356
fixture.detectChanges();
6457

6558
expect(menuItemElement.hasAttribute('aria-haspopup')).toBe(false);
@@ -69,6 +62,7 @@ describe('MenuTrigger', () => {
6962
expect(menuItem.hasMenu).toBeTrue();
7063

7164
fixture.componentInstance.trigger.menuTemplateRef = null;
65+
fixture.changeDetectorRef.markForCheck();
7266
fixture.detectChanges();
7367

7468
expect(menuItem.hasMenu).toBeFalse();
@@ -83,6 +77,7 @@ describe('MenuTrigger', () => {
8377
expect(menuItemElement.getAttribute('aria-expanded')).toBe('false');
8478

8579
fixture.componentInstance.trigger.menuTemplateRef = null;
80+
fixture.changeDetectorRef.markForCheck();
8681
fixture.detectChanges();
8782

8883
expect(menuItemElement.hasAttribute('aria-expanded')).toBeFalse();

src/cdk/menu/pointer-focus-tracker.spec.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
1-
import {
2-
Component,
3-
QueryList,
4-
ElementRef,
5-
ViewChildren,
6-
AfterViewInit,
7-
provideZoneChangeDetection,
8-
} from '@angular/core';
1+
import {Component, QueryList, ElementRef, ViewChildren, AfterViewInit} from '@angular/core';
92
import {waitForAsync, ComponentFixture, TestBed} from '@angular/core/testing';
103
import {createMouseEvent, dispatchEvent} from '../../cdk/testing/private';
114
import {Observable} from 'rxjs';
@@ -26,7 +19,6 @@ describe('FocusMouseManger', () => {
2619

2720
beforeEach(waitForAsync(() => {
2821
TestBed.configureTestingModule({
29-
providers: [provideZoneChangeDetection()],
3022
imports: [MultiElementWithConditionalComponent, MockWrapper],
3123
}).compileComponents();
3224
}));
@@ -68,6 +60,7 @@ describe('FocusMouseManger', () => {
6860

6961
expect(mockElements.length).toBe(2);
7062
fixture.componentInstance.showThird = true;
63+
fixture.changeDetectorRef.markForCheck();
7164
fixture.detectChanges();
7265
getComponentsForTesting();
7366

src/material/menu/menu.spec.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import {
2525
Type,
2626
ViewChild,
2727
ViewChildren,
28-
provideZoneChangeDetection,
2928
} from '@angular/core';
3029
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
3130
import {MatRipple} from '@angular/material/core';
@@ -64,7 +63,7 @@ describe('MDC-based MatMenu', () => {
6463
declarations: any[] = [],
6564
): ComponentFixture<T> {
6665
TestBed.configureTestingModule({
67-
providers: [provideZoneChangeDetection(), ...providers],
66+
providers,
6867
imports: [MatMenuModule, NoopAnimationsModule],
6968
declarations: [component, ...declarations],
7069
}).compileComponents();
@@ -96,6 +95,7 @@ describe('MDC-based MatMenu', () => {
9695
expect(triggerElement.getAttribute('aria-haspopup')).toBe('menu');
9796

9897
fixture.componentInstance.trigger.menu = null;
98+
fixture.changeDetectorRef.markForCheck();
9999
fixture.detectChanges();
100100

101101
expect(triggerElement.hasAttribute('aria-haspopup')).toBe(false);
@@ -371,6 +371,7 @@ describe('MDC-based MatMenu', () => {
371371

372372
// Add 50 items to make the menu scrollable
373373
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
374+
fixture.changeDetectorRef.markForCheck();
374375
fixture.detectChanges();
375376

376377
const triggerEl = fixture.componentInstance.triggerEl.nativeElement;
@@ -650,6 +651,7 @@ describe('MDC-based MatMenu', () => {
650651
instance.ariaLabel = 'Custom aria-label';
651652
instance.ariaLabelledby = 'custom-labelled-by';
652653
instance.ariaDescribedby = 'custom-described-by';
654+
fixture.changeDetectorRef.markForCheck();
653655
fixture.detectChanges();
654656

655657
expect(menuPanel.getAttribute('aria-label')).toBe('Custom aria-label');
@@ -658,6 +660,7 @@ describe('MDC-based MatMenu', () => {
658660

659661
// Change these to empty strings to make sure that we don't preserve empty attributes.
660662
instance.ariaLabel = instance.ariaLabelledby = instance.ariaDescribedby = '';
663+
fixture.changeDetectorRef.markForCheck();
661664
fixture.detectChanges();
662665

663666
expect(menuPanel.hasAttribute('aria-label')).toBe(false);
@@ -904,11 +907,11 @@ describe('MDC-based MatMenu', () => {
904907
}));
905908

906909
it('should throw if assigning a menu that contains the trigger', fakeAsync(() => {
907-
expect(() => {
908-
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
909-
fixture.detectChanges();
910-
tick(500);
911-
}).toThrowError(/menu cannot contain its own trigger/);
910+
const errorSpy = spyOn(console, 'error');
911+
const fixture = createComponent(InvalidRecursiveMenu, [], [FakeIcon]);
912+
fixture.detectChanges();
913+
tick(500);
914+
expect(errorSpy.calls.first().args[1]).toMatch(/menu cannot contain its own trigger/);
912915
}));
913916

914917
it('should be able to swap out a menu after the first time it is opened', fakeAsync(() => {
@@ -1222,6 +1225,7 @@ describe('MDC-based MatMenu', () => {
12221225
expect(Math.floor(panelRect.bottom)).toBeLessThan(viewportHeight);
12231226

12241227
fixture.componentInstance.extraItems = new Array(50).fill('Hello there');
1228+
fixture.changeDetectorRef.markForCheck();
12251229
fixture.detectChanges();
12261230
panelRect = panel.getBoundingClientRect();
12271231
expect(Math.floor(panelRect.bottom)).toBe(viewportHeight);
@@ -1408,6 +1412,7 @@ describe('MDC-based MatMenu', () => {
14081412
trigger.style.top = '200px';
14091413

14101414
fixture.componentInstance.yPosition = 'above';
1415+
fixture.changeDetectorRef.markForCheck();
14111416
fixture.detectChanges();
14121417

14131418
fixture.componentInstance.trigger.openMenu();
@@ -1425,6 +1430,7 @@ describe('MDC-based MatMenu', () => {
14251430
tick(500);
14261431

14271432
fixture.componentInstance.yPosition = 'below';
1433+
fixture.changeDetectorRef.markForCheck();
14281434
fixture.detectChanges();
14291435

14301436
fixture.componentInstance.trigger.openMenu();
@@ -2499,6 +2505,7 @@ describe('MDC-based MatMenu', () => {
24992505
fixture.detectChanges();
25002506
tick(500);
25012507
fixture.detectChanges();
2508+
flush();
25022509

25032510
expect(lazyTrigger.classList)
25042511
.withContext('Expected the trigger to be highlighted')
@@ -2711,7 +2718,6 @@ describe('MatMenu default overrides', () => {
27112718
TestBed.configureTestingModule({
27122719
imports: [MatMenuModule, NoopAnimationsModule],
27132720
providers: [
2714-
provideZoneChangeDetection(),
27152721
{
27162722
provide: MAT_MENU_DEFAULT_OPTIONS,
27172723
useValue: {overlapTrigger: true, xPosition: 'before', yPosition: 'above'},

0 commit comments

Comments
 (0)