test: check all properties in common.expectsError

[BridgeAR]

This makes sure all properties that are meant to be checked will
actually be tested for. Otherwise properties might be missed.

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

[BridgeAR]

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

[lpinca]

Landed in 2fef227.

[joyeecheung]

Is it intentional that code will be checked after message? Right now if an error with a wrong code is thrown, one will have to search for the message formatter with the message in lib/internal/errors.js in order to understand what code the actual error has.

[joyeecheung]

cc @BridgeAR ^

[targos]

This commit lands cleanly on v9.x-staging but tests don't pass. Should it be backported?

[BridgeAR]

I guess in that case it would be best to backport it to make sure all properties are actually properly tested.

[BridgeAR]

@joyeecheung sorry, I overlooked your comment.

It was not intentional to move the code check after message. That is actually now depending on the insertion order of the properties that should be checked. I actually recommend not to use common.expectsError in case it is not needed (in all cases when it is not used as callback). If assert.throws is used instead, all properties are going to be compared. That makes debugging much easier.

Since this function is still used currently, I am going to open another PR to add a similar functionality to common.expectsError. That way the order is not important anymore.