test: fix test failure on aix

[BridgeAR]

This makes sure there is enough stack space on different systems. 75 should definitely be enough on all systems.

Alternative to #20927

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[gireeshpunathil]

75 works in my AIX box. Further thinking on it, --stack_size is not a function of the OS thread stack size, instead the amount we are offering to v8 - which means this should be platform agnostic from the native stack perspective, but a function of how much frame size each JS code has consumed already, which can be platform specific. (An explanation for the platform different behaviors, if anyone is interested).

[richardlau]

I'm not going to block this, but I am wary of magic values in test files. Can you guarantee 75 will be a safe value in the future? It feels like setting the stack_size introduces an inherent flakiness -- we picked 50 previously and that turned out to be insufficient.

[gireeshpunathil]

I guess the number which we are taking about is the amount of kilobytes that is required for the frames to execute the methods in this call chain:

RangeError: Maximum call stack size exceeded at runInThisContext (bootstrap_node.js:496:20) at NativeModule.compile (bootstrap_node.js:591:18) at NativeModule.require (bootstrap_node.js:541:18) at module.js:30:12 at NativeModule.compile (bootstrap_node.js:596:7) at Function.NativeModule.require (bootstrap_node.js:541:18) at setupGlobalConsole (bootstrap_node.js:308:33) at startup (bootstrap_node.js:70:7) at bootstrap_node.js:608:3 

if those sequences change and start consuming more stack space, 75 will become insufficient too.

[BridgeAR]

@richardlau I was surprised that it turned out as insufficient because I expected it to be a fixed value on all platforms. However, it would be very surprising if the new value would not be sufficient anymore (also in future cases) because that could only happen in case V8 changes something significantly to the worse or we add multiple extra layers and that is also very unlikely.

We should always realize if the value is insufficient on a CI run and I guess the aix failure originally just slipped through.

[Trott]

FWIW, this was also observed by @danbev on a macOS laptop. Not sure if that warrants changing the description from saying AIX or not.

Also in the "might not really require amending the commit message" department: The stylization of AIX is all caps.

[addaleax]

Fwiw, I think this would have been more easily debuggable if this assertion had come last and the assertion on L22 would print out stderr in its failure message – maybe we can fix up the test for that as well?

[Trott]

Oh, actually, I had a branch to fix this and then didn't open a PR, but maybe it can be tacked onto this. IIRC, the change was this:

assert.strictEqual(ret.status,0,`EXIT CODE: ${ret.status}, STDERR: ${ret.stderr}`);

[Trott]

I opened #20948 so that this one doesn't get delayed with any bike-shedding or CI re-running.

[BridgeAR]

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

[BridgeAR]

Please +1 to fast track this.

[joyeecheung]

There were some infra issues with the last CI. New CI: https://ci.nodejs.org/job/node-test-pull-request/15113/

[Trott]

node-test-commit-osx re-run: https://ci.nodejs.org/job/node-test-commit-osx/18888/

[Trott]

(Removed fast-track label because it's been open for four days and has three approvals, so all it needs is a clean CI.)

[Trott]

Landed in cfc3866