Skip to content

Commit 50d00d8

Browse files
jerome-benoitjerome-benoit
authored
fix: properly handle async function at all benchmark steps (#133)
Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com> Co-authored-by: Jérôme Benoit <jerome.benoit@sap.com>
1 parent ef1caa1 commit 50d00d8

File tree

4 files changed

+97
-47
lines changed

4 files changed

+97
-47
lines changed

‎src/task.ts‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
import {
1515
absoluteDeviation,
1616
getVariance,
17+
isAsyncFnResource,
1718
isAsyncTask,
1819
medianSorted,
1920
quantileSorted,
@@ -32,6 +33,9 @@ export default class Task extends EventTarget {
3233
*/
3334
name: string;
3435

36+
/**
37+
* Task function
38+
*/
3539
fn: Fn;
3640

3741
/*
@@ -69,34 +73,52 @@ export default class Task extends EventTarget {
6973
const samples: number[] = [];
7074
if (this.opts.beforeAll != null) {
7175
try {
72-
await this.opts.beforeAll.call(this);
76+
if (await isAsyncFnResource(this.opts.beforeAll)) {
77+
await this.opts.beforeAll.call(this);
78+
} else {
79+
this.opts.beforeAll.call(this);
80+
}
7381
} catch (error) {
7482
return { error };
7583
}
7684
}
77-
const isAsync = await isAsyncTask(this);
7885

86+
const asyncBeforeEach = this.opts.beforeEach != null
87+
&& (await isAsyncFnResource(this.opts.beforeEach));
88+
const asyncTask = await isAsyncTask(this);
89+
const asyncAfterEach = this.opts.afterEach != null
90+
&& (await isAsyncFnResource(this.opts.afterEach));
91+
92+
// TODO: factor out
7993
const executeTask = async () => {
8094
if (this.opts.beforeEach != null) {
81-
await this.opts.beforeEach.call(this);
95+
if (asyncBeforeEach) {
96+
await this.opts.beforeEach.call(this);
97+
} else {
98+
this.opts.beforeEach.call(this);
99+
}
82100
}
83101

84102
let taskTime = 0;
85-
if (isAsync) {
103+
if (asyncTask) {
86104
const taskStart = this.bench.now();
87-
await this.fn.call(this);
105+
await this.fn();
88106
taskTime = this.bench.now() - taskStart;
89107
} else {
90108
const taskStart = this.bench.now();
91-
this.fn.call(this);
109+
this.fn();
92110
taskTime = this.bench.now() - taskStart;
93111
}
94112

95113
samples.push(taskTime);
96114
totalTime += taskTime;
97115

98116
if (this.opts.afterEach != null) {
99-
await this.opts.afterEach.call(this);
117+
if (asyncAfterEach) {
118+
await this.opts.afterEach.call(this);
119+
} else {
120+
this.opts.afterEach.call(this);
121+
}
100122
}
101123
};
102124

@@ -123,7 +145,11 @@ export default class Task extends EventTarget {
123145

124146
if (this.opts.afterAll != null) {
125147
try {
126-
await this.opts.afterAll.call(this);
148+
if (await isAsyncFnResource(this.opts.afterAll)) {
149+
await this.opts.afterAll.call(this);
150+
} else {
151+
this.opts.afterAll.call(this);
152+
}
127153
} catch (error) {
128154
return { error };
129155
}

‎src/utils.ts‎

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -7,58 +7,69 @@ export const hrtimeNow = () => nanoToMs(Number(process.hrtime.bigint()));
77

88
export const now = () => performance.now();
99

10-
function isPromiseLike<T>(
10+
/**
11+
* Checks if a value is a promise-like object.
12+
*
13+
* @param maybePromiseLike - the value to check
14+
* @returns true if the value is a promise-like object
15+
*/
16+
const isPromiseLike = <T>(
1117
maybePromiseLike: any,
12-
): maybePromiseLike is PromiseLike<T> {
13-
return (
14-
maybePromiseLike !== null
15-
&& typeof maybePromiseLike === 'object'
16-
&& typeof maybePromiseLike.then === 'function'
17-
);
18-
}
18+
): maybePromiseLike is PromiseLike<T> => maybePromiseLike !== null
19+
&& typeof maybePromiseLike === 'object'
20+
&& typeof maybePromiseLike.then === 'function';
1921

20-
// eslint-disable-next-line @typescript-eslint/no-empty-function
21-
const AsyncFunctionConstructor = (async () => {}).constructor;
22+
type AsyncFunctionType<A extends unknown[], R> = (...args: A) => PromiseLike<R>;
2223

2324
/**
24-
* An async function check method only consider runtime support async syntax
25+
* An async function check helper only considering runtime support async syntax
26+
*
27+
* @param fn - the function to check
28+
* @returns true if the function is an async function
2529
*/
26-
export const isAsyncFunction = (fn: Fn) => fn.constructor === AsyncFunctionConstructor;
30+
const isAsyncFunction = (
31+
fn: Fn,
32+
// eslint-disable-next-line @typescript-eslint/no-empty-function
33+
): fn is AsyncFunctionType<unknown[], unknown> => fn?.constructor === (async () => {}).constructor;
2734

28-
export const isAsyncTask = async (task: Task) => {
29-
if (isAsyncFunction(task.fn)) {
35+
/**
36+
* An async function check helper considering runtime support async syntax and promise return
37+
*
38+
* @param fn - the function to check
39+
* @returns true if the function is an async function or returns a promise
40+
*/
41+
export const isAsyncFnResource = async (fn: Fn): Promise<boolean> => {
42+
if (fn == null) {
43+
return false;
44+
}
45+
if (isAsyncFunction(fn)) {
3046
return true;
3147
}
3248
try {
33-
if (task.opts.beforeEach != null) {
34-
try {
35-
await task.opts.beforeEach.call(task);
36-
} catch (e) {
37-
// ignore
38-
}
39-
}
40-
const call = task.fn();
41-
const promiseLike = isPromiseLike(call);
49+
const fnCall = fn();
50+
const promiseLike = isPromiseLike(fnCall);
4251
if (promiseLike) {
52+
// silence promise rejection
4353
try {
44-
await call;
45-
} catch (e) {
46-
// ignore
47-
}
48-
}
49-
if (task.opts.afterEach != null) {
50-
try {
51-
await task.opts.afterEach.call(task);
52-
} catch (e) {
54+
await fnCall;
55+
} catch {
5356
// ignore
5457
}
5558
}
5659
return promiseLike;
57-
} catch (e) {
60+
} catch {
5861
return false;
5962
}
6063
};
6164

65+
/**
66+
* An async task check helper considering runtime support async syntax and promise return
67+
*
68+
* @param task - the task to check
69+
* @returns true if the task is an async task
70+
*/
71+
export const isAsyncTask = async (task: Task): Promise<boolean> => isAsyncFnResource(task?.fn);
72+
6273
/**
6374
* Computes the average of a sample.
6475
*
@@ -69,7 +80,6 @@ export const average = (samples: number[]) => {
6980
if (samples.length === 0) {
7081
throw new Error('samples must not be empty');
7182
}
72-
7383
return samples.reduce((a, b) => a + b, 0) / samples.length || 0;
7484
};
7585

‎test/index.test.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -312,10 +312,14 @@ test('task beforeAll, afterAll, beforeEach, afterEach', async () => {
312312
await bench.warmup();
313313
await bench.run();
314314

315-
expect(beforeAll.mock.calls.length).toBe(2);
316-
expect(afterAll.mock.calls.length).toBe(2);
317-
expect(beforeEach.mock.calls.length).toBe(iterations * 2 /* warmup + run */);
318-
expect(afterEach.mock.calls.length).toBe(iterations * 2);
315+
expect(beforeAll.mock.calls.length).toBe(4 /* async check + warmup + run */);
316+
expect(afterAll.mock.calls.length).toBe(4 /* async check + warmup + run */);
317+
expect(beforeEach.mock.calls.length).toBe(
318+
2 + iterations * 2 /* async check + warmup + run */,
319+
);
320+
expect(afterEach.mock.calls.length).toBe(
321+
2 + iterations * 2 /* async check + warmup + run */,
322+
);
319323
expect(beforeEach.mock.calls.length).toBe(afterEach.mock.calls.length);
320324
});
321325

‎test/isAsyncTask.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,16 @@ import { isAsyncTask } from '../src/utils';
44

55
const bench = new Bench();
66

7+
test('isAsyncTask undefined', () => {
8+
// @ts-expect-error: testing with undefined
9+
expect(isAsyncTask(undefined)).resolves.toBe(false);
10+
});
11+
12+
test('isAsyncTask null', () => {
13+
// @ts-expect-error: testing with null
14+
expect(isAsyncTask(null)).resolves.toBe(false);
15+
});
16+
717
test('isAsyncTask sync', () => {
818
const task = new Task(bench, 'foo', () => 1);
919
expect(isAsyncTask(task)).resolves.toBe(false);
@@ -26,7 +36,7 @@ test('isAsyncTask promiseLike', () => {
2636
expect(isAsyncTask(task)).resolves.toBe(true);
2737
});
2838

29-
test('isAsyncTask promise with catch', () => {
39+
test('isAsyncTask promise with error', () => {
3040
const task = new Task(bench, 'foo', () => Promise.reject(new Error('foo')));
3141
expect(isAsyncTask(task)).resolves.toBe(true);
3242
});

0 commit comments

Comments
 (0)