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

Add debugging helper? #31

Closed
BarthesSimpson opened this issue Mar 30, 2018 · 15 comments
Closed

Add debugging helper? #31

BarthesSimpson opened this issue Mar 30, 2018 · 15 comments

Comments

@BarthesSimpson
Copy link

BarthesSimpson commented Mar 30, 2018

I know you're trying to keep this awesome library lightweight. In general, I advocate avoiding using Enzyme's .debug() method, but there are times where it's invaluable in hunting down a mistake in a test. I'm currently using a few helpers to supplement your library, like this:

/* debug.js */
import pretty from 'pretty'

export default element => {
  console.log(pretty(element.outerHTML))
}
/* changeInputValue.js */
import { Simulate } from 'react-testing-library'

export default (input, value) => {
  input.value = value
  Simulate.change(input)
}

Usage:

import { debug, changeInputValue } from 'helpers/react-testing-library'

it('Adds a new resource', () => {
    const { container, getByTestId, queryByTestId } = render(<App />)
    expect(queryByTestId('resource-1')).not.toBeInTheDOM()
    const input = getByTestId('textInput')
    changeInputValue(input, 'Test Value')
    Simulate.click(getByTestId('button-with-redux'))
    debug(container)
    expect(getByTestId('resource-1').textContent).toBe('Test Value')
})

Are you open to including helpers like this in the core library?

@antsmartian
Copy link
Collaborator

antsmartian commented Mar 30, 2018

Thanks for your issue and we are glad you are liking this library. I'm wondering do we really need to add these items into this library. Because to me both debug and changeInputValue seems like a util, that one can write along with their test cases (mostly again to their own taste & preferences). And if we start supporting utils like changeInputValue, then well, we might need to support similar change* methods for select, radio etc, which I'm not really sure if it worth adding it in library.

Coming back to your debug, again its a matter of preferences for the developers. Because I personally attach a debugger, when something is failing, well other developers might do the same way or like you had suggested. Again I'm thinking on whether to add debug as part of this library.

May be when the library throws error like can't find element (when you call get*), I guess in those cases we can make the error message along with html prettified. That might help everyone I guess to see what exactly went wrong.

@kentcdodds Any thoughts?

@kentcdodds
Copy link
Member

I agree with @antoaravinth. I do like his idea here:

May be when the library throws error like can't find element (when you call get*), I guess in those cases we can make the error message along with html prettified. That might help everyone I guess to see what exactly went wrong.

We'd have to be careful though. The element to be printed could be VERY large. Perhaps we could do something like:

if (htmlContent.length < config.maxLengthToLog) {
  console.log(htmlContent)
} else {
  console.log(`The html content has the length of ${htmlContent.length} which is larger than the default maxLengthToLog of ${config.maxLengthToLog}. Change the default in the config`)
}

Of course the messaging would be improved there and you could import the config file in your setupTest file or something. What do you think? I think I'm in favor. I think the default would be something like 7000 or something.

@antsmartian
Copy link
Collaborator

@kentcdodds I was thinking in the same boat :) Yes we can add these things, wondering whether we need to add this helper in query* methods as well. So fundamentally in all our API which talks with container (DOM actually), should be showing the prettified html when things goes wrong.

@kentcdodds
Copy link
Member

We can't do it for the query* methods because sometimes you actually want those to return null (to verify something does not exist). But yes, we could do this for the get* methods.

I realize this still doesn't quite satisfy what you are trying to accomplish @BarthesSimpson and I'm sorry for that, but I do think your proposed methods are outside the scope of this project.

However! I do have a solution. I demonstrate it in the client-side config of my testing workshop.

I recommend everyone have a file like this file which re-exports the utils I use from react-testing-library as well as some custom utilities that I like to have in my app. Because I have <rootDir>/test in the modulePaths config, I can import that file as if its a node module in any test file like this.

So you could put such utilities in a file like that and it wouldn't make any difference that it's not included by default :)

@antsmartian
Copy link
Collaborator

@kentcdodds Yeah your right about query. Anyways, will look into adding those infos to get* functions in sometime.

@BarthesSimpson
Copy link
Author

Thanks for the quick and thorough replies. @kentcdodds I do something similar to what you describe with a helper file in your modulePaths (but with more helpers, and using an alias because the helpers are used by multiple teams). Are there helpers you are interested in adding to the core lib? If so, we are interested in contributing, and if not we may just release a separate package.

@kentcdodds
Copy link
Member

Why don't you make a separate package and if it makes sense later we can discuss including it in this library. Thanks!

@lgandecki
Copy link
Collaborator

