fix(NODE-6706): Move creation and deletion of install location

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,33 +34,51 @@ 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
    */
   async run(): Promise<void> {
     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();
+    }
   }
 
   /**

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<BenchmarkResult> {
     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');
     }
   }
 }

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: '[email protected]',
+          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: '[email protected].0',
+        library: '[email protected].1',
         options: {},
         tags: ['test']
       };