Repository: mcollina/autocannon
Pull request #357 (2021-04-07 14:29:56)

Feat: Add a warmup option
### Adds a warmup option so that the server being tested can get up to speed before the stats start to be generated

Closes: https://github.com/mcollina/autocannon/issues/202

### To test:
To test add a warmup flag to the command line or in the config: Eg
```
node autocannon -c 3 -d -W [ -c 2 -d 2 ] http://localhost:5000
```
or
```
node autocannon -c 3 -d --warmup [ --connections 2 --duration 2 ] http://localhost:5000
```
- Check that it works in both command line and imported modes
- Check that additional flags are being correctly utilised and aliased 
- Check that the display on the command line whilst running is displaying the warmup
- Check that the final reported stats do not include the warmup period
- Due to refactoring in `lib/init` check that all calls to `init` work as expected.

### Notes
- Replaces `minimist` with `subarg` so that we can include subarguments such as `--warmup`. `subarg` uses `minimist` internally so the change is minimal - however `subarg` does not use nested aliases and so a new function (`lib/subargAliases`) has been created to enable this
- `lib/init` Has had some refactoring. This was due to the original implementation relying on recursive calling of `_init` which already took considerable cognitive load to trace through the possible paths. Adding a warmup path to the recursion would only have made this situation worse and more bug prone. Instead I have removed the recursive call to `_init` and created two new helper functions that call `_init` exactly as required. 


https://user-images.githubusercontent.com/4930551/114038133-841eae00-9879-11eb-8d1a-a9d2e30732fb.mov

mcollina: That would be really nice to have @simoneb! Could you make it so @skywickenden.
skywickenden: @simoneb @mcollina Unfortunately I don't think that can be achieved. `minimist` would require `-W` to be a boolean in all cases to make it parse without a value. Also, if I add a default value then it is used in all cases - even when `-W` is omitted, so a default warmup would always run. 

Patch:
diff --git a/README.md b/README.md
index 41cd579..e3824c3 100644
--- a/README.md
+++ b/README.md
@@ -52,6 +52,10 @@ Available options:
         A path to a Unix Domain Socket or a Windows Named Pipe. A URL is still required in order to send the correct Host header and path.
   -w/--workers
         Number of worker threads to use to fire requests.
+  -W/--warmup
+       Use a warm up interval before starting sampling.
+       This enables startup processes to finish and traffic to normalize before sampling begins
+       use -c and -d sub args e.g. `--warmpup [ -c 1 -d 3 ]`
   --on-port
         Start the command listed after -- on the command line. When it starts listening on a port,
         start sending requests to that port. A URL is still required in order to send requests to
@@ -118,6 +122,10 @@ Available options:
   -E/--expectBody EXPECTED
         Ensure the body matches this value. If enabled, mismatches count towards bailout.
         Enabling this option will slow down the load testing.
+  --renderStatusCodes
+        Print status codes and their respective statistics.
+  --debug
+        Print connection errors to stderr.        
   -v/--version
         Print the version number.
   -h/--help
diff --git a/autocannon.js b/autocannon.js
index 44be8f4..0c534ae 100755
--- a/autocannon.js
+++ b/autocannon.js
@@ -3,7 +3,6 @@
 'use strict'
 
 const crossArgv = require('cross-argv')
-const minimist = require('minimist')
 const fs = require('fs')
 const os = require('os')
 const net = require('net')
@@ -12,10 +11,12 @@ const URL = require('url').URL
 const spawn = require('child_process').spawn
 const managePath = require('manage-path')
 const hasAsyncHooks = require('has-async-hooks')
+const subarg = require('subarg')
 const help = fs.readFileSync(path.join(__dirname, 'help.txt'), 'utf8')
 const printResult = require('./lib/printResult')
 const initJob = require('./lib/init')
 const track = require('./lib/progressTracker')
+const generateSubArgAliases = require('./lib/subargAliases')
 const { checkURL, ofURL } = require('./lib/url')
 const { parseHAR } = require('./lib/parseHAR')
 
@@ -30,63 +31,69 @@ module.exports.track = track
 module.exports.start = start
 module.exports.printResult = printResult
 module.exports.parseArguments = parseArguments
