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

Bug: html() makes the output pretty making comparisons difficult #1334

Closed
nandi95 opened this issue Feb 18, 2022 · 4 comments
Closed

Bug: html() makes the output pretty making comparisons difficult #1334

nandi95 opened this issue Feb 18, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@nandi95
Copy link
Contributor

nandi95 commented Feb 18, 2022

Describe the bug
The change introduced by #1120 has caused my test suite to fail.
The test checks that the focused element is same as the expected but given the document.activeElement?.outerHTML is on one line and html() is on multiple with indentation this fails.

To Reproduce
https://github.com/karnama/vueish/blob/main/src/components/select/UISelect.test.ts#L534

Expected behavior
The test to pass

Related information:

  • @vue/test-utils version: 2.0.0-rc.18
  • Vue version: 3.2.31
  • node version: 17.5.0
  • npm (or yarn) version: 8.4.1

Additional context
If this is desired, what is the best way to remove the indentation and new lines? a regex? something else?
Why was this change introduced? If people have to look at the html in the console the canuse the pretty() themselves no? or otherwise the differences are highlighted in the console.

@nandi95 nandi95 added the bug Something isn't working label Feb 18, 2022
@lmiller1990
Copy link
Member

lmiller1990 commented Feb 21, 2022

HTML and snapshot assertions have been painful over the years. What's the question exactly, how best to format HTML? I think the only way to expect consistency from an undefined behavior (whitespace around HTML is not something VTU specifies) would be to format it on your end using something like prettier.

For your particular assertion, I'd usually just have an id or data-testid and compare the two, so you are not relying HTML whitespace (which has no semantic meaning).

I am not sure this would be classes as a bug - changing anything around html will probably be a regression for others relying on the same thing. Ultimately, I don't think anyone should be relying on comparing HTML strings for any kind of testing. Do you think using an id or something similar be a more reliable alternative for your test?

@nandi95
Copy link
Contributor Author

nandi95 commented Feb 21, 2022

I have not considered that. It is a good idea, excpect I would rather find something else that is unique to that html than a test specific attribute as I'm reluctant to change the component for the sake of testing.

I don't think anyone should be relying on comparing HTML strings for any kind of testing

Perhaps we can make a note of this in the documentation?
I'm happy to open a PR

@nandi95 nandi95 closed this as completed Feb 21, 2022
@lmiller1990
Copy link
Member

lmiller1990 commented Feb 22, 2022

We could add a note, but I'm not sure where. The API part of the docs should not really contain advice, just the API. Where would you add the note?

This is also why I think snapshots for DOM comparison is basically useless - knowing how the string looks doesn't tell you anything about if it actually behaves correctly.

@TheJaredWilcurt
Copy link
Contributor

The default serializer that Vue-CLI ships with is lacking any customization features.

For greater control over the formatting of your snapshots, you can create your own custom snapshot serializer, or use a library with controls built in for you to tweak the formatting:

This is also why I think snapshots for DOM comparison is basically useless

Snapshots are extremely useful, when used well. For example, my team at work uses many open source components that lack any tests of their own. During dependency updates we can see DOM changes in these 3rd party components in our own tests, which alert us of breaking changes, like logic resulting in something being disabled or hidden, or changes to class names that would affect our styling overrides. We are often the first people to report bugs in these repos because of this very basic level of testing. It is easy to over use them, or abuse the feature. But Snapshots should not be avoided entirely. Every component I test starts with a full snapshot, followed by targeting specific elements of DOM for snapshots when I need to validate the DOM's expected state matches the component's state given a scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants