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

[core] Improve test coverage #16453

Merged
merged 13 commits into from
Jul 4, 2019
Merged

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Jul 2, 2019

Converts all tests for components applying density. I'm touching the test anyway for #16410 so that's always a good time for refactoring.

@eps1lon eps1lon added the test label Jul 2, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Jul 2, 2019

No bundle size changes comparing 3bbf941...114bc4f

Generated by 🚫 dangerJS against 114bc4f

@eps1lon eps1lon force-pushed the test/testing-library-migration branch 3 times, most recently from 165ee08 to 0ad86f6 Compare July 3, 2019 11:34
assert.strictEqual(handleEmpty.callCount, 1);
assert.strictEqual(muiFormControl.onEmpty.callCount, 2);
fireEvent.change(container.querySelector('input'), { target: { value: '' } });
expect(handleEmpty.callCount).to.equal(2);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was 1 is now 2 because of implicit act. Those values don't really make sense to me right now but I will follow-up on this because the current behavior probably causes #16458.

<ListItem>
<ListItemText primary="primary" />
<ListItemSecondaryAction />
</ListItem>,
);
const listItem = findOutermostIntrinsic(wrapper);
assert.strictEqual(listItem.hasClass(classes.container), true);
assert.strictEqual(wrapper.find('li > div').exists(), true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't do what it was intended to do since it also matched <li><span /><div /></li> which is important for the next test.

@eps1lon eps1lon force-pushed the test/testing-library-migration branch from 0ad86f6 to 114bc4f Compare July 3, 2019 12:14
@eps1lon eps1lon marked this pull request as ready for review July 3, 2019 12:21
@eps1lon eps1lon requested a review from joshwooding July 3, 2019 15:09
expect(root).to.have.class(classes.sizeSmall);

root = render(<IconButton size="medium">book</IconButton>).container.firstChild;
expect(root).not.to.have.class(classes.sizeSmall);
Copy link
Member

@joshwooding joshwooding Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you copied the previous tests but, should we test that the medium class has been applied?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no medium class. It's one of the issues I have with moving default styles to root instead their actual variant.

@eps1lon eps1lon merged commit 73e9950 into mui:master Jul 4, 2019
@eps1lon eps1lon deleted the test/testing-library-migration branch July 4, 2019 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants