See here: https://github.com/nodejs/http2/blob/master/test/parallel/test-http2-client-stream-destroy-before-connect.js#L14-L22

Specifically:

// Do not mustCall the server side callbacks, they may or may not be called
--
// depending on the OS. The determination is based largely on operating
// system specific timings
server.on('stream', (stream) => {
  stream.on('error', common.expectsError({
  code: 'ERR_HTTP2_STREAM_ERROR',
  type: Error,
  message: 'Stream closed with error code 2'
}));
//....

Depending on the operating system, the 'stream' event may or may not fire and the 'error' event may or may not fire. This is currently a bit non-deterministic because it depends entirely on async timings at the operating system level. I'm working on making it more deterministic but this is what we have currently.

While refactoring this is minimal, it's also a bit pointless -- that is, changing common.expectsError() to always be a common.mustCall() is unnecessary since it is trivial to do common.mustCall(common.expectsError()) as necessary.

I'm a fan of less magic in common anyway.

I would not consider this "magic", rather being more explicit... As can be seen in 1b2733f (#14088) all existing cases of expectsError (177 in total) we're compatible with a mustCall. Also, the term expects implies something should happen.
The case demonstrated above can be easily solved with an if since it's the exception to the rule.

I would not consider this "magic"

FWIW, if I had my way, common.expectsError() wouldn't exist at all. If I'm new to the project and I see a function called expectsError(), what does that even do? I would expect it to act like assert.throws(). I would expect the first argument to be a function that is expected to cause an error.

Our tests would be clearer and more self-contained if we simply wrote out the function that verifies the error. DRY in a test can be an anti-pattern.

That said, my views on what is and isn't appropriate for common are increasingly out of step with the project, it seems. I suspect there will be some 😕 and 👎 on this comment.

I would expect the first argument to be a function that is expected to cause an error.

Which it does now 😄
I agree in principle, we should either expose the internal/Error or create a mockNodeError(type, code, message) and do deepStrictEqual

Which it does now 😄

Optionally. Which: ugh. But yes, you are correct and my comment there misses the mark.

I agree in principle, we should either expose the internal/Error or create a mockNodeError(type, code, message) and do deepStrictEqual

In principle, yes. Unfortunately, deepStrictEqual() does not work on errors because of non-enumerable properties:

assert.deepStrictEqual(new Error('foo'), new Error('bar')); // surprise! this passes!

On the upside, there's no reason someone can't submit a PR to fix that shortcoming.

On the upside, there's no reason someone can't submit a PR to fix that shortcoming.

Ack. That's a good idea, so we stop patchwork cycle.

While refactoring this is minimal, it's also a bit pointless -- that is, changing common.expectsError() to always be a common.mustCall() is unnecessary since it is trivial to do common.mustCall(common.expectsError()) as necessary.

My only feeling on this would be that having mustCall as the default might help to catch errors where you forgot to include that, kinda seems like a safer default. Then you could have an option to explicitly opt out.

Not all that happy about this but it's not important enough to fight :-)