Skip to content

Commit

Permalink
test: Extend zoneless lint to cover tests (#29193)
Browse files Browse the repository at this point in the history
Only allow `provideZoneChangeDetection` and `NgZone` in tests that
explicitly test integration with Zone.js (and tests which have not yet
been migrated to zoneless).

(cherry picked from commit df8adda)
  • Loading branch information
mmalerba committed Jun 5, 2024
1 parent 65648a4 commit d25593f
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 66 deletions.
12 changes: 0 additions & 12 deletions src/cdk/scrolling/scroll-dispatcher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,6 @@ describe('ScrollDispatcher', () => {
expect(serviceSpy).toHaveBeenCalled();
}));

it('should not execute the global events in the Angular zone', () => {
scroll.scrolled(0).subscribe(() => {});
dispatchFakeEvent(document, 'scroll', false);

expect(fixture.ngZone!.isStable).toBe(true);
});

it('should not execute the scrollable events in the Angular zone', () => {
dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
expect(fixture.ngZone!.isStable).toBe(true);
});

it('should be able to unsubscribe from the global scrollable', () => {
const spy = jasmine.createSpy('global scroll callback');
const subscription = scroll.scrolled(0).subscribe(spy);
Expand Down
52 changes: 52 additions & 0 deletions src/cdk/scrolling/scroll-dispatcher.zone.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import {Component, ElementRef, ViewChild, provideZoneChangeDetection} from '@angular/core';
import {ComponentFixture, TestBed, inject, waitForAsync} from '@angular/core/testing';
import {dispatchFakeEvent} from '../testing/private';
import {ScrollDispatcher} from './scroll-dispatcher';
import {CdkScrollable} from './scrollable';
import {ScrollingModule} from './scrolling-module';

describe('ScrollDispatcher Zone.js integration', () => {
beforeEach(waitForAsync(() => {
TestBed.configureTestingModule({
imports: [ScrollingModule, ScrollingComponent],
providers: [provideZoneChangeDetection()],
});

TestBed.compileComponents();
}));

describe('Basic usage', () => {
let scroll: ScrollDispatcher;
let fixture: ComponentFixture<ScrollingComponent>;

beforeEach(inject([ScrollDispatcher], (s: ScrollDispatcher) => {
scroll = s;

fixture = TestBed.createComponent(ScrollingComponent);
fixture.detectChanges();
}));

it('should not execute the global events in the Angular zone', () => {
scroll.scrolled(0).subscribe(() => {});
dispatchFakeEvent(document, 'scroll', false);

expect(fixture.ngZone!.isStable).toBe(true);
});

it('should not execute the scrollable events in the Angular zone', () => {
dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
expect(fixture.ngZone!.isStable).toBe(true);
});
});
});

/** Simple component that contains a large div and can be scrolled. */
@Component({
template: `<div #scrollingElement cdkScrollable style="height: 9999px"></div>`,
standalone: true,
imports: [ScrollingModule],
})
class ScrollingComponent {
@ViewChild(CdkScrollable) scrollable: CdkScrollable;
@ViewChild('scrollingElement') scrollingElement: ElementRef<HTMLElement>;
}
32 changes: 3 additions & 29 deletions src/cdk/scrolling/viewport-ruler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import {TestBed, inject, fakeAsync, tick} from '@angular/core/testing';
import {TestBed, fakeAsync, inject, tick} from '@angular/core/testing';
import {dispatchFakeEvent} from '../testing/private';
import {ScrollingModule} from './public-api';
import {ViewportRuler} from './viewport-ruler';
import {dispatchFakeEvent} from '../testing/private';
import {NgZone} from '@angular/core';
import {Subscription} from 'rxjs';

describe('ViewportRuler', () => {
let viewportRuler: ViewportRuler;
let ngZone: NgZone;

let startingWindowWidth = window.innerWidth;
let startingWindowHeight = window.innerHeight;
Expand All @@ -24,9 +21,8 @@ describe('ViewportRuler', () => {
}),
);

beforeEach(inject([ViewportRuler, NgZone], (v: ViewportRuler, n: NgZone) => {
beforeEach(inject([ViewportRuler], (v: ViewportRuler) => {
viewportRuler = v;
ngZone = n;
scrollTo(0, 0);
}));

Expand Down Expand Up @@ -133,27 +129,5 @@ describe('ViewportRuler', () => {
expect(spy).toHaveBeenCalledTimes(1);
subscription.unsubscribe();
}));

it('should run the resize event outside the NgZone', () => {
const spy = jasmine.createSpy('viewport changed spy');
const subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));

dispatchFakeEvent(window, 'resize');
expect(spy).toHaveBeenCalledWith(false);
subscription.unsubscribe();
});

it('should run events outside of the NgZone, even if the subcription is from inside', () => {
const spy = jasmine.createSpy('viewport changed spy');
let subscription: Subscription;

ngZone.run(() => {
subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));
dispatchFakeEvent(window, 'resize');
});

expect(spy).toHaveBeenCalledWith(false);
subscription!.unsubscribe();
});
});
});
53 changes: 53 additions & 0 deletions src/cdk/scrolling/viewport-ruler.zone.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import {NgZone, provideZoneChangeDetection} from '@angular/core';
import {TestBed, inject} from '@angular/core/testing';
import {Subscription} from 'rxjs';
import {dispatchFakeEvent} from '../testing/private';
import {ScrollingModule} from './scrolling-module';
import {ViewportRuler} from './viewport-ruler';

