test: Updated benchmark test results to output result files (#2350)

Signed-off-by: mrickard <[email protected]>
Co-authored-by: Bob Evans <[email protected]>

Author: mrickard

.gitignore

@@ -19,3 +19,5 @@ test/versioned-external/TEMP_TESTS
 nr-security-home
 # Needed for testing
 !test/integration/moduleLoading/node_modules
+# benchmark results
+benchmark_results

bin/compare-bench-results.js

@@ -27,7 +27,7 @@ const processFile = async (file) => {
   }
 }
 
-const reportResults = (resultFiles) => {
+const reportResults = async (resultFiles) => {
   const baseline = resultFiles[0]
   const downstream = resultFiles[1]
 
@@ -88,8 +88,7 @@ const reportResults = (resultFiles) => {
       allPassing = allPassing && filePassing
 
       return [
-        '<details>',
-        `<summary>${testFile}: ${passMark(filePassing)}</summary>`,
+        `#### ${testFile}: ${passMark(filePassing)}`,
         '',
         results,
         '',
@@ -105,11 +104,22 @@ const reportResults = (resultFiles) => {
     console.log('')
   }
 
-  console.log(`### Benchmark Results: ${passMark(allPassing)}`)
-  console.log('')
-  console.log('### Details')
-  console.log('_Lower is better._')
-  console.log(details)
+  const date = new Date()
+  let content = `### Benchmark Results: ${passMark(allPassing)}\n\n\n\n`
+  content += `${date.toISOString()}\n\n`
+  content += '### Details\n\n'
+  content += '_Lower is better._\n\n'
+  content += `${details}\n`
+
+  const resultPath = 'benchmark_results'
+  try {
+    await fs.stat(resultPath)
+  } catch (e) {
+    await fs.mkdir(resultPath)
+  }
+  const fileName = `${resultPath}/comparison_${date.getTime()}.md`
+  await fs.writeFile(fileName, content)
+  console.log(`Done! Benchmark test comparison written to ${fileName}`)
 
   if (!allPassing) {
     process.exitCode = -1
@@ -118,9 +128,11 @@ const reportResults = (resultFiles) => {
 
 const iterate = async () => {
   const files = process.argv.slice(2)
-  const results = files.map(async (file) => {
-    return processFile(file)
-  })
+  const results = await Promise.all(
+    files.map(async (file) => {
+      return await processFile(file)
+    })
+  )
   reportResults(results)
 }
 

bin/run-bench.js

@@ -5,29 +5,20 @@
 
 'use strict'
 
-/* eslint sonarjs/cognitive-complexity: ["error", 21] -- TODO: https://issues.newrelic.com/browse/NEWRELIC-5252 */
-
 const cp = require('child_process')
 const glob = require('glob')
 const path = require('path')
 const { errorAndExit } = require('./utils')
+const fs = require('fs/promises')
 
 const cwd = path.resolve(__dirname, '..')
 const benchpath = path.resolve(cwd, 'test/benchmark')
 
 const tests = []
+const testPromises = []
 const globs = []
 const opts = Object.create(null)
 
-// replacement for former async-lib cb
-const testCb = (err, payload) => {
-  if (err) {
-    console.error(err)
-    return
-  }
-  return payload
-}
-
 process.argv.slice(2).forEach(function forEachFileArg(file) {
   if (/^--/.test(file)) {
     opts[file.substring(2)] = true
@@ -48,105 +39,118 @@ if (tests.length === 0 && globs.length === 0) {
   globs.push(path.join(benchpath, '*.bench.js'), path.join(benchpath, '**/*.bench.js'))
 }
 
-class ConsolePrinter {
-  /* eslint-disable no-console */
-  addTest(name, child) {
-    console.log(name)
-    child.stdout.on('data', (d) => process.stdout.write(d))
-    child.stderr.on('data', (d) => process.stderr.write(d))
-    child.once('exit', () => console.log(''))
-  }
-
-  finish() {
-    console.log('')
-  }
-  /* eslint-enable no-console */
-}
-
-class JSONPrinter {
+class Printer {
   constructor() {
     this._tests = Object.create(null)
   }
 
   addTest(name, child) {
     let output = ''
-    this._tests[name] = null
     child.stdout.on('data', (d) => (output += d.toString()))
-    child.stdout.on('end', () => (this._tests[name] = JSON.parse(output)))
     child.stderr.on('data', (d) => process.stderr.write(d))
+
+    this._tests[name] = new Promise((resolve) => {
+      child.stdout.on('end', () => {
+        try {
+          this._tests[name] = JSON.parse(output)
+        } catch (e) {
+          console.error(`Error parsing test results for ${name}`, e)
+          this._tests[name] = output
+        }
+        resolve()
+      })
+    })
   }
 
-  finish() {
-    /* eslint-disable no-console */
-    console.log(JSON.stringify(this._tests, null, 2))
-    /* eslint-enable no-console */
+  async finish() {
+    if (opts.console) {
+      /* eslint-disable no-console */
+      console.log(JSON.stringify(this._tests, null, 2))
+      /* eslint-enable no-console */
+    }
+    const resultPath = 'benchmark_results'
+    try {
+      await fs.stat(resultPath)
+    } catch (e) {
+      await fs.mkdir(resultPath)
+    }
+    const content = JSON.stringify(this._tests, null, 2)
+    const fileName = `${resultPath}/benchmark_${new Date().getTime()}.json`
+    await fs.writeFile(fileName, content)
+    console.log(`Done! Test output written to ${fileName}`)
   }
 }
 
 run()
 
 async function run() {
-  const printer = opts.json ? new JSONPrinter() : new ConsolePrinter()
+  const printer = new Printer()
+  let currentTest = 0
 
-  const resolveGlobs = (cb) => {
+  const resolveGlobs = () => {
     if (!globs.length) {
-      cb()
+      console.error(`There aren't any globs to resolve.`)
+      return
     }
-    const afterGlobbing = (err, resolved) => {
-      if (err) {
-        errorAndExit(err, 'Failed to glob', -1)
-        cb(err)
+    const afterGlobbing = (resolved) => {
+      if (!resolved) {
+        return errorAndExit(new Error('Failed to glob'), 'Failed to glob', -1)
       }
-      resolved.forEach(function mergeResolved(files) {
-        files.forEach(function mergeFile(file) {
-          if (tests.indexOf(file) === -1) {
-            tests.push(file)
-          }
-        })
-      })
-      cb() // ambient scope
+
+      function mergeFile(file) {
+        if (tests.indexOf(file) === -1) {
+          tests.push(file)
+        }
+      }
+      function mergeResolved(files) {
+        files.forEach(mergeFile)
+      }
+
+      return resolved.forEach(mergeResolved)
     }
 
     const globbed = globs.map((item) => glob.sync(item))
-    return afterGlobbing(null, globbed)
+    return afterGlobbing(globbed)
   }
 
-  const spawnEachFile = (file, spawnCb) => {
+  const spawnEachFile = async (file) => {
     const test = path.relative(benchpath, file)
 
     const args = [file]
     if (opts.inspect) {
       args.unshift('--inspect-brk')
     }
 
-    const child = cp.spawn('node', args, { cwd: cwd, stdio: 'pipe' })
-    printer.addTest(test, child)
+    const child = cp.spawn('node', args, { cwd: cwd, stdio: 'pipe', silent: true })
 
-    child.on('error', spawnCb)
+    child.on('error', (err) => {
+      console.error(`Error in child test ${test}`, err)
+      throw err
+    })
     child.on('exit', function onChildExit(code) {
+      currentTest = currentTest + 1
       if (code) {
-        spawnCb(new Error('Benchmark exited with code ' + code))
+        console.error(`(${currentTest}/${tests.length}) FAILED: ${test} exited with code ${code}`)
+        return
       }
-      spawnCb()
+      console.log(`(${currentTest}/${tests.length}) ${file} has completed`)
     })
+    printer.addTest(test, child)
   }
 
-  const afterSpawnEachFile = (err, cb) => {
-    if (err) {
-      errorAndExit(err, 'Spawning failed:', -2)
-      return cb(err)
-    }
-    cb()
-  }
-
-  const runBenchmarks = async (cb) => {
+  const runBenchmarks = async () => {
     tests.sort()
-    await tests.forEach((file) => spawnEachFile(file, testCb))
-    await afterSpawnEachFile(null, testCb)
-    return cb()
+    for await (const file of tests) {
+      await spawnEachFile(file)
+    }
+    const keys = Object.keys(printer._tests)
+    for (const key of keys) {
+      testPromises.push(printer._tests[key])
+    }
   }
 
-  await resolveGlobs(testCb)
-  await runBenchmarks(testCb)
+  await resolveGlobs()
+  await runBenchmarks()
+  await Promise.all(testPromises)
   printer.finish()
 }

test/benchmark/async-hooks.bench.js

@@ -9,7 +9,6 @@ const benchmark = require('../lib/benchmark')
 
 const suite = benchmark.createBenchmark({
   name: 'async hooks',
-  async: true,
   fn: runBenchmark
 })
 
@@ -52,10 +51,11 @@ tests.forEach((test) => suite.add(test))
 
 suite.run()
 
-function runBenchmark(agent, cb) {
+function runBenchmark() {
   let p = Promise.resolve()
   for (let i = 0; i < 300; ++i) {
     p = p.then(function noop() {})
   }
-  p.then(cb)
+
+  return p
 }

test/benchmark/datastore-shim/recordBatch.bench.js

@@ -19,31 +19,34 @@ function makeInit(instrumented) {
 
 suite.add({
   name: 'instrumented operation',
-  async: true,
   initialize: makeInit(true),
   agent: {},
-  fn: function (agent, done) {
-    testDatastore.testBatch('test', done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testBatch('test', resolve)
+    })
   }
 })
 
 suite.add({
   name: 'uninstrumented operation',
   initialize: makeInit(false),
-  async: true,
-  fn: function (agent, done) {
-    testDatastore.testBatch('test', done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testBatch('test', resolve)
+    })
   }
 })
 
 suite.add({
   name: 'instrumented operation in transaction',
-  async: true,
   agent: {},
   initialize: makeInit(true),
   runInTransaction: true,
-  fn: function (agent, done) {
-    testDatastore.testBatch('test', done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testBatch('test', resolve)
+    })
   }
 })
 

test/benchmark/datastore-shim/recordOperation.bench.js

@@ -19,31 +19,34 @@ function makeInit(instrumented) {
 
 suite.add({
   name: 'instrumented operation in transaction',
-  async: true,
   agent: {},
   initialize: makeInit(true),
   runInTransaction: true,
-  fn: function (agent, done) {
-    testDatastore.testOp(done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testOp(resolve)
+    })
   }
 })
 
 suite.add({
   name: 'instrumented operation',
-  async: true,
   initialize: makeInit(true),
   agent: {},
-  fn: function (agent, done) {
-    testDatastore.testOp(done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testOp(resolve)
+    })
   }
 })
 
 suite.add({
   name: 'uninstrumented operation',
   initialize: makeInit(false),
-  async: true,
-  fn: function (agent, done) {
-    testDatastore.testOp(done)
+  fn: function () {
+    return new Promise((resolve) => {
+      testDatastore.testOp(resolve)
+    })
   }
 })