I was thinking about this, having worked with rather complex (and sometimes tangled..) react components, and was trying to figure out what would be the most useful in real life.

  1. So, printing a dom below the limit that Kent proposed:

screen shot 2018-04-01 at 11 58 03

Printing dom structure would definitely helpful, but when we debug with tools like cypress we don't look at html structure, we look at the page as the user would - right? This printout is not very readable, it's not immediately obvious what you are looking at.

  1. So.. We could possibly "render" the page in the console:

screen shot 2018-04-01 at 11 59 17

This is better but since we are skipping a lot of visual helps.. it's hard to understand what's going on. Where are the buttons, how are the elements really laid out on the page. etc..

  1. Third option, print the html but make the actual text in contrast to the DOM elements (but keep them for a reference):

screen shot 2018-04-01 at 12 00 25

I'd strongly vote for the option number 3. We could do more here. Obviously I'm highlighting empty space (not very problematic but could be avoided). We could highlight alts/labels, to be able to understand the elements that dont have texts on them... Possibly remove classes and styles to clean up the print out?

What are you guys thinking?

It would be nice to have full highlighting here, but as far as I can see no one done it, it would be quite a bit of work, and I think the most important part is still the contrast between text and markup, for comparison:

screen shot 2018-04-01 at 12 04 11

Keep in mind that with different background it's easy to get the text unreadable. But the simple invert for the things you want to stand out will always work nicely ( I think )

@antsmartian
Copy link
Collaborator

antsmartian commented Apr 1, 2018

@lgandecki Option 3 looks, cool. Yeah that will definitely help the user to quickly see through their page, but writing the highlighting thing would be big work I suppose (not sure whether you had already implemented the same). I was actually looking to implement the idea by printing the dom, using jest-matcher-utils, which has few methods like print*, which actually prints the given dom in colorful manner (this is actually used by jest when your assertions fail). Since we already uses the jest utils, for our custom assertions, I thought we no need to add any new library as such for getting this task done. Implementing your last screenshot, may be, from my opinion, is huge work or we can see if jest-utils could do that.

@lgandecki
Copy link
Collaborator

All of the things mentioned above are working. Besides the last screenshot, which, I agree - would be.a lot of work, and, worse than that - I don't think there is a way to detect the background, so it'd be very easy to make this unreadable for users. (that's why my proposition is just simple invert)

I'm not familiar with jest printing dom - but if it already does nice DOM formatting with colors then just ignore my comment. :)

@antsmartian
Copy link
Collaborator

antsmartian commented Apr 1, 2018

@lgandecki Sure. We can see what @kentcdodds has to say on our thoughts, before we could come to a conclusion..

@lgandecki
Copy link
Collaborator

lgandecki commented Apr 1, 2018

I'm looking at
https://github.com/facebook/jest/blob/master/packages/jest-matcher-utils/src/index.js

And I don't see anything related to html there.

As for printing out colored JavaScript it actually uses this - https://www.npmjs.com/package/babel-code-frame

which says:

highlightCode

boolean, defaults to false.

Toggles syntax highlighting the code as JavaScript for terminals.

I gave it a try and it produced this:
screen shot 2018-04-01 at 14 50 10

Which is a bit inconsistent - Sometimes the actual text is yellow (same as html tags) sometimes it's the default color text.
We could probably use that and improve it.

edit: babel-code-frame is actually using internally this one - https://github.com/babel/babel/tree/master/packages/babel-highlight and it's super simple

@kentcdodds
Copy link
Member

Hmmm... Yeah, that formatting and highlighting is a little bit off. I'd love the HTML to be formatted the same way that jest formats the snapshots which I think is more legible (and totally possible with pretty-format + the built-in DOM plugins). From there we'd just need to have a good highlighting solution. I'm definitely interested in using something like that for this. Would be very cool!

julienw pushed a commit to julienw/react-testing-library that referenced this issue Dec 20, 2018
…ng-library#31)

- Changes queries to default to exact string matching
- Can opt-in to fuzzy matches by passing { exact: true } as the last arg
- Queries that search inner text collapse whitespace
  (queryByText, queryByLabelText)

BREAKING CHANGE: Strings are considered to be an exact match now. You can opt-into fuzzy matching, but it's recommended to use a regex instead.
@balazsorban44
Copy link

We are using a UI library, which unfortunately makes the HTML a bit too verbose to read even with pretty printed HTML. I was curious if it would be possible to somehow "serve" the HTML snippet when calling debug, or at least save it to a .html file?

@kentcdodds
Copy link
Member

I don't think we'd implement something like that in core, but you could definitely write a helper module that does that!

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

No branches or pull requests

5 participants