Skip to content

test: refactor common.expectsError

Rodrigo Muino Tomonari requested to merge github/fork/BridgeAR/2019-12-25-refactor-common-expects-error into master

This completely refactors the expectsError behavior: so far it's almost identical to assert.throws(fn, object) in case it was used with a function as first argument. It had a magical property check that allowed to verify a functions type in case the type property was set. This pattern is now completely removed and assert.throws() should be used instead.

The main intent for common.expectsError() is to verify error cases for callback based APIs. This is now more flexible by accepting all validation possibilities that assert.throws() accepts. No magical properties exist anymore. This reduces surprising behavior for developers who are not used to the Node.js core code base.

The custom prefer-common-expectserror eslint rule comes obsolete with this change and common us used significantly less frequent.

Refs: https://github.com/nodejs/node/pull/30186#issuecomment-549182738

Most changes are trivial and it should be possible to skim through most of them. Files that have been changed more "significantly":

  • test/parallel/test-crypto.js
  • test/common/index.js
  • test/common/README.md
  • test/parallel/test-eslint-prefer-common-expectserror.js (removed)
  • test/parallel/test-http-invalid-path-chars.js
  • test/parallel/test-net-better-error-messages-port-hostname.js
  • test/parallel/test-net-connect-immediate-finish.js
  • test/parallel/test-stream-writable-change-default-encoding.js
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

Merge request reports