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

Pretty-print html() output by default #1147

Closed
ianwalter opened this issue Feb 13, 2019 · 10 comments · Fixed by #1229
Closed

Pretty-print html() output by default #1147

ianwalter opened this issue Feb 13, 2019 · 10 comments · Fixed by #1229

Comments

@ianwalter
Copy link

ianwalter commented Feb 13, 2019

What problem does this feature solve?

My team is using @vue/test-utils for snapshot testing components but the snapshots are hard to parse because the HTML string output from .html() is not formatted in a readable way. If html() pretty-printed the HTML it would be a lot easier to review changes to the component-generated HTML.

What does the proposed API look like?

html() would pretty-print the returned HTML string by default, perhaps using something like https://github.com/prettydiff/prettydiff. If necessary, a user could bypass pretty-printing by passing an option to .html(), perhaps .html(false) or .html({ prettyPrint: false }).

@eddyerburgh
Copy link
Member

For snapshot tests, I recommend using the DOM element:

expect(wrapper.element).toMatchSnapshot()

@ianwalter
Copy link
Author

Thank you for your recommendation Edd. I'm sorry that I forgot to mention that I'm mounting/testing the components in Chrome with Puppeteer using page.evaluate so the return value (which then gets its snapshot taken) has to be serializable.

@denisinvader
Copy link

I think you can just use diffable-html in your tests

@briwa
Copy link
Contributor

briwa commented Mar 23, 2019

I needed this too in my test! @eddyerburgh if anyone isn't actively looking into this, I'm interested to work on this, but I guess we might have to agree on the API, whether it's .html(true | false) or .html({ prettyPrint: true | false }).

@addisonssense
Copy link
Contributor

@briwa @eddyerburgh I have a tentative Pull Request for this feature. Are there any preferences on .html(bool) vs .html({ prettyPrint: bool })?

@briwa
Copy link
Contributor

briwa commented Mar 27, 2019

@addisonssense I'd vote for .html(pretty = false) but let's hear it from the majority.

@eddyerburgh
Copy link
Member

I prefer html({ prettyPrint: true | false }) in case we add extra options in the future

@addisonssense
Copy link
Contributor

I have created a PR to resolve this issue. I welcome feedback :)
#1229

@eddyerburgh
Copy link
Member

Thinking about it more, I think it should pretty print by default. Vue Test Utils is still in beta so we can make breaking changes now.

@TheJaredWilcurt
Copy link

Please see: eddyerburgh/jest-serializer-vue#13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants