From c61aa3191e1158065377c7b8617dd7c2e5a738ef Mon Sep 17 00:00:00 2001 From: Warren James Date: Fri, 6 Dec 2024 14:56:28 -0500 Subject: [PATCH] fix(NODE-6606): install bson libraries to tmp directory (#24) --- packages/bson-bench/src/base.ts | 7 +- packages/bson-bench/src/common.ts | 22 +++--- packages/bson-bench/src/task.ts | 70 ++++++++++-------- packages/bson-bench/test/unit/common.test.ts | 73 +++++++++++++------ packages/bson-bench/test/unit/suite.test.ts | 11 ++- packages/bson-bench/test/unit/task.test.ts | 74 +++++++++++++++++++- packages/bson-bench/test/utils.ts | 20 +++--- 7 files changed, 204 insertions(+), 73 deletions(-) diff --git a/packages/bson-bench/src/base.ts b/packages/bson-bench/src/base.ts index 6012eab6..3b758b51 100644 --- a/packages/bson-bench/src/base.ts +++ b/packages/bson-bench/src/base.ts @@ -1,5 +1,6 @@ import * as BSON from 'bson'; import { readFileSync } from 'fs'; +import { join } from 'path'; import { performance } from 'perf_hooks'; import * as process from 'process'; @@ -116,10 +117,12 @@ function run(bson: BSONLib | ConstructibleBSON, config: BenchmarkSpecification) function listener(message: RunBenchmarkMessage) { if (message.type === 'runBenchmark') { - const packageSpec = new Package(message.benchmark.library); + const packageSpec = new Package(message.benchmark.library, message.benchmark.installLocation); let bson: BSONLib; try { - bson = require(packageSpec.computedModuleName); + bson = require( + join(message.benchmark.installLocation, 'node_modules', packageSpec.computedModuleName) + ); } catch (error) { reportErrorAndQuit(error as Error); return; diff --git a/packages/bson-bench/src/common.ts b/packages/bson-bench/src/common.ts index acca78a1..9ac1dd60 100644 --- a/packages/bson-bench/src/common.ts +++ b/packages/bson-bench/src/common.ts @@ -1,6 +1,6 @@ import * as cp from 'child_process'; import { once } from 'events'; -import * as path from 'path'; +import { join } from 'path'; import { exists } from './utils'; @@ -26,8 +26,10 @@ export class Package { gitCommitish?: string; // path to local library localPath?: string; + installPath: string; - constructor(libSpec: string) { + constructor(libSpec: string, installPath: string) { + this.installPath = installPath; let match: RegExpExecArray | null; if ((match = NPM_PACKAGE_REGEX.exec(libSpec))) { this.type = 'npm'; @@ -44,7 +46,8 @@ export class Package { this.library = match[1] as 'bson' | 'bson-ext'; this.localPath = match[2]; - this.computedModuleName = `${this.library}-local-${this.localPath.replaceAll(path.sep, '_')}`; + const cleanedLocalPath = this.localPath.replaceAll('/', '_').replaceAll('\\', '_'); + this.computedModuleName = `${this.library}-local-${cleanedLocalPath}`; } else { throw new Error('unknown package specifier'); } @@ -55,7 +58,7 @@ export class Package { */ check(): B | undefined { try { - return require(this.computedModuleName); + return require(join(this.installPath, 'node_modules', this.computedModuleName)); } catch { return undefined; } @@ -90,10 +93,10 @@ export class Package { break; } - const npmInstallProcess = cp.exec( - `npm install ${this.computedModuleName}@${source} --no-save`, - { encoding: 'utf8', cwd: __dirname } - ); + const npmInstallProcess = cp.exec(`npm install ${this.computedModuleName}@${source}`, { + encoding: 'utf8', + cwd: this.installPath + }); const exitCode: number = (await once(npmInstallProcess, 'exit'))[0]; if (exitCode !== 0) { @@ -130,11 +133,12 @@ export type BenchmarkSpecification = { /** Specifier of the bson or bson-ext library to be used. Can be an npm package, git repository or * local package */ library: string; + installLocation?: string; }; export interface RunBenchmarkMessage { type: 'runBenchmark'; - benchmark: BenchmarkSpecification; + benchmark: Omit & { installLocation: string }; } export interface ResultMessage { diff --git a/packages/bson-bench/src/task.ts b/packages/bson-bench/src/task.ts index 98d5a71a..201e53e6 100644 --- a/packages/bson-bench/src/task.ts +++ b/packages/bson-bench/src/task.ts @@ -1,6 +1,7 @@ import { type ChildProcess, fork } from 'child_process'; import { once } from 'events'; -import { writeFile } from 'fs/promises'; +import { mkdir, rm, writeFile } from 'fs/promises'; +import { tmpdir } from 'os'; import * as path from 'path'; import { @@ -11,13 +12,14 @@ import { type PerfSendResult, type ResultMessage } from './common'; +import { exists } from './utils'; /** * An individual benchmark task that runs in its own Node.js process */ export class Task { result: BenchmarkResult | undefined; - benchmark: BenchmarkSpecification; + benchmark: Omit & { installLocation: string }; taskName: string; testName: string; /** @internal */ @@ -25,11 +27,13 @@ export class Task { /** @internal */ hasRun: boolean; + static packageInstallLocation: string = path.join(tmpdir(), 'bsonBench'); + constructor(benchmarkSpec: BenchmarkSpecification) { this.result = undefined; - this.benchmark = benchmarkSpec; this.children = []; this.hasRun = false; + this.benchmark = { ...benchmarkSpec, installLocation: Task.packageInstallLocation }; this.taskName = `${path.basename(this.benchmark.documentPath, 'json')}_${ this.benchmark.operation @@ -174,31 +178,41 @@ export class Task { // install required modules before running child process as new Node processes need to know that // it exists before they can require it. - const pack = new Package(this.benchmark.library); - 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 resultOrError: ResultMessage | ErrorMessage = (await once(child, 'message'))[0]; - - // wait for child to close - await once(child, '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'); + 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 }); } } } diff --git a/packages/bson-bench/test/unit/common.test.ts b/packages/bson-bench/test/unit/common.test.ts index 5af8f2cd..270b5abb 100644 --- a/packages/bson-bench/test/unit/common.test.ts +++ b/packages/bson-bench/test/unit/common.test.ts @@ -1,27 +1,42 @@ import { expect } from 'chai'; -import { sep } from 'path'; +import { mkdir, rm } from 'fs/promises'; +import { tmpdir } from 'os'; +import { join, sep } from 'path'; import { Package } from '../../lib/common'; -import { clearTestedDeps } from '../utils'; +import { clearTestedDeps, exists } from '../utils'; describe('common functionality', function () { const BSON_PATH = process.env.BSON_PATH; context('Package', function () { - beforeEach(clearTestedDeps); - after(clearTestedDeps); + let installDir: string; + + after(async function () { + await rm(installDir, { recursive: true, force: true }); + }); + + beforeEach(async function () { + await clearTestedDeps(installDir); + }); + + before(async function () { + installDir = join(tmpdir(), 'bsonBenchTest'); + await mkdir(installDir); + }); context('constructor()', function () { + //github.com/mongodb-js/dbx-js-tools/pull/24/files context('when given a correctly formatted npm package', function () { it('sets computedModuleName correctly', function () { - const pack = new Package('bson@6.0.0'); + const pack = new Package('bson@6.0.0', installDir); expect(pack).to.haveOwnProperty('computedModuleName', 'bson-6.0.0'); }); }); context('when given a correctly formatted git repository', function () { it('sets computedModuleName correctly', function () { - const pack = new Package('bson#eb98b8c39d6d5ba4ce7231ab9e0f29495d74b994'); + const pack = new Package('bson#eb98b8c39d6d5ba4ce7231ab9e0f29495d74b994', installDir); expect(pack).to.haveOwnProperty( 'computedModuleName', 'bson-git-eb98b8c39d6d5ba4ce7231ab9e0f29495d74b994' @@ -31,13 +46,16 @@ describe('common functionality', function () { context('when trying to install an npm package apart from bson or bson-ext', function () { it('throws an error', function () { - expect(() => new Package('notBson@1.0.0')).to.throw(Error, /unknown package specifier/); + expect(() => new Package('notBson@1.0.0', installDir)).to.throw( + Error, + /unknown package specifier/ + ); }); }); context('when trying to install a git package apart from bson or bson-ext', function () { it('throws an error', function () { - expect(() => new Package('notBson#abcdabcdabcd')).to.throw( + expect(() => new Package('notBson#abcdabcdabcd', installDir)).to.throw( Error, /unknown package specifier/ ); @@ -50,7 +68,7 @@ describe('common functionality', function () { console.log('Skipping since BSON_PATH is undefined'); this.skip(); } - const pack = new Package(`bson:${BSON_PATH}`); + const pack = new Package(`bson:${BSON_PATH}`, installDir); expect(pack).to.haveOwnProperty( 'computedModuleName', `bson-local-${BSON_PATH.replaceAll(sep, '_')}` @@ -62,14 +80,14 @@ describe('common functionality', function () { context('#check()', function () { context('when package is not installed', function () { it('returns undefined', function () { - const pack = new Package('bson@6'); + const pack = new Package('bson@6', installDir); expect(pack.check()).to.be.undefined; }); }); context('when package is installed', function () { it('returns the module', async function () { - const pack = new Package('bson@6.0.0'); + const pack = new Package('bson@6.0.0', installDir); await pack.install(); expect(pack.check()).to.not.be.undefined; }); @@ -79,26 +97,31 @@ describe('common functionality', function () { context('#install()', function () { context('when given a correctly formatted npm package that exists', function () { for (const lib of ['bson@6.0.0', 'bson-ext@4.0.0', 'bson@latest', 'bson-ext@latest']) { - it(`installs ${lib} successfully`, async function () { - const pack = new Package(lib); + it(`installs ${lib} successfully to the specified install directory`, async function () { + const pack = new Package(lib, installDir); await pack.install(); + + expect(await exists(join(installDir, 'node_modules', pack.computedModuleName))).to.be + .true; }); } }); context('when given a correctly formatted npm package that does not exist', function () { it('throws an error', async function () { - const bson9000 = new Package('bson@9000'); + const bson9000 = new Package('bson@9000', installDir); const error = await bson9000.install().catch(error => error); expect(error).to.be.instanceOf(Error); }); }); context('when given a correctly formatted git package using commit that exists', function () { - it('installs successfully', async function () { - const bson6Git = new Package('bson#58c002d'); + it('installs successfully to specified install directory', async function () { + const bson6Git = new Package('bson#58c002d', installDir); const maybeError = await bson6Git.install().catch(error => error); expect(maybeError).to.be.undefined; + expect(await exists(join(installDir, 'node_modules', bson6Git.computedModuleName))).to.be + .true; }); }); @@ -107,7 +130,10 @@ describe('common functionality', function () { function () { // TODO: NODE-6361: Unskip and fix this test. it.skip('throws an error', async function () { - const bson6Git = new Package('bson#58c002d87bca9bbe7c7001cc6acae54e90a951bcf'); + const bson6Git = new Package( + 'bson#58c002d87bca9bbe7c7001cc6acae54e90a951bcf', + installDir + ); const maybeError = await bson6Git.install().catch(error => error); expect(maybeError).to.be.instanceOf(Error); }); @@ -118,9 +144,11 @@ describe('common functionality', function () { 'when given a correctly formatted git package using git tag that exists', function () { it('installs successfully', async function () { - const bson6Git = new Package('bson#v6.0.0'); + const bson6Git = new Package('bson#v6.0.0', installDir); const maybeError = await bson6Git.install().catch(error => error); expect(maybeError).to.be.undefined; + expect(await exists(join(installDir, 'node_modules', bson6Git.computedModuleName))).to + .be.true; }); } ); @@ -129,7 +157,7 @@ describe('common functionality', function () { 'when given a correctly formatted git package using git tag that does not exist', function () { it('throws an error', async function () { - const bson6Git = new Package('bson#v999.999.9'); + const bson6Git = new Package('bson#v999.999.9', installDir); const maybeError = await bson6Git.install().catch(error => error); expect(maybeError).to.be.instanceOf(Error); }); @@ -143,16 +171,19 @@ describe('common functionality', function () { this.skip(); } - const bsonLocal = new Package(`bson:${BSON_PATH}`); + const bsonLocal = new Package(`bson:${BSON_PATH}`, installDir); const maybeError = await bsonLocal.install().catch(error => error); expect(maybeError).to.not.be.instanceOf(Error, maybeError.message); + expect(await exists(join(installDir, 'node_modules', bsonLocal.computedModuleName))).to.be + .true; }); }); context('when given a path that does not exist', function () { it('throws an error', async function () { const bsonLocal = new Package( - `bson:/highly/unlikely/path/to/exist/that/should/point/to/bson` + `bson:/highly/unlikely/path/to/exist/that/should/point/to/bson`, + installDir ); const maybeError = await bsonLocal.install().catch(error => error); expect(maybeError).to.be.instanceOf(Error); diff --git a/packages/bson-bench/test/unit/suite.test.ts b/packages/bson-bench/test/unit/suite.test.ts index 32db20f8..6dae328a 100644 --- a/packages/bson-bench/test/unit/suite.test.ts +++ b/packages/bson-bench/test/unit/suite.test.ts @@ -1,13 +1,18 @@ import { expect } from 'chai'; import { readFile } from 'fs/promises'; -import { Suite } from '../../lib'; +import { Suite, Task } from '../../lib'; import { exists } from '../../src/utils'; import { clearTestedDeps } from '../utils'; describe('Suite', function () { - beforeEach(clearTestedDeps); - after(clearTestedDeps); + 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 () { diff --git a/packages/bson-bench/test/unit/task.test.ts b/packages/bson-bench/test/unit/task.test.ts index fc789d68..4d197604 100644 --- a/packages/bson-bench/test/unit/task.test.ts +++ b/packages/bson-bench/test/unit/task.test.ts @@ -10,8 +10,13 @@ import { clearTestedDeps } from '../utils'; const LOCAL_BSON = path.join(__dirname, '..', '..', 'node_modules', 'bson'); describe('Task', function () { - beforeEach(clearTestedDeps); - after(clearTestedDeps); + beforeEach(async function () { + await clearTestedDeps(Task.packageInstallLocation); + }); + + after(async function () { + await clearTestedDeps(Task.packageInstallLocation); + }); const BSON_PATH = process.env.BSON_PATH; const BSON_EXT_PATH = process.env.BSON_EXT_PATH; @@ -110,6 +115,71 @@ 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; + }); }); }); diff --git a/packages/bson-bench/test/utils.ts b/packages/bson-bench/test/utils.ts index f5d5c246..78cda50a 100644 --- a/packages/bson-bench/test/utils.ts +++ b/packages/bson-bench/test/utils.ts @@ -1,18 +1,22 @@ import * as fs from 'fs/promises'; import * as path from 'path'; +import { exists } from '../src/utils'; + export { exists } from '../src/utils'; /** * Remove all installed bson and bson-ext versions that have been installed by tests */ -export async function clearTestedDeps() { - for await (const dirent of await fs.opendir('../../node_modules')) { - if (/^(bson-ext|bson)-(git|local)?.*$/.test(dirent.name)) { - await fs.rm(path.join('../../node_modules', dirent.name), { - recursive: true, - force: true - }); +export async function clearTestedDeps(installDir: string) { + const targetDir = path.join(installDir, 'node_modules'); + if (await exists(targetDir)) + for await (const dirent of await fs.opendir(targetDir)) { + if (/^(bson-ext|bson)-(git|local)?.*$/.test(dirent.name)) { + await fs.rm(path.join(targetDir, dirent.name), { + recursive: true, + force: true + }); + } } - } }