+const alias = {
+  connections: 'c',
+  pipelining: 'p',
+  timeout: 't',
+  duration: 'd',
+  amount: 'a',
+  json: 'j',
+  renderLatencyTable: ['l', 'latency'],
+  onPort: 'on-port',
+  method: 'm',
+  headers: ['H', 'header'],
+  body: 'b',
+  form: 'F',
+  servername: 's',
+  bailout: 'B',
+  input: 'i',
+  maxConnectionRequests: 'M',
+  maxOverallRequests: 'O',
+  connectionRate: 'r',
+  overallRate: 'R',
+  ignoreCoordinatedOmission: 'C',
+  reconnectRate: 'D',
+  renderProgressBar: 'progress',
+  renderStatusCodes: 'statusCodes',
+  title: 'T',
+  version: 'v',
+  forever: 'f',
+  idReplacement: 'I',
+  socketPath: 'S',
+  excludeErrorStats: 'x',
+  expectBody: 'E',
+  workers: 'w',
+  warmup: 'W',
+  help: 'h'
+}
+
+const defaults = {
+  connections: 10,
+  timeout: 10,
+  pipelining: 1,
+  duration: 10,
+  reconnectRate: 0,
+  renderLatencyTable: false,
+  renderProgressBar: true,
+  renderStatusCodes: false,
+  json: false,
+  forever: false,
+  method: 'GET',
+  idReplacement: false,
+  excludeErrorStats: false,
+  debug: false,
+  workers: 0
+}
 
 function parseArguments (argvs) {
-  const argv = minimist(argvs, {
+  let argv = subarg(argvs, {
     boolean: ['json', 'n', 'help', 'renderLatencyTable', 'renderProgressBar', 'renderStatusCodes', 'forever', 'idReplacement', 'excludeErrorStats', 'onPort', 'debug', 'ignoreCoordinatedOmission'],
-    alias: {
-      connections: 'c',
-      pipelining: 'p',
-      timeout: 't',
-      duration: 'd',
-      amount: 'a',
-      json: 'j',
-      renderLatencyTable: ['l', 'latency'],
-      onPort: 'on-port',
-      method: 'm',
-      headers: ['H', 'header'],
-      body: 'b',
-      form: 'F',
-      servername: 's',
-      bailout: 'B',
-      input: 'i',
-      maxConnectionRequests: 'M',
-      maxOverallRequests: 'O',
-      connectionRate: 'r',
-      overallRate: 'R',
-      ignoreCoordinatedOmission: 'C',
-      reconnectRate: 'D',
-      renderProgressBar: 'progress',
-      renderStatusCodes: 'statusCodes',
-      title: 'T',
-      version: 'v',
-      forever: 'f',
-      idReplacement: 'I',
-      socketPath: 'S',
-      excludeErrorStats: 'x',
-      expectBody: 'E',
-      workers: 'w',
-      help: 'h'
-    },
-    default: {
-      connections: 10,
-      timeout: 10,
-      pipelining: 1,
-      duration: 10,
-      reconnectRate: 0,
-      renderLatencyTable: false,
-      renderProgressBar: true,
-      renderStatusCodes: false,
-      json: false,
-      forever: false,
-      method: 'GET',
-      idReplacement: false,
-      excludeErrorStats: false,
-      debug: false,
-      workers: 0
-    },
+    alias,
+    default: defaults,
     '--': true
   })
+  // subarg does not convert aliases in sub arguments
+  argv = generateSubArgAliases(argv)
 
   argv.url = argv._.length > 1 ? argv._ : argv._[0]
 
@@ -183,7 +190,7 @@ function parseArguments (argvs) {
   }
 
   // This is to distinguish down the line whether it is
-  // run via command-line or programatically
+  // run via command-line or programmatically
   argv[Symbol.for('internal')] = true
 
   return argv
diff --git a/help.txt b/help.txt
index e4fab21..aaee499 100644
--- a/help.txt
+++ b/help.txt
@@ -17,6 +17,10 @@ Available options:
         A path to a Unix Domain Socket or a Windows Named Pipe. A URL is still required in order to send the correct Host header and path.
   -w/--workers
         Number of worker threads to use to fire requests.
+  -W/--warmup
+       Use a warm up interval before starting sampling.
+       This enables startup processes to finish and traffic to normalize before sampling begins
+       use -c and -d sub args e.g. `--warmup [ -c 1 -d 3 ]`
   --on-port
         Start the command listed after -- on the command line. When it starts listening on a port,
         start sending requests to that port. A URL is still required in order to send requests to
diff --git a/lib/init.js b/lib/init.js
index 570ee32..4b45720 100644
--- a/lib/init.js
+++ b/lib/init.js
@@ -10,26 +10,61 @@ const runTracker = require('./runTracker')
 const track = require('./progressTracker')
 
 function init (opts, cb) {
-  return _init(opts, null, cb)
+  const cbPassedIn = (typeof cb === 'function')
+  if (!cbPassedIn && !opts.forever) {
+    if (opts.warmup) {
+      return runWithWarmup(opts)
+    } else {
+      return run(opts)
+    }
+  } else {
+    return _init(opts, null, cb)
+  }
 }
 
-function _init (opts, tracker, cb) {
-  const cbPassedIn = (typeof cb === 'function')
-  cb = cb || noop
+function run (opts) {
+  const tracker = new EE()
+  const promise = new Promise((resolve, reject) => {
+    _init(opts, tracker, (err, results) => {
+      if (err) return reject(err)
+      resolve(results)
+    })
+  })
+  tracker.then = promise.then.bind(promise)
+  tracker.catch = promise.catch.bind(promise)
 
-  tracker = tracker || new EE()
+  return tracker
+}
 
-  if (!cbPassedIn && !opts.forever) {
-    const promise = new Promise((resolve, reject) => {
-      _init(opts, tracker, (err, results) => {
+function runWithWarmup (opts) {
+  const warmupOpts = {
+    ...opts,
+    ...opts.warmup,
+    warmupRunning: true,
+    renderResultsTable: false
+  }
+  const mainTracker = new EE()
+  const warmUpTracker = new EE()
+  const promise = new Promise((resolve, reject) => {
+    _init(warmupOpts, warmUpTracker, (err, warmupResults) => {
+      if (err) return reject(err)
+      _init(opts, mainTracker, (err, results) => {
         if (err) return reject(err)
+        results.warmup = warmupResults
         resolve(results)
       })
     })
-    tracker.then = promise.then.bind(promise)
-    tracker.catch = promise.catch.bind(promise)
-    return tracker
-  }
+  })
+  mainTracker.then = promise.then.bind(promise)
+  mainTracker.catch = promise.catch.bind(promise)
+  return mainTracker
+}
+
+function _init (opts, tracker, cb) {
+  const cbPassedIn = (typeof cb === 'function')
+  cb = cb || noop
+
+  tracker = tracker || new EE()
 
   opts = validateOpts(opts, cbPassedIn)
 
diff --git a/lib/progressTracker.js b/lib/progressTracker.js
index 35c06e4..8613424 100644
--- a/lib/progressTracker.js
+++ b/lib/progressTracker.js
@@ -28,7 +28,8 @@ function track (instance, opts) {
 
   const chalk = new Chalk.Instance(testColorSupport({ stream: opts.outputStream, alwaysReturn: true }))
   // this default needs to be set after chalk is setup, because chalk is now local to this func
-  opts.progressBarString = opts.progressBarString || `${chalk.green('running')} [:bar] :percent`
+  const runningDescription = opts.warmupRunning ? 'warmup' : 'running'
+  opts.progressBarString = opts.progressBarString || `${chalk.green(runningDescription)} [:bar] :percent`
 
   const iOpts = instance.opts
   let durationProgressBar
@@ -49,9 +50,10 @@ function track (instance, opts) {
         msg += `\n${iOpts.workers} workers`
       }
 
+      const runningType = opts.warmupRunning ? 'warmup' : 'test'
       const message = iOpts.amount
-        ? `Running ${iOpts.amount} requests test @ ${iOpts.url}${socketPath}\n${msg}\n`
-        : `Running ${iOpts.duration}s test @ ${iOpts.url}${socketPath}\n${msg}\n`
+        ? `Running ${iOpts.amount} requests ${runningType} @ ${iOpts.url}${socketPath}\n${msg}\n`
+        : `Running ${iOpts.duration}s ${runningType} @ ${iOpts.url}${socketPath}\n${msg}\n`
 
       logToStream(message)
 
diff --git a/lib/subargAliases.js b/lib/subargAliases.js
new file mode 100644
index 0000000..d7f0bf3
--- /dev/null
+++ b/lib/subargAliases.js
@@ -0,0 +1,43 @@
+'use strict'
+
+const clone = require('clone')
+
+const subArgAlias = {
+  warmup: {
+    c: 'connections',
+    d: 'duration'
+  }
+}
+
+// Expects args to have already been processed by subarg
+function generateSubargAliases (args) {
+  const aliasedArgs = clone(args)
+
+  function isParentAliasInArgs (argKey) {
+    return aliasedArgs[argKey]
+  }
+
+  function isSubargAnAlias (parentAlias, subArg) {
+    return parentAlias[subArg]
+  }
+
+  function mapAliasForSubarg (parentAlias, parentKey) {
+    const parentArgs = aliasedArgs[parentKey]
+    for (const subArg in parentArgs) {
+      if (isSubargAnAlias(parentAlias, subArg)) {
+        parentArgs[parentAlias[subArg]] = parentArgs[subArg]
+      }
+    }
+  }
+
+  for (const parentKey in subArgAlias) {
+    const parentAlias = subArgAlias[parentKey]
+    if (isParentAliasInArgs(parentKey)) {
+      mapAliasForSubarg(parentAlias, parentKey)
+    }
+  }
+
+  return aliasedArgs
+}
+
+module.exports = generateSubargAliases
diff --git a/package.json b/package.json
index 88a5cf9..448c1c8 100644
--- a/package.json
+++ b/package.json
@@ -42,7 +42,7 @@
     "busboy": "^0.3.1",
     "pre-commit": "^1.1.2",
     "proxyquire": "^2.1.3",
-    "sinon": "^9.2.4",
+    "sinon": "^10.0.1",
     "split2": "^3.2.2",
     "standard": "^16.0.3",
     "tap": "^14.10.8"
@@ -61,13 +61,13 @@
     "http-parser-js": "^0.5.2",
     "hyperid": "^2.0.3",
     "manage-path": "^2.0.0",
-    "minimist": "^1.2.0",
     "on-net-listen": "^1.1.1",
     "pretty-bytes": "^5.4.1",
     "progress": "^2.0.3",
     "reinterval": "^1.1.0",
     "retimer": "^3.0.0",
     "semver": "^7.3.2",
+    "subarg": "^1.0.0",
     "timestring": "^6.0.0"
   }
 }


Test patch:
diff --git a/test/run.test.js b/test/run.test.js
index 0ec38cd..2d5a1ea 100644
--- a/test/run.test.js
+++ b/test/run.test.js
@@ -872,3 +872,48 @@ test('should throw on invalid HAR', (t) => {
     t.end()
   })
 })
+
+test('should run when no callback is passed in', (t) => {
+  t.plan(1)
+
+  const tracker = initJob({
+    url: 'http://localhost:' + server.address().port,
+    connections: 1,
+    duration: 1
+  })
+  t.resolveMatch(tracker, { connections: 1 }, 'The main tracker should resolve')
+})
+
+test('Should run a warmup if one is passed in', (t) => {
+  t.plan(1)
+
+  const tracker = initJob({
+    url: 'http://localhost:' + server.address().port,
+    connections: 1,
+    duration: 1,
+    warmup: {
+      connections: 1,
+      duration: 1
+    }
+  })
+  t.resolves(tracker, 'The main tracker should resolve')
+})
+
+test('The warmup should not pollute the main result set', (t) => {
+  t.plan(3)
+
+  const tracker = initJob({
+    url: 'http://localhost:' + server.address().port,
+    connections: 3,
+    duration: 1,
+    warmup: {
+      connections: 4,
+      duration: 2
+    }
+  })
+  tracker.then((result) => {
+    t.equal(result.connections, 3, 'connections should equal the main connections and not the warmup connections')
+    t.ok(result.duration >= 1, 'duration should equal the main duration and not the warmup duration')
+    t.type(result.warmup, 'object')
+  })
+})
diff --git a/test/subargAliases.test.js b/test/subargAliases.test.js
new file mode 100644
index 0000000..c74b894
--- /dev/null
+++ b/test/subargAliases.test.js
@@ -0,0 +1,44 @@
+'use strict'
+
+const test = require('tap').test
+const generateSubArgAliases = require('../lib/subargAliases')
+
+test('generateSubArgAliases Should generate warmup aliases', (t) => {
+  t.plan(4)
+
+  const args = {
+    connections: 1,
+    duration: 2,
+    warmup: {
+      c: 3,
+      d: 4
+    }
+  }
+
+  const result = generateSubArgAliases(args)
+
+  t.equal(result.connections, 1)
+  t.equal(result.duration, 2)
+  t.equal(result.warmup.connections, 3)
+  t.equal(result.warmup.duration, 4)
+})
+
+test('generateSubArgAliases should not process aliases that are not defined in subargAliases.js', (t) => {
+  t.plan(5)
+
+  const args = {
+    connections: 1,
+    warmup: {
+      c: 3,
+      T: 'A title'
+    }
+  }
+
+  const result = generateSubArgAliases(args)
+
+  t.equal(result.connections, 1)
+  t.equal(result.warmup.connections, 3)
+  t.equal(result.warmup.c, 3)
+  t.equal(result.warmup.T, 'A title')
+  t.equal(Object.keys(result.warmup).length, 3)
+})