describe('ViewportRuler', () => {
let viewportRuler: ViewportRuler;
let ngZone: NgZone;

// Create a very large element that will make the page scrollable.
let veryLargeElement: HTMLElement = document.createElement('div');
veryLargeElement.style.width = '6000px';
veryLargeElement.style.height = '6000px';

beforeEach(() =>
TestBed.configureTestingModule({
imports: [ScrollingModule],
providers: [provideZoneChangeDetection(), ViewportRuler],
}),
);

beforeEach(inject([ViewportRuler, NgZone], (v: ViewportRuler, n: NgZone) => {
viewportRuler = v;
ngZone = n;
scrollTo(0, 0);
}));

describe('changed event', () => {
it('should run the resize event outside the NgZone', () => {
const spy = jasmine.createSpy('viewport changed spy');
const subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));

dispatchFakeEvent(window, 'resize');
expect(spy).toHaveBeenCalledWith(false);
subscription.unsubscribe();
});

it('should run events outside of the NgZone, even if the subcription is from inside', () => {
const spy = jasmine.createSpy('viewport changed spy');
let subscription: Subscription;

ngZone.run(() => {
subscription = viewportRuler.change(0).subscribe(() => spy(NgZone.isInAngularZone()));
dispatchFakeEvent(window, 'resize');
});

expect(spy).toHaveBeenCalledWith(false);
subscription!.unsubscribe();
});
});
});
2 changes: 1 addition & 1 deletion src/cdk/text-field/autofill.zone.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {ComponentFixture, TestBed, inject} from '@angular/core/testing';
import {AutofillMonitor} from './autofill';
import {TextFieldModule} from './text-field-module';

