Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Aug 11, 2021

WHY are these changes introduced?

I am updating existing tests for CalloutCard, Caption, CheckableButton, Resizer, VideoThumbnail components to use the new modern framework.

WHAT is this pull request doing?

Part of test modernization (https://docs.google.com/spreadsheets/d/1GBuEZbOpVYJLNISK7gL69DU8rCxocn5L5GYaKtPXPbU/edit#gid=1498187033), updating tests using {mountWithAppProvider} from 'test-utilities/legacy' to {mountWithApp} from 'test-utilities'.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 11, 2021

size-limit report

Path Size
cjs 142.51 KB (0%)
esm 96.27 KB (0%)
esnext 139.44 KB (0%)
css 33.74 KB (0%)

@ghost ghost force-pushed the polaris_video branch from cc6ab7d to 1d83884 Compare August 11, 2021 21:14
@ghost ghost changed the title [Test Modernization]: Add test modernization to CalloutCard, Caption, CheckableButton, Checkbox, Tag, Resizer, VideoThumbnail [Test Modernization]: Add test modernization to CalloutCard, Caption, CheckableButton, Resizer, VideoThumbnail Aug 11, 2021
@ghost ghost force-pushed the polaris_video branch from 1d83884 to d346ecd Compare August 12, 2021 13:42
);
element.find(Checkbox).first().simulate('click', {

element.find(Checkbox)!.find('input')!.trigger('onKeyUp', {
Copy link
Author

Choose a reason for hiding this comment

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

This is another change, I am open for suggestions :)

Copy link
Contributor

Choose a reason for hiding this comment

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

My feeling is that there should be a test that checks that the function is passed to the Checkbox component, but not testing if it’s called, as this is already tested in Checkbox.test.tsx (As discussed on Slack)

@ghost ghost marked this pull request as ready for review August 12, 2021 13:44
@ghost ghost requested review from a team and rdott August 12, 2021 13:44
Copy link
Contributor

@rdott rdott left a comment

Choose a reason for hiding this comment

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

🚀

@ghost ghost force-pushed the polaris_video branch from f107d5a to 215341d Compare August 16, 2021 19:59
@ghost ghost merged commit 75c9a54 into main Aug 16, 2021
@ghost ghost deleted the polaris_video branch August 16, 2021 20:13
BPScott pushed a commit that referenced this pull request Aug 17, 2021
This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants