Revert "test: reduce runtime"

[Trott]

This reverts commit 352ae23.

The commit appears to have introduced unreliability in one of the two refactored tests in AIX in CI (see #20688 (comment)) and macOS (see #20888).

Not looking to fast track this or anything (unless this becomes more of a problem for people). Just want it opened in case this is still an issue in 48 hours. The benefits of the change are small (and might be offset by unintentional slow-down on some platforms, see #20688 (comment)).

/cc @danbev @MylesBorins @BridgeAR @nodejs/testing

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes

[danbev]

Reverting does allow test-async-wrap-pop-id-during-load.js to pass for me.

[BridgeAR]

@danbev would you be so kind and check what stack size allows the test to pass for you? I tested this locally and it crashed with 46 on my machine and passed with 50. Seems like aix and others need more stack size. A fix should very likely also be to set the number to 75 (and that would still mean a much faster failing test than before).

[danbev]

would you be so kind and check what stack size allows the test to pass for you?

A stack size of 75 passes for me.

[gireeshpunathil]

do we really need to revert? we understood and ratified the need for #20688 and we now also know the reason for the failure. Isn't it a matter of fixing it?

[Trott]

do we really need to revert? we understood and ratified the need for #20688 and we now also know the reason for the failure. Isn't it a matter of fixing it?

@gireeshpunathil I think your questions/comments are addressed in the PR description:

Not looking to fast track this or anything (unless this becomes more of a problem for people). Just want it opened in case this is still an issue in 48 hours. The benefits of the change are small (and might be offset by unintentional slow-down on some platforms, see #20688 (comment)).

[Trott]

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

[refack]

@Trott, as a possible future S.O.P. why not add test to flaky list instead of reverting? I kinda like the ⚠️ it's more informative than the 🔴 as it answers the user's question of "is this a real fail or a flake" while keeping the issue visible.

[Trott]

@Trott, as a possible future S.O.P. why not add test to flaky list instead of reverting? I kinda like the ⚠️ it's more informative than the 🔴 as it answers the user's question of "is this a real fail or a flake" while keeping the issue visible.

@refack Excellent and totally valid question. I have mixed feelings about marking things as flaky because I feel like it gives everyone a pass on ignoring the problems with the test. It furthers the idea that test failures are someone else's problem and I feel like the only way the project can keep scaling is if CI failures are everyone's problem. I especially dislike it for things that fail more than once in a very long while.

That said, that's all my opinion only and I'm not sure that opinion is widely shared.

[Trott]

#20940 landed, so this is (hopefully) not needed.