describe('AutofillMonitor', () => {
describe('AutofillMonitor Zone.js integration', () => {
let autofillMonitor: AutofillMonitor;
let fixture: ComponentFixture<Inputs>;
let testComponent: Inputs;
Expand Down
5 changes: 3 additions & 2 deletions src/dev-app/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
// Load `$localize` for examples using it.
import '@angular/localize/init';

import {provideHttpClient} from '@angular/common/http';
import {
importProvidersFrom,
provideZoneChangeDetection,
provideExperimentalZonelessChangeDetection,
// tslint:disable-next-line:no-zone-dependencies -- Allow manual testing of dev-app with zones
provideZoneChangeDetection,
} from '@angular/core';
import {bootstrapApplication} from '@angular/platform-browser';
import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
import {provideHttpClient} from '@angular/common/http';
import {RouterModule} from '@angular/router';

import {Directionality} from '@angular/cdk/bidi';
Expand Down
35 changes: 17 additions & 18 deletions src/material-experimental/popover-edit/popover-edit.spec.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,17 @@
import {DataSource} from '@angular/cdk/collections';
import {LEFT_ARROW, UP_ARROW, RIGHT_ARROW, DOWN_ARROW, TAB} from '@angular/cdk/keycodes';
import {MatTableModule} from '@angular/material/table';
import {dispatchKeyboardEvent} from '../../cdk/testing/private';
import {DOWN_ARROW, LEFT_ARROW, RIGHT_ARROW, TAB, UP_ARROW} from '@angular/cdk/keycodes';
import {CommonModule} from '@angular/common';
import {
Component,
Directive,
ElementRef,
ViewChild,
provideZoneChangeDetection,
} from '@angular/core';
import {ComponentFixture, fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
import {Component, Directive, ElementRef, ViewChild} from '@angular/core';
import {ComponentFixture, TestBed, fakeAsync, flush, tick} from '@angular/core/testing';
import {FormsModule, NgForm} from '@angular/forms';
import {MatTableModule} from '@angular/material/table';
import {BehaviorSubject} from 'rxjs';
import {dispatchKeyboardEvent} from '../../cdk/testing/private';

import {
CdkPopoverEditColspan,
HoverContentState,
FormValueContainer,
HoverContentState,
PopoverEditClickOutBehavior,
} from '@angular/cdk-experimental/popover-edit';
import {MatPopoverEditModule} from './index';
Expand Down Expand Up @@ -297,12 +291,6 @@ const testCases = [
] as const;

describe('Material Popover Edit', () => {
beforeEach(() => {
TestBed.configureTestingModule({
providers: [provideZoneChangeDetection()],
});
});

for (const [componentClass, label] of testCases) {
describe(label, () => {
let component: BaseTestComponent;
Expand All @@ -317,6 +305,7 @@ describe('Material Popover Edit', () => {
component = fixture.componentInstance;
fixture.detectChanges();
tick(10);
fixture.detectChanges();
}));

describe('row hover content', () => {
Expand Down Expand Up @@ -432,6 +421,7 @@ describe('Material Popover Edit', () => {

it('does not trigger edit when disabled', fakeAsync(() => {
component.nameEditDisabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

// Uses Enter to open the lens.
Expand All @@ -452,6 +442,7 @@ describe('Material Popover Edit', () => {

it('unsets tabindex to 0 on disabled cells', () => {
component.nameEditDisabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(component.getEditCell().hasAttribute('tabindex')).toBe(false);
Expand Down Expand Up @@ -594,6 +585,7 @@ matPopoverEditTabOut`, fakeAsync(() => {

it('positions the lens at the top left corner and spans the full width of the cell', fakeAsync(() => {
component.openLens();
fixture.detectChanges();

const paneRect = component.getEditPane()!.getBoundingClientRect();
const cellRect = component.getEditCell().getBoundingClientRect();
Expand All @@ -610,16 +602,19 @@ matPopoverEditTabOut`, fakeAsync(() => {
);

component.colspan = {before: 1};
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

component.openLens();
fixture.detectChanges();

let paneRect = component.getEditPane()!.getBoundingClientRect();
expectPixelsToEqual(paneRect.top, cellRects[0].top);
expectPixelsToEqual(paneRect.left, cellRects[0].left);
expectPixelsToEqual(paneRect.right, cellRects[1].right);

component.colspan = {after: 1};
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

paneRect = component.getEditPane()!.getBoundingClientRect();
Expand All @@ -630,6 +625,7 @@ matPopoverEditTabOut`, fakeAsync(() => {
// expectPixelsToEqual(paneRect.right, cellRects[2].right);

component.colspan = {before: 1, after: 1};
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

paneRect = component.getEditPane()!.getBoundingClientRect();
Expand Down Expand Up @@ -706,13 +702,15 @@ matPopoverEditTabOut`, fakeAsync(() => {
expect(component.lensIsOpen()).toBe(false);

component.openLens();
fixture.detectChanges();

expect(component.getInput()!.value).toBe('Hydragon');
clearLeftoverTimers();
}));

it('resets the lens to original value', fakeAsync(() => {
component.openLens();
fixture.detectChanges();

component.getInput()!.value = 'Hydragon';
component.getInput()!.dispatchEvent(new Event('input'));
Expand All @@ -733,6 +731,7 @@ matPopoverEditTabOut`, fakeAsync(() => {
fixture.detectChanges();

component.openLens();
fixture.detectChanges();

component.getInput()!.value = 'Hydragon X';
component.getInput()!.dispatchEvent(new Event('input'));
Expand Down
14 changes: 13 additions & 1 deletion tools/tslint-rules/noZoneDependenciesRule.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import minimatch from 'minimatch';
import * as Lint from 'tslint';
import ts from 'typescript';
import minimatch from 'minimatch';

/**
* NgZone properties that are ok to access.
Expand Down Expand Up @@ -49,4 +49,16 @@ class Walker extends Lint.RuleWalker {

return super.visitPropertyAccessExpression(node);
}

override visitNamedImports(node: ts.NamedImports): void {
if (!this._enabled) {
return;
}

node.elements.forEach(specifier => {
if (specifier.name.getText() === 'provideZoneChangeDetection') {
this.addFailureAtNode(specifier, `Using zone change detection is not allowed.`);
}
});
}
}
8 changes: 5 additions & 3 deletions tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,12 @@
"no-zone-dependencies": [
true,
[
// Tests may need to verify behavior with zones.
"**/*.spec.ts",
// Allow in tests that specficially test integration with Zone.js.
"**/*.zone.spec.ts",
// TODO(mmalerba): following files to be cleaned up and removed from this list:
"**/cdk/a11y/focus-trap/focus-trap.ts"
"**/src/cdk/a11y/focus-trap/focus-trap.ts",
"**/src/cdk/testing/tests/testbed.spec.ts",
"**/src/material/**/*.spec.ts"
]
]
},
Expand Down

0 comments on commit d25593f

Please sign in to comment.