test: convert new tests to use error types

[jackhorton]

Checklist

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

Affected core subsystem(s)

test

Some new tests were added recently that rely on error messages that are incompatible with Node-ChakraCore. This PR simply reverts the test to only relying on error type, rather than error message. vcbuild test is actually failing for me on an unrelated test, sequential/test-inspector-port-cluster, but I might make a new issue for that separately.

[jasnell]

I'd keep this one as common.expectsError()

[jackhorton]

Updated to expectsError. Just for my own understanding, when is it preferred to use common.expectsError(func, { type }) over assert.throws(func, type)?

[BridgeAR]

It was originally recommended to use common.expectsError as it was much more powerful than assert.throws. This changed recently and now it is possible to also use a object for the validation. common.expectsError is still better in two small cases: it validates the error type and it allows a RegExp for the messages. There will always be a valid use case for common.expectsError even if those things would be overcome as well: it is possible to use it as callback and that will not be possible with assert.throws.

I personally recommend to always use assert.throws if it is suitable. I might update the docs about it some time soon.

[BridgeAR]

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

[jackhorton]

I have looked at a handful of the CI errors and none of them look related to this change, is that expected? OS X had a missing tool, Linux had a YAML issue, and some others didn't look like they reported why they failed at all.

[BridgeAR]

@jackhorton to some extend: yes. But not as bad as it has happened here. We have to many flakes and other issues with our CI at the moment as it seems.

[lpinca]

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

[BridgeAR]

Landed in 1729af2 🎉

[MylesBorins]

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

[jackhorton]

Not sure what the status quo is for this -- theres no functional update, it just makes for fewer special cases in Node-ChakraCore. I think im fine with not backporting it?