Components missing tests

[dev0T]

There was an issue (#137) for this but it got closed. I want to keep track of the components that still need testing so I am opening this new one so if anyone else want to contribute.

Components currently missing tests:

• [x] BottomNavigation
• [x] Breadcrumbs
• [x] ButtonGroup
• [x] Card
• [x] Carousel
• [x] ChatBubble
• [x] Checkbox
• [x] Collapse
• [x] Countdown
• [x] Divider
• [x] Drawer
• [x] Dropdown
• [x] Fileinput
• [x] Form
• [x] Hero
• [x] Indicator
• [x] Input
• [x] InputGroup
• [x] Kbd
• [x] Link
• [x] Mask
• [x] Menu
• [x] NavBar
• [x] Pagination
• [x] PhoneMockup
• [x] Progress
• [x] RadialProgress
• [x] Rating
• [x] Select
• [x] Stack
• [x] Stats
• [x] Steps
• [x] Swap
• [x] Table
• [x] Theme
• [x] Toast
• [x] Tooltip
• [x] WindowMockup

[benjitrosch]

Thanks for putting this together. The last issue was closed by mistake, but let's use yours from now on 😄

[oxcened]

Thanks for the issue, was wondering why the previous issue had been closed indeed.

Just contributed to the Rating component, I think it would be wise for the future to get them assigned so that two people don't risk working on the same thing. :)

One thing I'd like to point out is that currently (as far as I know) tailwind's css doesn't get loaded by react testing library, hence I had to replace a hidden class with an inline style to make the tests relevant. Asserting classes isn't an option either as it goes againts the guiding principles. Any way we could handle that?

[dev0T]

Thanks for the issue, was wondering why the previous issue had been closed indeed.

Just contributed to the Rating component, I think it would be wise for the future to get them assigned so that two people don't risk working on the same thing. :)

One thing I'd like to point out is that currently (as far as I know) tailwind's css doesn't get loaded by react testing library, hence I had to replace a hidden class with an inline style to make the tests relevant. Asserting classes isn't an option either as it goes againts the guiding principles. Any way we could handle that?

I agree assigning is a good idea, but I believe that currently the only person that would be able assign people is @benjitrosch. I think it would be good if we let others now here if we're working on something

Regarding the TailwindCSS and react testing library problem, considering that we're testing a component library and not an actual application, in my opinion we're not going against the guiding principles. We need to have a way to verify our component's logic is behaving the way it is intended and in this case, it consists adding class names. We have to believe TailwindCSS is working the way it is supposed to because it isn't our responsibility to test tailwind itself, as the tailwind team already does this. I don't think changing the component's code and using inline CSS is better than asserting classes.

Just to illustrate, if one was working in an application that uses custom components and styles are handled using TailwindCSS, the best way one would make sure styles are being displayed correctly would be using an actual browser (i.e cypress)

[benjitrosch]

@oxcened Everything that @dev0T is right on and I couldn't have said better myself. We're testing the component library not TailwindCSS itself. Not a fan of adding the inline style, especially not for the sake of a unit test.

As for assigning issues to people, I'm against it. We tried that in the past (on the old test issue if I'm remembering right) but people disappear without ever making a PR, so someone who would've done the work is discouraged because they think the ticket is taken. These tests shouldn't take more than 10-15 minutes to write, so the window for overlap is incredibly small. It's better to just accept PR's on a first come first serve basis.

[oxcened]

Completely agree that we shouldn't test tailwind, no doubt on that. However I standby the opinion that tests should resemble the user interaction, I may be wrong here since it's a component library but still that's what I feel is the purpose of them. Don't see as much value in those when you are asserting classes and whatnot. But I accept your reasoning, I've updated my PR accordingly 😄

[dev0T]

Completely agree that we shouldn't test tailwind, no doubt on that. However I standby the opinion that tests should resemble the user interaction, I may be wrong here since it's a component library but still that's what I feel is the purpose of them. Don't see as much value in those when you are asserting classes and whatnot. But I accept your reasoning, I've updated my PR accordingly 😄

I understand what you mean and I don't disagree! Since we're don't have Tailwind during testing it might appear that we're testing nothing but we're making sure that the component logic is correct. Like I said previously, in this situation if one would like to test the actual styles, that would need to happen when components are rendered in the actual browser, since the alternatives are either using inline CSS or creating mock tailwind classes that are added to the components before each test, both options won't solve the problem (it might even make it worse)

[benjitrosch]

We're so close to full coverage, thank you for the help everyone!