Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

fix(spec): do not count requestAnimationFrame as a pending timer #854

Merged
merged 1 commit into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 16 additions & 9 deletions lib/zone-spec/fake-async-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
args: any[];
delay: number;
isPeriodic: boolean;
isRequestAnimationFrame: boolean;
}

interface MicroTaskScheduledFunction {
Expand All @@ -35,7 +36,7 @@

scheduleFunction(
cb: Function, delay: number, args: any[] = [], isPeriodic: boolean = false,
id: number = -1): number {
isRequestAnimationFrame: boolean = false, id: number = -1): number {
Copy link
Collaborator

@JiaLiPassion JiaLiPassion Jul 25, 2017

Choose a reason for hiding this comment

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

@vikerman , should we also add an option flag in FakeAsyncTestSpec to control whether to consider RAF as timer or not?

constructor(namePrefix: string, isRequestAnimationFrame: boolean = false) {
      this.name = 'fakeAsyncTestZone for ' + namePrefix;
      this.isRequestAnimationFrame = isRequestAnimationFrame;
    }

Copy link
Contributor Author

@vikerman vikerman Jul 25, 2017

Choose a reason for hiding this comment

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

Done. Added it as an optional parameter to FakeAsyncTestSpec. Will enforce it in Angular 5.0 as a breaking change but leave it as it is for now.

let currentId: number = id < 0 ? this.nextId++ : id;
let endTime = this._currentTime + delay;

Expand All @@ -46,7 +47,8 @@
func: cb,
args: args,
delay: delay,
isPeriodic: isPeriodic
isPeriodic: isPeriodic,
isRequestAnimationFrame: isRequestAnimationFrame
};
let i = 0;
for (; i < this._schedulerQueue.length; i++) {
Expand Down Expand Up @@ -100,7 +102,8 @@
' tasks. Does your code use a polling timeout?');
}
// If the only remaining tasks are periodic, finish flushing.
if (!(this._schedulerQueue.filter(task => !task.isPeriodic).length)) {
if (!(this._schedulerQueue.filter(task => !task.isPeriodic && !task.isRequestAnimationFrame)
.length)) {
break;
}
let current = this._schedulerQueue.shift();
Expand Down Expand Up @@ -131,7 +134,7 @@
pendingPeriodicTimers: number[] = [];
pendingTimers: number[] = [];

constructor(namePrefix: string) {
constructor(namePrefix: string, private trackPendingRequestAnimationFrame = false) {
this.name = 'fakeAsyncTestZone for ' + namePrefix;
}

Expand Down Expand Up @@ -174,7 +177,7 @@
return () => {
// Requeue the timer callback if it's not been canceled.
if (this.pendingPeriodicTimers.indexOf(id) !== -1) {
this._scheduler.scheduleFunction(fn, interval, args, true, id);
this._scheduler.scheduleFunction(fn, interval, args, true, false, id);
}
};
}
Expand All @@ -185,12 +188,14 @@
};
}

private _setTimeout(fn: Function, delay: number, args: any[]): number {
private _setTimeout(fn: Function, delay: number, args: any[], isTimer = true): number {
let removeTimerFn = this._dequeueTimer(this._scheduler.nextId);
// Queue the callback and dequeue the timer on success and error.
let cb = this._fnAndFlush(fn, {onSuccess: removeTimerFn, onError: removeTimerFn});
let id = this._scheduler.scheduleFunction(cb, delay, args);
this.pendingTimers.push(id);
let id = this._scheduler.scheduleFunction(cb, delay, args, false, !isTimer);
if (isTimer) {
this.pendingTimers.push(id);
}
return id;
}

Expand Down Expand Up @@ -302,7 +307,9 @@
case 'mozRequestAnimationFrame':
// Simulate a requestAnimationFrame by using a setTimeout with 16 ms.
// (60 frames per second)
task.data['handleId'] = this._setTimeout(task.invoke, 16, (task.data as any)['args']);
task.data['handleId'] = this._setTimeout(
task.invoke, 16, (task.data as any)['args'],
this.trackPendingRequestAnimationFrame);
break;
default:
throw new Error('Unknown macroTask scheduled in fake async test: ' + task.source);
Expand Down
7 changes: 7 additions & 0 deletions test/zone-spec/fake-async-test.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,13 @@ describe('FakeAsyncTestZoneSpec', () => {
expect(ran).toEqual(true);
});
});
it('does not count as a pending timer', () => {
fakeAsyncTestZone.run(() => {
requestAnimationFrame(() => {});
});
expect(testZoneSpec.pendingTimers.length).toBe(0);
expect(testZoneSpec.pendingPeriodicTimers.length).toBe(0);
});
it('should cancel a scheduled requestAnimatiomFrame', () => {
fakeAsyncTestZone.run(() => {
let ran = false;
Expand Down