From 0eefbd9da7f9ce42a675722daac669d78f171abf Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 4 Mar 2025 13:33:23 -0500 Subject: [PATCH 1/2] Move tmpdir creation to Suite.run --- packages/bson-bench/src/suite.ts | 59 ++++++--- packages/bson-bench/src/task.ts | 73 +++++------ packages/bson-bench/test/unit/suite.test.ts | 130 ++++++++++++++++++-- packages/bson-bench/test/unit/task.test.ts | 78 ++---------- 4 files changed, 199 insertions(+), 141 deletions(-) diff --git a/packages/bson-bench/src/suite.ts b/packages/bson-bench/src/suite.ts index 639f7006..965327a5 100644 --- a/packages/bson-bench/src/suite.ts +++ b/packages/bson-bench/src/suite.ts @@ -1,7 +1,10 @@ -import { writeFile } from 'fs/promises'; +import { mkdir, rm, writeFile } from 'fs/promises'; +import { tmpdir } from 'os'; +import * as path from 'path'; import { type BenchmarkResult, type BenchmarkSpecification, type PerfSendResult } from './common'; import { Task } from './task'; +import { exists } from './utils'; /** * A collection of related Tasks @@ -10,8 +13,10 @@ export class Suite { tasks: Task[]; name: string; hasRun: boolean; + _errors: { task: Task; error: Error }[]; private _results: PerfSendResult[]; + static packageInstallLocation: string = path.join(tmpdir(), 'bsonBench'); constructor(name: string) { this.name = name; @@ -29,6 +34,16 @@ export class Suite { return this; } + async makeInstallLocation() { + if (!(await exists(Suite.packageInstallLocation))) { + await mkdir(Suite.packageInstallLocation); + } + } + + async cleanUpInstallLocation() { + await rm(Suite.packageInstallLocation, { recursive: true, force: true }); + } + /** * Run all Tasks. Throws error if suite has already been run * Collects all results and thrown errors from child Tasks @@ -36,26 +51,34 @@ export class Suite { async run(): Promise { if (this.hasRun) throw new Error('Suite has already been run'); - console.log(`Suite: ${this.name}`); - for (const task of this.tasks) { - const result = await task.run().then( - (_r: BenchmarkResult) => task.getResults(), - (e: Error) => e - ); - if (result instanceof Error) { - console.log(`\t${task.testName} ✗`); - this._errors.push({ task, error: result }); - } else { - console.log(`\t${task.testName} ✓`); - this._results.push(result); + try { + // install required modules before running child process as new Node processes need to know that + // it exists before they can require it. + await this.makeInstallLocation(); + + console.log(`Suite: ${this.name}`); + for (const task of this.tasks) { + const result = await task.run().then( + (_r: BenchmarkResult) => task.getResults(), + (e: Error) => e + ); + if (result instanceof Error) { + console.log(`\t${task.testName} ✗`); + this._errors.push({ task, error: result }); + } else { + console.log(`\t${task.testName} ✓`); + this._results.push(result); + } } - } - for (const { task, error } of this._errors) { - console.log(`Task ${task.taskName} failed with Error '${error.message}'`); - } + for (const { task, error } of this._errors) { + console.log(`Task ${task.taskName} failed with Error '${error.message}'`); + } - this.hasRun = true; + this.hasRun = true; + } finally { + await this.cleanUpInstallLocation(); + } } /** diff --git a/packages/bson-bench/src/task.ts b/packages/bson-bench/src/task.ts index d0f5ee67..ea33ff62 100644 --- a/packages/bson-bench/src/task.ts +++ b/packages/bson-bench/src/task.ts @@ -1,7 +1,6 @@ import { type ChildProcess, fork } from 'child_process'; import { once } from 'events'; -import { mkdir, rm, writeFile } from 'fs/promises'; -import { tmpdir } from 'os'; +import { writeFile } from 'fs/promises'; import * as path from 'path'; import { @@ -12,7 +11,7 @@ import { type PerfSendResult, type ResultMessage } from './common'; -import { exists } from './utils'; +import { Suite } from './suite'; /** * An individual benchmark task that runs in its own Node.js process @@ -27,13 +26,11 @@ export class Task { /** @internal */ hasRun: boolean; - static packageInstallLocation: string = path.join(tmpdir(), 'bsonBench'); - constructor(benchmarkSpec: BenchmarkSpecification) { this.result = undefined; this.children = []; this.hasRun = false; - this.benchmark = { ...benchmarkSpec, installLocation: Task.packageInstallLocation }; + this.benchmark = { ...benchmarkSpec, installLocation: Suite.packageInstallLocation }; this.taskName = `${path.basename(this.benchmark.documentPath, 'json')}_${ this.benchmark.operation @@ -172,43 +169,33 @@ export class Task { async run(): Promise { if (this.hasRun && this.result) return this.result; - // install required modules before running child process as new Node processes need to know that - // it exists before they can require it. - if (!(await exists(Task.packageInstallLocation))) { - await mkdir(Task.packageInstallLocation); - } - - try { - const pack = new Package(this.benchmark.library, Task.packageInstallLocation); - if (!pack.check()) await pack.install(); - // spawn child process - const child = fork(`${__dirname}/base`, { - stdio: ['inherit', 'inherit', 'inherit', 'ipc'], - serialization: 'advanced' - }); - child.send({ type: 'runBenchmark', benchmark: this.benchmark }); - this.children.push(child); - - // listen for results or error - const resultOrErrorPromise = once(child, 'message'); - // Wait for process to exit - const exit = once(child, 'exit'); - - const resultOrError: ResultMessage | ErrorMessage = (await resultOrErrorPromise)[0]; - await exit; - - this.hasRun = true; - switch (resultOrError.type) { - case 'returnResult': - this.result = resultOrError.result; - return resultOrError.result; - case 'returnError': - throw resultOrError.error; - default: - throw new Error('Unexpected result returned from child process'); - } - } finally { - await rm(Task.packageInstallLocation, { recursive: true, force: true }); + const pack = new Package(this.benchmark.library, Suite.packageInstallLocation); + if (!pack.check()) await pack.install(); + // spawn child process + const child = fork(`${__dirname}/base`, { + stdio: ['inherit', 'inherit', 'inherit', 'ipc'], + serialization: 'advanced' + }); + child.send({ type: 'runBenchmark', benchmark: this.benchmark }); + this.children.push(child); + + // listen for results or error + const resultOrErrorPromise = once(child, 'message'); + // Wait for process to exit + const exit = once(child, 'exit'); + + const resultOrError: ResultMessage | ErrorMessage = (await resultOrErrorPromise)[0]; + await exit; + + this.hasRun = true; + switch (resultOrError.type) { + case 'returnResult': + this.result = resultOrError.result; + return resultOrError.result; + case 'returnError': + throw resultOrError.error; + default: + throw new Error('Unexpected result returned from child process'); } } } diff --git a/packages/bson-bench/test/unit/suite.test.ts b/packages/bson-bench/test/unit/suite.test.ts index 30f6fff4..69459d65 100644 --- a/packages/bson-bench/test/unit/suite.test.ts +++ b/packages/bson-bench/test/unit/suite.test.ts @@ -1,19 +1,10 @@ import { expect } from 'chai'; import { readFile } from 'fs/promises'; -import { Suite, Task } from '../../lib'; +import { Suite } from '../../lib'; import { exists } from '../../src/utils'; -import { clearTestedDeps } from '../utils'; describe('Suite', function () { - beforeEach(async function () { - await clearTestedDeps(Task.packageInstallLocation); - }); - - after(async function () { - await clearTestedDeps(Task.packageInstallLocation); - }); - describe('#task()', function () { it('returns the Suite it was called on', function () { const suite = new Suite('test'); @@ -78,6 +69,123 @@ describe('Suite', function () { expect(await suite.run().catch(e => e)).to.be.instanceOf(Error); }); }); + + it('creates a temp directory for packages', async function () { + const s = new Suite('test'); + s.task({ + documentPath: 'test/documents/long_largeArray.json', + library: 'bson@5', + operation: 'deserialize', + warmup: 100, + iterations: 10000, + options: {} + }); + + const checkForDirectory = async () => { + for (let i = 0; i < 10; i++) { + if (await exists(Suite.packageInstallLocation)) return true; + } + return false; + }; + const suiteRunPromise = s.run().catch(e => e); + + const result = await Promise.race([checkForDirectory(), suiteRunPromise]); + expect(typeof result).to.equal('boolean'); + expect(result).to.be.true; + + const suiteRunResult = await suiteRunPromise; + expect(suiteRunResult).to.not.be.instanceOf(Error); + }); + + context('after completing successfully', function () { + it('deletes the temp directory', async function () { + const s = new Suite('test'); + s.task({ + documentPath: 'test/documents/long_largeArray.json', + library: 'bson@5', + operation: 'deserialize', + warmup: 100, + iterations: 100, + options: {} + }); + + const maybeError = await s.run().catch(e => e); + expect(maybeError).to.not.be.instanceOf(Error); + + const tmpdirExists = await exists(Suite.packageInstallLocation); + expect(tmpdirExists).to.be.false; + }); + }); + + context('after failing', function () { + it('deletes the temp directory', async function () { + const s = new Suite('test'); + s.task({ + documentPath: 'test/documents/array.json', + library: 'bson@5', + operation: 'deserialize', + warmup: 100, + iterations: 100, + options: {} + }); + + // bson throws error when passed array as top-level input + await s.run(); + + const tmpdirExists = await exists(Suite.packageInstallLocation); + expect(tmpdirExists).to.be.false; + }); + }); + + context('when running multiple tasks', function () { + const counts = { makeInstallLocation: 0, cleanUpInstallLocation: 0 }; + class SuiteCounter extends Suite { + constructor(n: string) { + super(n); + } + + async makeInstallLocation() { + counts.makeInstallLocation++; + return await super.makeInstallLocation(); + } + + async cleanUpInstallLocation() { + counts.cleanUpInstallLocation++; + return await super.cleanUpInstallLocation(); + } + } + + let suite: SuiteCounter; + before(async function () { + suite = new SuiteCounter('test'); + const benchmark = { + documentPath: 'test/documents/long_largeArray.json', + warmup: 10, + iterations: 10, + library: 'bson@5.0.0', + options: {} + }; + suite + .task({ + ...benchmark, + operation: 'serialize' + }) + .task({ + ...benchmark, + operation: 'deserialize' + }); + + await suite.run(); + }); + + it('creates the tmp directory exactly once', async function () { + expect(counts.makeInstallLocation).to.equal(1); + }); + + it('deletes the tmp directory exactly once', async function () { + expect(counts.cleanUpInstallLocation).to.equal(1); + }); + }); }); describe('#writeResults()', function () { @@ -87,7 +195,7 @@ describe('Suite', function () { documentPath: 'test/documents/long_largeArray.json', warmup: 10, iterations: 10, - library: 'bson@5.0.0', + library: 'bson@5.0.1', options: {}, tags: ['test'] }; diff --git a/packages/bson-bench/test/unit/task.test.ts b/packages/bson-bench/test/unit/task.test.ts index 64e2ad6f..b84753ac 100644 --- a/packages/bson-bench/test/unit/task.test.ts +++ b/packages/bson-bench/test/unit/task.test.ts @@ -1,7 +1,8 @@ import { expect } from 'chai'; -import { rm } from 'fs/promises'; +import { mkdir, rm } from 'fs/promises'; import * as path from 'path'; +import { Suite } from '../../lib/suite'; import { Task } from '../../lib/task'; import { type BenchmarkSpecification, type PerfSendResult } from '../../src/common'; import { exists } from '../../src/utils'; @@ -10,17 +11,21 @@ import { clearTestedDeps } from '../utils'; const LOCAL_BSON = path.join(__dirname, '..', '..', 'node_modules', 'bson'); describe('Task', function () { + before(async function () { + if (!(await exists(Suite.packageInstallLocation))) await mkdir(Suite.packageInstallLocation); + }); + beforeEach(async function () { - await clearTestedDeps(Task.packageInstallLocation); + await clearTestedDeps(Suite.packageInstallLocation); }); after(async function () { - await clearTestedDeps(Task.packageInstallLocation); + await rm(Suite.packageInstallLocation, { recursive: true, force: true }); }); const BSON_PATH = process.env.BSON_PATH; const BSON_EXT_PATH = process.env.BSON_EXT_PATH; - const versions = ['bson@6.2.0', 'bson@4.0.0', 'bson@5.0.0', 'bson#v6.1.0', `bson:${LOCAL_BSON}`]; + const versions = ['bson@6.2.0', 'bson@4.0.0', 'bson@5.0.1', 'bson#v6.1.0', `bson:${LOCAL_BSON}`]; const operations: ('serialize' | 'deserialize')[] = ['serialize', 'deserialize']; if (BSON_PATH) versions.push(`bson:${BSON_PATH}`); if (BSON_EXT_PATH) versions.push(`bson:${BSON_EXT_PATH}`); @@ -99,71 +104,6 @@ describe('Task', function () { expect(maybeError).to.be.instanceOf(Error); expect(maybeError).to.have.property('message', 'failed to serialize input object'); }); - - it('deletes the temp directory', async function () { - const task = new Task({ - documentPath: 'test/documents/array.json', - library: 'bson@5', - operation: 'deserialize', - warmup: 100, - iterations: 100, - options: {} - }); - - // bson throws error when passed array as top-level input - const maybeError = await task.run().catch(e => e); - - expect(maybeError).to.be.instanceOf(Error); - expect(maybeError).to.have.property('message', 'failed to serialize input object'); - - const tmpdirExists = await exists(Task.packageInstallLocation); - expect(tmpdirExists).to.be.false; - }); - }); - - it('creates a temp directory for packages', async function () { - const task = new Task({ - documentPath: 'test/documents/long_largeArray.json', - library: 'bson@5', - operation: 'deserialize', - warmup: 100, - iterations: 10000, - options: {} - }); - - const checkForDirectory = async () => { - for (let i = 0; i < 10; i++) { - if (await exists(Task.packageInstallLocation)) return true; - } - return false; - }; - const taskRunPromise = task.run().catch(e => e); - - const result = await Promise.race([checkForDirectory(), taskRunPromise]); - expect(typeof result).to.equal('boolean'); - expect(result).to.be.true; - - const taskRunResult = await taskRunPromise; - expect(taskRunResult).to.not.be.instanceOf(Error); - }); - - context('after completing successfully', function () { - it('deletes the temp directory', async function () { - const task = new Task({ - documentPath: 'test/documents/long_largeArray.json', - library: 'bson@5', - operation: 'deserialize', - warmup: 100, - iterations: 100, - options: {} - }); - - const maybeError = await task.run().catch(e => e); - expect(maybeError).to.not.be.instanceOf(Error); - - const tmpdirExists = await exists(Task.packageInstallLocation); - expect(tmpdirExists).to.be.false; - }); }); }); From b00f102508b4fbecb94265829b14ae115f57c111 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 4 Mar 2025 15:01:02 -0500 Subject: [PATCH 2/2] restore test version --- packages/bson-bench/test/unit/task.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/bson-bench/test/unit/task.test.ts b/packages/bson-bench/test/unit/task.test.ts index b84753ac..cd2d6a7b 100644 --- a/packages/bson-bench/test/unit/task.test.ts +++ b/packages/bson-bench/test/unit/task.test.ts @@ -25,7 +25,7 @@ describe('Task', function () { const BSON_PATH = process.env.BSON_PATH; const BSON_EXT_PATH = process.env.BSON_EXT_PATH; - const versions = ['bson@6.2.0', 'bson@4.0.0', 'bson@5.0.1', 'bson#v6.1.0', `bson:${LOCAL_BSON}`]; + const versions = ['bson@6.2.0', 'bson@4.0.0', 'bson@5.0.0', 'bson#v6.1.0', `bson:${LOCAL_BSON}`]; const operations: ('serialize' | 'deserialize')[] = ['serialize', 'deserialize']; if (BSON_PATH) versions.push(`bson:${BSON_PATH}`); if (BSON_EXT_PATH) versions.push(`bson:${BSON_EXT_PATH}`);