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

tests failing in v4.17.5 #16

Closed
davidmarkclements opened this issue Jul 21, 2018 · 3 comments · Fixed by pinojs/pino#464
Closed

tests failing in v4.17.5 #16

davidmarkclements opened this issue Jul 21, 2018 · 3 comments · Fixed by pinojs/pino#464

Comments

@davidmarkclements
Copy link
Member

due to pinojs/pino#456

it should be fine to fix the pino-pretty test

cc @mcollina @jsumners

@jsumners
Copy link
Member

But the test isn't broken. It explicitly looks for the null serialization because pinojs/pino#382

@disintegrator
Copy link

Hey folks, is someone looking at this or shall I pick it up? I wasn't aware of the integration suite when I was creating my PR 🤦‍♂️
@jsumners My change in pinojs/pino#457 caused this by preventing the stack property from being printed in the JSON output if it is false-y.
I can think of a couple of ways forward:

  • If you want the logger to emit "stack": null then I'd need to update my fix
  • If we are happy to not output stack if it is false-y then the test needs to be updated

Any other suggestions?

@davidmarkclements
Copy link
Member Author

If you update your fix it will be a major - which is fine because we are close to releasing v5 - see the next-major branch

However for v4 it can't be an update so we need to change the expectation in pino-pretty

Please feel free to PR!

@mcollina do you concur

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 a pull request may close this issue.

3 participants