test : refactor parallel/test-tls-cert-chains-concat

[juggernaut451]

added common.mustCall and replaced the function with arrow function

Fixes : #14544

Checklist

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

Affected core subsystem(s)

test

[BridgeAR]

@juggernaut451 thanks a lot for your contribution. I personally feel like it is not really the right direction though to always add common.mustCall. @nodejs/collaborators do we actually want to add must calls everywhere?

This is more of a general thing and has nothing to do with this specific PR.

[richardlau]

do we actually want to add must calls everywhere?

IMO we do for callbacks otherwise the tests could pass if the callback was never called.

[BridgeAR]

@richardlau I do not agree on that point if the callback is from a heavily used main API. If it is a callback that is unique to the test itself it should be added.

[richardlau]

@BridgeAR I'd argue that it's a much simpler rule to apply everywhere rather than have to try to decide if an API is heavily used enough. If you do feel strongly about this I suggest raising a separate issue for discussion so that we don't clog up this PR 😄.

[addaleax]

LGTM

@BridgeAR I see your concerns, but I don’t really see a downside of having these checks here… I guess it’s a bit of churn, but I’d say we’ve done worse things in the past when it comes to that

[Trott]

No comment on this particular change, but in general: I'd prefer we be judicious in our use of common.mustCall(). Obviously, if the test may pass erroneously if the callback does not run, then we should use it. Otherwise, we should only use it (IMO) if it clearly improves the error message when the test fails. We should be thoughtful about it.

[jasnell]

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

[juggernaut451]

Resolved :)

[BridgeAR]

Landed in cbc7eb7 🎉