Investigate flaky test-benchmark-misc

[AshCripps]

• Version: Master
• Platform: Custom Suites Freestyle
• Subsystem: benchmark

internal/readline/utils.js:163
 return str.replace(ansi, '');
 ^

TypeError: Cannot read property 'replace' of undefined
 at stripVTControlCharacters (internal/readline/utils.js:163:14)
 at getStringWidth (internal/readline/utils.js:71:11)
 at main (/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/benchmark/misc/getstringwidth.js:24:5)
 at /home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/benchmark/common.js:40:28
 at processTicksAndRejections (internal/process/task_queues.js:79:11)
assert.js:102
 throw new AssertionError(obj);
 ^

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

1 !== 0

This has been failing node-daily-master since 10-01-2020.
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1802/
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1803/
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1804/
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1805/
https://ci.nodejs.org/view/Node.js%20Daily/job/node-daily-master/1806/

[sam-github]

Not flaky, flat-out broken, fails on master, in ubuntu:

% ./out/Release/node test/benchmark/test-benchmark-misc.js 
internal/readline/utils.js:162
 return str.replace(ansi, '');
 ^

TypeError: Cannot read property 'replace' of undefined
 at stripVTControlCharacters (internal/readline/utils.js:162:14)
 at getStringWidth (internal/readline/utils.js:71:11)
 at main (/home/sam/w/core/node/benchmark/misc/getstringwidth.js:24:5)
 at /home/sam/w/core/node/benchmark/common.js:40:28
 at processTicksAndRejections (internal/process/task_queues.js:79:11)
assert.js:102
 throw new AssertionError(obj);
 ^

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

1 !== 0

 at ChildProcess.<anonymous> (/home/sam/w/core/node/test/common/benchmark.js:35: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'
}

[Trott]

Didn't look too closely, but on the surface, appears to be a victim of recent readline changes. Perhaps fe05818 and/or 5baae14? /ping @BridgeAR

[Trott]

(By the way, if this turns out to be a genuine bug in Node.js core, I never want to hear anyone ever say again "Why do we have tests for the benchmarks?")

[sam-github]

@Trott thanks

I'm doing to do a manual bisect, but my underpowered machine is making slow progress.

[Trott]

I'm doing to do a manual bisect, but my underpowered machine is making slow progress.

You're almost certainly ahead of me. I'm doing work while my machine compiles the master branch....

[BridgeAR]

It's 539df73. The issue is the benchmark test. It has the type set to an empty string and there is no default value in the corresponding benchmark file. It passes undefined to the function. That was coerced to a string originally but no internal implementation passes anything through besides a string.

The solution is to use a default value in the benchmark or to pass through a legit value.

[BridgeAR]

The way our benchmark tests work is less than ideal. I won't find time for it right away but I'll look into rewriting the implementation so that we do not have to maintain our tests anymore.

[Trott]

The way our benchmark tests work is less than ideal. I won't find time for it right away but I'll look into rewriting the implementation so that we do not have to maintain our tests anymore.

Yeah, instead of having default values, it may be best if the test is smart enough to determine the possible options and pop the first value off the default list, or something like that.

[Trott]

Quick fix: #31377

[BridgeAR]

Yeah, instead of having default values, it may be best if the test is smart enough to determine the possible options and pop the first value off the default list, or something like that.

Exactly that 👍