revert: benchmark: test refactoring

[addaleax]

This fixes running benchmark tests. Please 👍 this comment to approve fast-tracking.

Example failure:

=== release test-benchmark-async-hooks ===                   
Path: benchmark/test-benchmark-async-hooks
Error: wrk failed with 1. Output: Usage: wrk <options> <url>                            
  Options:                                            
    -c, --connections <N>  Connections to keep open   
    -d, --duration    <T>  Duration of test           
    -t, --threads     <N>  Number of threads to use   
                                                      
    -s, --script      <S>  Load Lua script file       
    -H, --header      <H>  Add header to request      
        --latency          Print latency statistics   
        --timeout     <T>  Socket/request timeout     
    -v, --version          Print version details      
                                                      
  Numeric arguments may include a SI unit (1k, 1M, 1G)
  Time arguments may include a time unit (2s, 2m, 2h)

    at ChildProcess.<anonymous> (/home/sqrt/src/node/master/benchmark/_http-benchmarkers.js:235:16)
    at Object.onceWrapper (events.js:428:26)
    at ChildProcess.emit (events.js:321:20)
    at maybeClose (internal/child_process.js:1051:16)
    at Socket.<anonymous> (internal/child_process.js:442:11)
    at Socket.emit (events.js:321:20)
    at Pipe.<anonymous> (net.js:673:12)
assert.js:102
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 0

    at ChildProcess.<anonymous> (/home/sqrt/src/node/master/test/common/benchmark.js:30:12)
    at ChildProcess.emit (events.js:321:20)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:276:12) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 1,
  expected: 0,
  operator: 'strictEqual'
}
Command: out/Release/node /home/sqrt/src/node/master/test/benchmark/test-benchmark-async-hooks.js
[00:00|% 100|+   0|-   1]: Done                              

(This is back to working after the revert.)

Refs: #31396

Fyi @BridgeAR

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/29063/

[cjihrig]

LGTM. Out of curiosity, are there any small fixes that could done in place of a full revert?

[addaleax]

@cjihrig I tried to debug it, but gave up after half an hour. Somewhere, the duration parameter gets set to a non-integer value, and wrk doesn’t accept that, but that’s all that I could find out.

[addaleax]

Landed in 2abf0af...5b0308c, thanks for the fast reviews!

[BridgeAR]

I just had a look at this. It's due to setting the duration environment variable which is used in TestDoubleBenchmarker. The env was spread into the envs together with duration set when using wrk. That way the string value from the env replaced the original duration set by the options and the benchmark failed.

A fix for this is pretty straight forward by only setting the env value in TestDoubleBenchmarker.

[BridgeAR]

There might be more ongoing though. I'll have a look at it later on again.