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

Generated UUID in 4.0.5 breaks snapshot testing #562

Closed
brandon-leapyear opened this issue Mar 11, 2020 · 9 comments · Fixed by #566
Closed

Generated UUID in 4.0.5 breaks snapshot testing #562

brandon-leapyear opened this issue Mar 11, 2020 · 9 comments · Fixed by #566

Comments

@brandon-leapyear
Copy link

brandon-leapyear commented Mar 11, 2020

This commit 78e3c2b added generateUUID, which generates a random UUID every time a tooltip is rendered, which changes Jest snapshots every time they're run.

That commit was done in #559, which says "No changes in workflows or documentation", which, while true, apparently didn't take into account changes in the DOM and testing. I would recommend a solution without the uuid state, if possible, but if not, some override that would disable the UUID in testing

@CrOrc
Copy link
Contributor

CrOrc commented Mar 16, 2020

It's better to use v4 from npm module uuid to implement generateUUID. Module "uuid" may be mocked in jest for testing. But it's impossible to mock function that is not exported from your module.

@rathpc
Copy link

rathpc commented Mar 20, 2020

@Rogger794 Are there any plans to fix this in an upcoming release?

@aronhelser
Copy link
Collaborator

aronhelser commented Mar 20, 2020 via email

@wwayne
Copy link
Collaborator

wwayne commented Apr 2, 2020

🎉 This issue has been resolved in version 4.1.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tom1700
Copy link

tom1700 commented Apr 3, 2020

Hi, I'm afraid the issue still stands. The uuid get's bundled together with rest of the code in /dist/index.js, which makes it unmockable.

@rathpc
Copy link

rathpc commented Apr 3, 2020

it would be nice if you could just optionally send a flag through the component to override the class. I think this would resolve the problem for snapshots

@tom1700
Copy link

tom1700 commented Apr 3, 2020

So it's not nice but it works:

jest.mock('crypto', () => ({
  randomBytes: (num: number) => new Array(num).fill(0),
}));

With this my classNames look like:

__react_component_tooltip t00000000-0000-4000-8000-000000000000

@markph1990
Copy link

Could this be an issue with rollup's config? I don't have experience with this tool, but it seems that we should exclude dependencies from our bundle

// rollup.config.js
...
export default {
external: Object.keys(pkg.dependencies || {}),
...

or put uuid inside peerDependecies which then would be automatically excluded via used rollup-plugin-peer-deps-external.

@rathpc
Copy link

rathpc commented Apr 3, 2020

my vote is for excluding dependencies or you could simply just exclude uuid, but since there are only two deps in package it is probably ok to do them all.

the single approach would be this:

external: ['uuid']

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.

7 participants