Skip to content

Commit c0bac58

Browse files
W-A-JamesW-A-James
authored
fix(NODE-6706): Move creation and deletion of install location (#35)
1 parent bfd4a57 commit c0bac58

File tree

4 files changed

+198
-140
lines changed

4 files changed

+198
-140
lines changed

‎packages/bson-bench/src/suite.ts

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import { writeFile } from 'fs/promises';
1+
import { mkdir, rm, writeFile } from 'fs/promises';
2+
import { tmpdir } from 'os';
3+
import * as path from 'path';
24

35
import { type BenchmarkResult, type BenchmarkSpecification, type PerfSendResult } from './common';
46
import { Task } from './task';
7+
import { exists } from './utils';
58

69
/**
710
* A collection of related Tasks
@@ -10,8 +13,10 @@ export class Suite {
1013
tasks: Task[];
1114
name: string;
1215
hasRun: boolean;
16+
1317
_errors: { task: Task; error: Error }[];
1418
private _results: PerfSendResult[];
19+
static packageInstallLocation: string = path.join(tmpdir(), 'bsonBench');
1520

1621
constructor(name: string) {
1722
this.name = name;
@@ -29,33 +34,51 @@ export class Suite {
2934
return this;
3035
}
3136

37+
async makeInstallLocation() {
38+
if (!(await exists(Suite.packageInstallLocation))) {
39+
await mkdir(Suite.packageInstallLocation);
40+
}
41+
}
42+
43+
async cleanUpInstallLocation() {
44+
await rm(Suite.packageInstallLocation, { recursive: true, force: true });
45+
}
46+
3247
/**
3348
* Run all Tasks. Throws error if suite has already been run
3449
* Collects all results and thrown errors from child Tasks
3550
*/
3651
async run(): Promise<void> {
3752
if (this.hasRun) throw new Error('Suite has already been run');
3853

39-
console.log(`Suite: ${this.name}`);
40-
for (const task of this.tasks) {
41-
const result = await task.run().then(
42-
(_r: BenchmarkResult) => task.getResults(),
43-
(e: Error) => e
44-
);
45-
if (result instanceof Error) {
46-
console.log(`\t${task.testName} ✗`);
47-
this._errors.push({ task, error: result });
48-
} else {
49-
console.log(`\t${task.testName} ✓`);
50-
this._results.push(result);
54+
try {
55+
// install required modules before running child process as new Node processes need to know that
56+
// it exists before they can require it.
57+
await this.makeInstallLocation();
58+
59+
console.log(`Suite: ${this.name}`);
60+
for (const task of this.tasks) {
61+
const result = await task.run().then(
62+
(_r: BenchmarkResult) => task.getResults(),
63+
(e: Error) => e
64+
);
65+
if (result instanceof Error) {
66+
console.log(`\t${task.testName} ✗`);
67+
this._errors.push({ task, error: result });
68+
} else {
69+
console.log(`\t${task.testName} ✓`);
70+
this._results.push(result);
71+
}
5172
}
52-
}
5373

54-
for (const { task, error } of this._errors) {
55-
console.log(`Task ${task.taskName} failed with Error '${error.message}'`);
56-
}
74+
for (const { task, error } of this._errors) {
75+
console.log(`Task ${task.taskName} failed with Error '${error.message}'`);
76+
}
5777

58-
this.hasRun = true;
78+
this.hasRun = true;
79+
} finally {
80+
await this.cleanUpInstallLocation();
81+
}
5982
}
6083

6184
/**

‎packages/bson-bench/src/task.ts

Lines changed: 30 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { type ChildProcess, fork } from 'child_process';
22
import { once } from 'events';
3-
import { mkdir, rm, writeFile } from 'fs/promises';
4-
import { tmpdir } from 'os';
3+
import { writeFile } from 'fs/promises';
54
import * as path from 'path';
65

76
import {
@@ -12,7 +11,7 @@ import {
1211
type PerfSendResult,
1312
type ResultMessage
1413
} from './common';
15-
import { exists } from './utils';
14+
import { Suite } from './suite';
1615

1716
/**
1817
* An individual benchmark task that runs in its own Node.js process
@@ -27,13 +26,11 @@ export class Task {
2726
/** @internal */
2827
hasRun: boolean;
2928

30-
static packageInstallLocation: string = path.join(tmpdir(), 'bsonBench');
31-
3229
constructor(benchmarkSpec: BenchmarkSpecification) {
3330
this.result = undefined;
3431
this.children = [];
3532
this.hasRun = false;
36-
this.benchmark = { ...benchmarkSpec, installLocation: Task.packageInstallLocation };
33+
this.benchmark = { ...benchmarkSpec, installLocation: Suite.packageInstallLocation };
3734

3835
this.taskName = `${path.basename(this.benchmark.documentPath, 'json')}_${
3936
this.benchmark.operation
@@ -172,43 +169,33 @@ export class Task {
172169
async run(): Promise<BenchmarkResult> {
173170
if (this.hasRun && this.result) return this.result;
174171

175-
// install required modules before running child process as new Node processes need to know that
176-
// it exists before they can require it.
177-
if (!(await exists(Task.packageInstallLocation))) {
178-
await mkdir(Task.packageInstallLocation);
179-
}
180-
181-
try {
182-
const pack = new Package(this.benchmark.library, Task.packageInstallLocation);
183-
if (!pack.check()) await pack.install();
184-
// spawn child process
185-
const child = fork(`${__dirname}/base`, {
186-
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
187-
serialization: 'advanced'
188-
});
189-
child.send({ type: 'runBenchmark', benchmark: this.benchmark });
190-
this.children.push(child);
191-
192-
// listen for results or error
193-
const resultOrErrorPromise = once(child, 'message');
194-
// Wait for process to exit
195-
const exit = once(child, 'exit');
196-
197-
const resultOrError: ResultMessage | ErrorMessage = (await resultOrErrorPromise)[0];
198-
await exit;
199-
200-
this.hasRun = true;
201-
switch (resultOrError.type) {
202-
case 'returnResult':
203-
this.result = resultOrError.result;
204-
return resultOrError.result;
205-
case 'returnError':
206-
throw resultOrError.error;
207-
default:
208-
throw new Error('Unexpected result returned from child process');
209-
}
210-
} finally {
211-
await rm(Task.packageInstallLocation, { recursive: true, force: true });
172+
const pack = new Package(this.benchmark.library, Suite.packageInstallLocation);
173+
if (!pack.check()) await pack.install();
174+
// spawn child process
175+
const child = fork(`${__dirname}/base`, {
176+
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
177+
serialization: 'advanced'
178+
});
179+
child.send({ type: 'runBenchmark', benchmark: this.benchmark });
180+
this.children.push(child);
181+
182+
// listen for results or error
183+
const resultOrErrorPromise = once(child, 'message');
184+
// Wait for process to exit
185+
const exit = once(child, 'exit');
186+
187+
const resultOrError: ResultMessage | ErrorMessage = (await resultOrErrorPromise)[0];
188+
await exit;
189+
190+
this.hasRun = true;
191+
switch (resultOrError.type) {
192+
case 'returnResult':
193+
this.result = resultOrError.result;
194+
return resultOrError.result;
195+
case 'returnError':
196+
throw resultOrError.error;
197+
default:
198+
throw new Error('Unexpected result returned from child process');
212199
}
213200
}
214201
}

‎packages/bson-bench/test/unit/suite.test.ts

Lines changed: 119 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,10 @@
11
import { expect } from 'chai';
22
import { readFile } from 'fs/promises';
33

4-
import { Suite, Task } from '../../lib';
4+
import { Suite } from '../../lib';
55
import { exists } from '../../src/utils';
6-
import { clearTestedDeps } from '../utils';
76

87
describe('Suite', function () {
9-
beforeEach(async function () {
10-
await clearTestedDeps(Task.packageInstallLocation);
11-
});
12-
13-
after(async function () {
14-
await clearTestedDeps(Task.packageInstallLocation);
15-
});
16-
178
describe('#task()', function () {
189
it('returns the Suite it was called on', function () {
1910
const suite = new Suite('test');
@@ -78,6 +69,123 @@ describe('Suite', function () {
7869
expect(await suite.run().catch(e => e)).to.be.instanceOf(Error);
7970
});
8071
});
72+
73+
it('creates a temp directory for packages', async function () {
74+
const s = new Suite('test');
75+
s.task({
76+
documentPath: 'test/documents/long_largeArray.json',
77+
library: 'bson@5',
78+
operation: 'deserialize',
79+
warmup: 100,
80+
iterations: 10000,
81+
options: {}
82+
});
83+
84+
const checkForDirectory = async () => {
85+
for (let i = 0; i < 10; i++) {
86+
if (await exists(Suite.packageInstallLocation)) return true;
87+
}
88+
return false;
89+
};
90+
const suiteRunPromise = s.run().catch(e => e);
91+
92+
const result = await Promise.race([checkForDirectory(), suiteRunPromise]);
93+
expect(typeof result).to.equal('boolean');
94+
expect(result).to.be.true;
95+
96+
const suiteRunResult = await suiteRunPromise;
97+
expect(suiteRunResult).to.not.be.instanceOf(Error);
98+
});
99+
100+
context('after completing successfully', function () {
101+
it('deletes the temp directory', async function () {
102+
const s = new Suite('test');
103+
s.task({
104+
documentPath: 'test/documents/long_largeArray.json',
105+
library: 'bson@5',
106+
operation: 'deserialize',
107+
warmup: 100,
108+
iterations: 100,
109+
options: {}
110+
});
111+
112+
const maybeError = await s.run().catch(e => e);
113+
expect(maybeError).to.not.be.instanceOf(Error);
114+
115+
const tmpdirExists = await exists(Suite.packageInstallLocation);
116+
expect(tmpdirExists).to.be.false;
117+
});
118+
});
119+
120+
context('after failing', function () {
121+
it('deletes the temp directory', async function () {
122+
const s = new Suite('test');
123+
s.task({
124+
documentPath: 'test/documents/array.json',
125+
library: 'bson@5',
126+
operation: 'deserialize',
127+
warmup: 100,
128+
iterations: 100,
129+
options: {}
130+
});
131+
132+
// bson throws error when passed array as top-level input
133+
await s.run();
134+
135+
const tmpdirExists = await exists(Suite.packageInstallLocation);
136+
expect(tmpdirExists).to.be.false;
137+
});
138+
});
139+
140+
context('when running multiple tasks', function () {
141+
const counts = { makeInstallLocation: 0, cleanUpInstallLocation: 0 };
142+
class SuiteCounter extends Suite {
143+
constructor(n: string) {
144+
super(n);
145+
}
146+
147+
async makeInstallLocation() {
148+
counts.makeInstallLocation++;
149+
return await super.makeInstallLocation();
150+
}
151+
152+
async cleanUpInstallLocation() {
153+
counts.cleanUpInstallLocation++;
154+
return await super.cleanUpInstallLocation();
155+
}
156+
}
157+
158+
let suite: SuiteCounter;
159+
before(async function () {
160+
suite = new SuiteCounter('test');
161+
const benchmark = {
162+
documentPath: 'test/documents/long_largeArray.json',
163+
warmup: 10,
164+
iterations: 10,
165+
library: '[email protected]',
166+
options: {}
167+
};
168+
suite
169+
.task({
170+
...benchmark,
171+
operation: 'serialize'
172+
})
173+
.task({
174+
...benchmark,
175+
operation: 'deserialize'
176+
});
177+
178+
await suite.run();
179+
});
180+
181+
it('creates the tmp directory exactly once', async function () {
182+
expect(counts.makeInstallLocation).to.equal(1);
183+
});
184+
185+
it('deletes the tmp directory exactly once', async function () {
186+
expect(counts.cleanUpInstallLocation).to.equal(1);
187+
});
188+
});
81189
});
82190

83191
describe('#writeResults()', function () {
@@ -87,7 +195,7 @@ describe('Suite', function () {
87195
documentPath: 'test/documents/long_largeArray.json',
88196
warmup: 10,
89197
iterations: 10,
90-
library: '[email protected].0',
198+
library: '[email protected].1',
91199
options: {},
92200
tags: ['test']
93201
};

0 commit comments

Comments
 (0)