The Wayback Machine - https://web.archive.org/web/20201109012035/https://github.com/testing-library/jest-dom/issues/159
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

log DOM on failing toBeInTheDocument() assertion #159

Open
afontcu opened this issue Nov 1, 2019 · 7 comments
Open

log DOM on failing toBeInTheDocument() assertion #159

afontcu opened this issue Nov 1, 2019 · 7 comments

Comments

@afontcu
Copy link
Member

@afontcu afontcu commented Nov 1, 2019

Hi! 馃憢

Describe the feature you'd like:

.toBeInTheDocument() should (could) output the same error as getByX() queries, so that it provides better context when debuggin what went wrong.

There's a recommended rule in eslint-plugin-testing-library that suggests using expect(queryByX('something').toBeInTheDocument()) over expect(getByX('something').toBeInTheDocument()).

I get the point of the rule, as getBy would already fail thus making toBeInTheDocument() redundant (even though I do this as I like to have explicit assertions in my tests). It is also useful as you don't have to "choose" between get/query when testing whether if an element is in the DOM or not.

However, the error message users get from getBy queries is far superior to toBeInTheDocument(), as it outputs the actual structure of DOM nodes, so you get immediate feedback of what's being rendered.

Thus, my suggestion is that a failing toBeInTheDocument() assertion should print the DOM nodes, giving a real equality between getBy vs. queryBy + toBeInTheDocument().

Thoughts?
Thank you! 馃檶


An image is worth a thousand words:

This is the output of a failing queryBy + toBeInTheDocument() assertion:

imatge

This is the output of a failing getBy + toBeInTheDocument() (optionally) assertion:

imatge

@gnapse
Copy link
Member

@gnapse gnapse commented Nov 4, 2019

Makes total sense. Are you up to do this?

@afontcu
Copy link
Member Author

@afontcu afontcu commented Nov 4, 2019

Sure, why not! :)

I just saw prettyDOM() is located in DTL, so I'm not sure what would be the right approach here. Should I import the function from there? Copy&paste its definition here..?


edited: wrote debug, meant prettyDOM :)

@gnapse
Copy link
Member

@gnapse gnapse commented Nov 4, 2019

Hmmmm, good question. Not sure what's the preferred approach. Import it from DTL implies having to add DTL as a dependency (technically is prettyDOM what we'd need to import/ from DTL, not debug, right?) @alexkrolick do you have any opinion about this?

@alexkrolick
Copy link
Contributor

@alexkrolick alexkrolick commented Nov 4, 2019

If needed, we could extract one of those functions into a common package. Could even call it @testing-library/dom-utils or something.

In theory this log should support the same env vars as the one in DTL for output length

@afontcu
Copy link
Member Author

@afontcu afontcu commented Nov 4, 2019

If needed, we could extract one of those functions into a common package. Could even call it @testing-library/dom-utils or something.

Sounds like an interesting idea. However, I fail to see other "utils" from current DTL implementation that would make their way to this shared package.

What if we keep the discussion open (maybe other stuff could join prettyDOM to @testing-library/dom-utils) and see where it goes, and I proceed with a PR that adds DTL as a dependency? Feels like the simplest approach, and I assume 99.999% of jest-dom users will already have DTL in their node_modules.

Does it sound reasonable?

@alexkrolick
Copy link
Contributor

@alexkrolick alexkrolick commented Nov 4, 2019

proceed with a PR that adds DTL as a dependency

Yep, let's just make sure we don't create a circular dep later

@afontcu
Copy link
Member Author

@afontcu afontcu commented Nov 4, 2019

Looks like jest-dom is already a dependency for DTL: https://github.com/testing-library/dom-testing-library/blob/master/package.json#L51. Will it be fine as long as we only import prettyDOM (which has no relation with jest-dom whatsoever)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can鈥檛 perform that action at this time.