Repository: mcollina/autocannon
Pull request #516 (2024-03-13 20:54:31)

Update README to make behavior of idReplacement more clear
As a user of Autocannon, it took far too long to figure out why the idReplacement system was not working. Ultimately it was the source code which revealed that replacement target was, _in its entirety_, `[<id>]`. It's a bit unusual to have a two character wrapper where both characters are different, so we interpreted the square braces as a code container in the same way backticks might be used. It appears in other parts of the README, you've used backticks to better represent that the entire string is the target. So I've updated the command line example with this as well.


Patch:
diff --git a/README.md b/README.md
index 579d2a7..dac38ed 100644
--- a/README.md
+++ b/README.md
@@ -113,7 +113,7 @@ Available options:
   -l/--latency
         Print all the latency data. default: false.
   -I/--idReplacement
-        Enable replacement of [<id>] with a randomly generated ID within the request body. default: false.
+        Enable replacement of `[<id>]` with a randomly generated ID within the request body. e.g. `/items/[<id>]`. default: false.
   -j/--json
         Print the output as newline delimited JSON. This will cause the progress bar and results not to be rendered. default: false.
   -f/--forever
diff --git a/lib/printResult.js b/lib/printResult.js
index 5b3b096..a2fed8e 100644
--- a/lib/printResult.js
+++ b/lib/printResult.js
@@ -15,20 +15,27 @@ const defaults = {
   verbose: true
 }
 
