-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 component level tests to withDuotoneControls HOC #48353
Conversation
@glendaviesnz I've been trying to get these tests to work with no success. Currently the only thing that's rendered in the test is <div>
<div />
</div> I'm expecting to see
I've tried removing all the conditionals around the panel rendering in the HOC but still no luck. With your experience in this area, would you be able to take a look and see if there's anything I'm doing obviously wrong? Would be much appreciated 🙏 |
Size Change: +430 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Sorry about the delay in responding, been AFK, will try and take a look today |
hmm, looking at this example it seems like this should work by wrapping |
@getdave I didn't have much time to look at this today, but |
Flaky tests detected in 208697e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4292235636
|
@glendaviesnz Thank you so much for taking the time to look at this. That is so strange. I even tried that myself. I must have had one piece of config incorrect. Let me see if I can figure out what's happening with dual buttons. |
@glendaviesnz I got this working by removing the additional passing of |
@glendaviesnz I had a think about how to test I think we'd need to mock |
const unsetOption = screen.queryByRole( 'button', { | ||
name: 'Unset', | ||
pressed: false, // the unset option should not be pressed. | ||
} ); | ||
expect( unsetOption ).toBeInTheDocument(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should tests for things like this belong in the components package under duotone-picker instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeh to be fair they could be. It's straying into e2e test territory.
Will have a look at it, but it does seem like that would need a bit more setup for sure - may not get back to it until end of week. |
I'm going to focus this PR on the |
@ajlende Do you see any value to these tests? I'm not managing to getting around to fix them up 😬 |
What?
Adds component level tests to verify high level functionality of
withDuotoneControls
HOC.Closes #48347
Why?
Need for greater confidence in functionality of Duotone UI extension, particular that it only renders for blocks which exhibit the correct block supports.
Note: more complex assertions will require e2e tests.
How?
Tests
attributes.style.color.duotone
.Testing Instructions
Run
Testing Instructions for Keyboard
Screenshots or screencast