Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make emit work in jest #3

Merged
merged 1 commit into from
Jul 28, 2020
Merged

Conversation

paolochiodi
Copy link
Contributor

This should fix #2, which is also similar to other issues reported to fastify itself (i.e. see fastify/fastify#2438)

The underlying cause here is jest fiddling with sandboxing, context and globals. The result is that the Error global object available to fastify-warning is not the same object in the main node process and thus FastifyWarning instanceOf Error that is run in the implementation of process.emitWarning (see https://github.com/nodejs/node/blob/v10.22.0/lib/internal/process/warning.js#L146) fails.

The issues caused by jest sandboxing model have been known for a while now (see jestjs/jest#2549 and jestjs/jest#8246) and looks like they won't be solved soon.

This PR works around it by not emitting an error, but rather calling emitWarning with string params instead (documented here: https://nodejs.org/dist/latest-v12.x/docs/api/process.html#process_process_emitwarning_warning_type_code_ctor)

The downside of this approach is that warning.create won't be returning a constructor for an object that inherits from Error, but that apparently was never used in fastify.

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@delvedor delvedor merged commit 8f258bc into fastify:master Jul 28, 2020
@delvedor
Copy link
Member

Landed in v0.2.0, thanks!

@paolochiodi paolochiodi deleted the jest-support branch July 28, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FastifyWarning not an instance of Error
3 participants