+class TableWithoutColor extends Table {
+  constructor (opts = {}) {
+    super({ ...opts, style: { head: [], border: [] } })
+  }
+}
+
 const printResult = (result, opts) => {
   opts = Object.assign({}, defaults, opts)
   let strResult = ''
 
   if (opts.verbose) {
     const chalk = new Chalk.Instance(testColorSupport({ stream: opts.outputStream, alwaysReturn: true }))
+    const ColorSafeTable = chalk.level === 0 ? TableWithoutColor : Table
 
-    const shortLatency = new Table({
+    const shortLatency = new ColorSafeTable({
       head: asColor(chalk.cyan, ['Stat', '2.5%', '50%', '97.5%', '99%', 'Avg', 'Stdev', 'Max'])
     })
     shortLatency.push(asLowRow(chalk.bold('Latency'), asMs(result.latency)))
     logToLocalStr('\n' + shortLatency.toString())
 
-    const requests = new Table({
+    const requests = new ColorSafeTable({
       head: asColor(chalk.cyan, ['Stat', '1%', '2.5%', '50%', '97.5%', 'Avg', 'Stdev', 'Min'])
     })
 
@@ -37,7 +44,7 @@ const printResult = (result, opts) => {
     logToLocalStr(requests.toString())
 
     if (opts.renderStatusCodes === true) {
-      const statusCodeStats = new Table({
+      const statusCodeStats = new ColorSafeTable({
         head: asColor(chalk.cyan, ['Code', 'Count'])
       })
       Object.keys(result.statusCodeStats).forEach(statusCode => {
@@ -57,7 +64,7 @@ const printResult = (result, opts) => {
     logToLocalStr('')
 
     if (opts.renderLatencyTable) {
-      const latencies = new Table({
+      const latencies = new ColorSafeTable({
         head: asColor(chalk.cyan, ['Percentile', 'Latency (ms)'])
       })
       percentiles.map((perc) => {
@@ -85,7 +92,7 @@ const printResult = (result, opts) => {
     logToLocalStr(`${format(result.mismatches)} requests with mismatched body`)
   }
   if (result.resets) {
-    logToLocalStr(`request pipeline was resetted ${format(result.resets)} ${result.resets === 1 ? 'time' : 'times'}`)
+    logToLocalStr(`request pipeline was reset ${format(result.resets)} ${result.resets === 1 ? 'time' : 'times'}`)
   }
 
   function logToLocalStr (msg) {
diff --git a/lib/requestIterator.js b/lib/requestIterator.js
index 7886e59..5c1d27e 100644
--- a/lib/requestIterator.js
+++ b/lib/requestIterator.js
@@ -1,6 +1,6 @@
 'use strict'
 
-const hyperid = require('hyperid')(true)
+const hyperid = require('hyperid')
 const inherits = require('util').inherits
 const requestBuilder = require('./httpRequestBuilder')
 const clone = require('lodash.clonedeep')
@@ -26,6 +26,7 @@ function RequestIterator (opts) {
     return new RequestIterator(opts)
   }
 
+  this.hyperid = hyperid({ urlSafe: true })
   this.resetted = false
   this.headers = {}
   this.initialContext = opts.initialContext || {}
@@ -108,7 +109,7 @@ RequestIterator.prototype.rebuildRequest = function () {
     data = this.requestBuilder(this.currentRequest, this.context)
     if (data) {
       this.currentRequest.requestBuffer = this.reqDefaults.idReplacement
-        ? Buffer.from(data.toString().replace(/\[<id>\]/g, hyperid()))
+        ? Buffer.from(data.toString().replace(/\[<id>\]/g, this.hyperid()))
         : data
     } else if (this.currentRequestIndex === 0) {
       // when first request fails to build, we can not reset pipeline, or it'll never end
diff --git a/package.json b/package.json
index db5ef25..ff558a1 100644
--- a/package.json
+++ b/package.json
@@ -38,6 +38,7 @@
   },
   "homepage": "https://github.com/mcollina/autocannon#readme",
   "devDependencies": {
+    "ansi-regex": "^5.0.1",
     "bl": "^6.0.0",
     "busboy": "^0.3.1",
     "pre-commit": "^1.1.2",


Test patch:
diff --git a/test/printResult.test.js b/test/printResult.test.js
index b2c3007..b7a47a1 100644
--- a/test/printResult.test.js
+++ b/test/printResult.test.js
@@ -4,6 +4,8 @@ const test = require('tap').test
 const split = require('split2')
 const path = require('path')
 const childProcess = require('child_process')
+const { Writable } = require('stream')
+const ansiRegex = require('ansi-regex')
 const printResult = require('../lib/printResult')
 
 test('should stdout (print) the result', (t) => {
@@ -139,3 +141,122 @@ test('should not print when verbose(V=0) is false', (t) => {
   const output = printResult(result, { verbose: false })
   t.ok(output.split('\n').length === 2)
 })
+
+test('should print with color when color is supported', (t) => {
+  t.plan(1)
+
+  const connections = 10
+  const pipelining = 2
+  const result = {
+    connections,
+    pipelining,
+    latency: {},
+    requests: {
+      sent: connections * pipelining
+    },
+    throughput: {
+      average: 3319,
+      mean: 3319,
+      stddev: 0,
+      min: 3318,
+      max: 3318,
+      total: 3318,
+      p0_001: 3319,
+      p0_01: 3319,
+      p0_1: 3319,
+      p1: 3319,
+      p2_5: 3319,
+      p10: 3319,
+      p25: 3319,
+      p50: 3319,
+      p75: 3319,
+      p90: 3319,
+      p97_5: 3319,
+      p99: 3319,
+      p99_9: 3319,
+      p99_99: 3319,
+      p99_999: 3319
+    }
+  }
+
+  const { FORCE_COLOR, NO_COLOR, COLOR, CI, COLORTERM } = process.env
+  delete process.env.FORCE_COLOR
+  delete process.env.NO_COLOR
+  delete process.env.COLOR
+  delete process.env.CI
+  process.env.COLORTERM = 'truecolor'
+  const outputStream = new Writable({
+    write () {}
+  })
+  outputStream.isTTY = true
+
+  // act
+  const output = printResult(result, { outputStream })
+  t.ok(ansiRegex().test(output))
+
+  // cleanup
+  process.env.FORCE_COLOR = FORCE_COLOR
+  process.env.NO_COLOR = NO_COLOR
+  process.env.COLOR = COLOR
+  process.env.CI = CI
+  process.env.COLORTERM = COLORTERM
+})
+
+test('should not print with any color when color is not supported', (t) => {
+  t.plan(1)
+
+  const connections = 10
+  const pipelining = 2
+  const result = {
+    connections,
+    pipelining,
+    latency: {},
+    requests: {
+      sent: connections * pipelining
+    },
+    throughput: {
+      average: 3319,
+      mean: 3319,
+      stddev: 0,
+      min: 3318,
+      max: 3318,
+      total: 3318,
+      p0_001: 3319,
+      p0_01: 3319,
+      p0_1: 3319,
+      p1: 3319,
+      p2_5: 3319,
+      p10: 3319,
+      p25: 3319,
+      p50: 3319,
+      p75: 3319,
+      p90: 3319,
+      p97_5: 3319,
+      p99: 3319,
+      p99_9: 3319,
+      p99_99: 3319,
+      p99_999: 3319
+    }
+  }
+
+  const { FORCE_COLOR, NO_COLOR, COLOR, CI, COLORTERM } = process.env
+  delete process.env.FORCE_COLOR
+  delete process.env.NO_COLOR
+  delete process.env.COLOR
+  delete process.env.CI
+  process.env.COLORTERM = 'truecolor'
+  const outputStream = new Writable({
+    write () {}
+  })
+
+  // act
+  const output = printResult(result, { outputStream })
+  t.ok(!ansiRegex().test(output))
+
+  // cleanup
+  process.env.FORCE_COLOR = FORCE_COLOR
+  process.env.NO_COLOR = NO_COLOR
+  process.env.COLOR = COLOR
+  process.env.CI = CI
+  process.env.COLORTERM = COLORTERM
+})
diff --git a/test/requestIterator.test.js b/test/requestIterator.test.js
index 1ca6a72..07f7a0e 100644
--- a/test/requestIterator.test.js
+++ b/test/requestIterator.test.js
@@ -202,7 +202,11 @@ test('request iterator should not replace any [<id>] tags with generated IDs whe
 })
 
 test('request iterator should replace all [<id>] tags with generated IDs when calling move with idReplacement enabled', (t) => {
-  t.plan(4)
+  t.plan(6)
+
+  function isUrlSafe (string) {
+    return /^[A-Za-z0-9\-_]+$/.test(string)
+  }
 
   const opts = server.address()
   opts.method = 'POST'
@@ -212,15 +216,19 @@ test('request iterator should replace all [<id>] tags with generated IDs when ca
 
   const iterator = new RequestIterator(opts)
   const first = iterator.currentRequest.requestBuffer.toString().trim()
+  const firstId = first.split('\n').pop().trim()
 
   t.equal(first.includes('[<id>]'), false, 'One or more [<id>] tags were not replaced')
-  t.equal(first.slice(-1), '0', 'Generated ID should end with request number')
+  t.ok(firstId.endsWith('0'), 'Generated ID should end with request number')
+  t.ok(isUrlSafe(firstId), 'Generated ID should be URL-safe')
 
   iterator.nextRequest()
   const second = iterator.currentRequest.requestBuffer.toString().trim()
+  const secondId = second.split('\n').pop().trim()
 
   t.equal(second.includes('[<id>]'), false, 'One or more [<id>] tags were not replaced')
-  t.equal(second.slice(-1), '1', 'Generated ID should end with a unique request number')
+  t.ok(secondId.endsWith('1'), 'Generated ID should end with a unique request number')
+  t.ok(isUrlSafe(secondId), 'Generated ID should be URL-safe')
 })
 
 test('request iterator should invoke onResponse callback when set', (t) => {