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

test(logger): use jest-serializer-ansi-escapes #7685

Merged
merged 2 commits into from
Jun 27, 2022
Merged

test(logger): use jest-serializer-ansi-escapes #7685

merged 2 commits into from
Jun 27, 2022

Conversation

mrazauskas
Copy link
Contributor

Pre-flight checklist

Motivation

I noticed that ansi escape sequences are included in snapshots of @docusaurus/logger tests. Recently I wrote a snapshot serializer which is replacing the sequences with human readable strings. For example, '\u001b[3;32mSuccess!\u001b[0m' gets snapshotted as '<italic, green>Success!</>'.

Just though – perhaps you will find it useful? After using the plugin in a couple of projects, it felt to me that the improved readability helps to maintain snapshots. Of course, in the end the same output is snapshotted.


jest-serializer-ansi-escapes is inspired by ConvertAnsi plugin which ships with pretty-format and is bundled with Jest as well. Currently ConvertAnsi does not support blue or underline. Also its implementation is based on ansi-styles (which does not work for vanilla sequences with more than one arguments: \u001b[3;32m).

These and similar limitations was the reason why I created jest-serializer-ansi-escapes. It supports vanilla escape sequences as well as the ones produced by libraries like chalk and it replaces color, style and cursor control escape sequences.

Test Plan

Snapshots are updated. All current tests should pass.

Test links

N/A

Related issues/PRs

N/A

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 26, 2022
import logger from '../index';

// cSpell:ignore mkeep
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, snapshots become cSpell friendly as well.

@netlify
Copy link

netlify bot commented Jun 26, 2022

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit f4271cb
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/62b877b971d1c000080e8b17
😎 Deploy Preview https://deploy-preview-7685--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Jun 26, 2022

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟢 92 🟢 100 🟢 100 🟢 100 🟢 90 Report
/docs/installation 🟠 85 🟢 100 🟢 100 🟢 100 🟢 90 Report

Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

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

Thanks! I agree this looks nicer. Can we make this serializer global? We have other places using ANSI snapshots as well.

BTW, the readability is not a huge issue for me, since there's a VS Code extension:

image

But I do buy the spell-checker point, so I want this ported to other places as well.

@Josh-Cena Josh-Cena added the pr: internal This PR does not touch production code, or is not meaningful enough to be in the changelog. label Jun 26, 2022
@mrazauskas
Copy link
Contributor Author

@Josh-Cena Thanks for review.

Just moved the serialiser to jest.config. I was thinking that after #5994 @docusaurus/logger is the only place which will have escape sequences in snapshots. Or?

@Josh-Cena
Copy link
Collaborator

Oh, I suddenly realize we already have this: #7131 Yeah I think this is fair enough. Thanks!

@Josh-Cena Josh-Cena merged commit bb558ff into facebook:main Jun 27, 2022
@mrazauskas mrazauskas deleted the chore-use-jest-serializer-ansi-escapes branch June 27, 2022 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: internal This PR does not touch production code, or is not meaningful enough to be in the changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants