Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block #18582

glendaviesnz · 2021-01-28T03:19:51Z

DO NOT MERGE until #18643 is merged back into this PR

Adds testing-framework/react to allow better unit testing of the Jetpack blocks.

See p1HpG7-b3G-p2 for background.

The philosophy behind testing-framework/react is to as much as possible test components in the same way a user would, so testing output rather than implementation details.

This PR adds the framework and adds unit tests using this approach to the WhatsApp block.

A change was needed to the block structure to pull out the settings to a separate component so they can be tested independently of the gutenberg slots that are normally needed to view these components in the ui - but this has the nice side effect of simplifying the block structure also ... but this means that the WhatsApp block functionality should be tested as part of the testing of this PR.

Sample tests can be run in the projects/plugins/jetpack folder with:

yarn jest extensions/blocks/send-a-message/whatsapp-button/test/edit.js
yarn jest extensions/blocks/send-a-message/whatsapp-button/test/configuration.js

matticbot · 2021-01-28T03:19:57Z

Caution: This PR has changes that must be merged to WordPress.com
Hello glendaviesnz! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D56122-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

jetpackbot · 2021-01-28T03:20:51Z

	Warnings
⚠️	The Privacy section is missing for this PR. Please specify whether this PR includes any changes to data or privacy.
⚠️	"Proposed changelog entry" is missing for this PR. Please include any meaningful changes
⚠️	"Testing instructions" are missing for this PR. Please add some

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 1ee629e

projects/plugins/jetpack/tests/jest-globals.js

aaronrobertshaw

This tests well for me as per the instructions 👍

It will be great to get some guidelines around what will qualify as sufficient coverage for individual blocks.

While testing the WhatsApp button, I found that with both the latest Gutenberg plugin and Jetpack, the toolbar menu is squashed and too narrow. This was the same with Jetpack master as well as this PR branch. Perhaps it could be addressed in a separate PR?

Without Gutenberg	With Gutenberg

projects/plugins/jetpack/extensions/blocks/send-a-message/whatsapp-button/test/settings.js

aaronrobertshaw · 2021-02-01T01:34:25Z

projects/plugins/jetpack/extensions/blocks/send-a-message/whatsapp-button/edit.js

@@ -200,57 +60,20 @@ export default function WhatsAppButtonEdit( { attributes, setAttributes, classNa
 		<div className={ getBlockClassNames() }>
 			{ ToolbarGroup && (


Do we still need this check for ToolbarGroup? Was this to support an older version of WP?

It looks like most other blocks in Jetpack don't make this check. This would only tidy the code a tiny bit so it's fine, just more for my understanding.

Yeh it seems like the only reason to have this would be for older versions of gutenberg - @apeatling do you know if that is the case and if it is still needed?

Just thought I'd look this one up. I think the check isn't needed any more, the ToolbarGroup component was added in WordPress/gutenberg#18534 in GB 7.0, and from looking at the Versions in WordPress doc, that was included in WP 5.4.0. According to the readme.txt the required WP version for Jetpack is currently WP 5.5, so we should be good :)

thanks for doing that research @andrewserong - have removed that check now

aaronrobertshaw · 2021-02-01T01:35:58Z

projects/plugins/jetpack/extensions/blocks/send-a-message/whatsapp-button/settings.js

+				onClick={ onToggle }
+				onKeyDown={ openOnArrowDown }
+				icon={ renderMaterialIcon(
+					<Path d="M10.82 12.49c.02-.16.04-.32.04-.49 0-.17-.02-.33-.04-.49l1.08-.82c.1-.07.12-.21.06-.32l-1.03-1.73c-.06-.11-.2-.15-.31-.11l-1.28.5c-.27-.2-.56-.36-.87-.49l-.2-1.33c0-.12-.11-.21-.24-.21H5.98c-.13 0-.24.09-.26.21l-.2 1.32c-.31.12-.6.3-.87.49l-1.28-.5c-.12-.05-.25 0-.31.11l-1.03 1.73c-.06.12-.03.25.07.33l1.08.82c-.02.16-.03.33-.03.49 0 .17.02.33.04.49l-1.09.83c-.1.07-.12.21-.06.32l1.03 1.73c.06.11.2.15.31.11l1.28-.5c.27.2.56.36.87.49l.2 1.32c.01.12.12.21.25.21h2.06c.13 0 .24-.09.25-.21l.2-1.32c.31-.12.6-.3.87-.49l1.28.5c.12.05.25 0 .31-.11l1.03-1.73c.06-.11.04-.24-.06-.32l-1.1-.83zM7 13.75c-.99 0-1.8-.78-1.8-1.75s.81-1.75 1.8-1.75 1.8.78 1.8 1.75S8 13.75 7 13.75zM18 1.01L8 1c-1.1 0-2 .9-2 2v3h2V5h10v14H8v-1H6v3c0 1.1.9 2 2 2h10c1.1 0 2-.9 2-2V3c0-1.1-.9-1.99-2-1.99z" />


While we are refactoring, would this icon path be a candidate to move to its own file?

I was trying to keep the refactoring down to just the parts required for better testing so the PR doesn't get too big, and cluttered with changes that don't affect testing ... but would be good to include in the follow up PR that looks at the dropdown sizing issue for sure.

amamujee · 2021-02-01T18:01:34Z

Would it make sense to break this up into different components @apeatling e.g. phone number, default message and toggle? Would that be easier and more intuitive than jamming it into a single tool?

glendaviesnz · 2021-02-01T22:36:45Z

While testing the WhatsApp button, I found that with both the latest Gutenberg plugin and Jetpack, the toolbar menu is squashed and too narrow. This was the same with Jetpack master as well as this PR branch. Perhaps it could be addressed in a separate PR?

Given @amamujee's question about refactoring this into separate toolbar buttons I think we should defer this to a separate issue/PR

glendaviesnz · 2021-02-01T23:02:12Z

Would it make sense to break this up into different components @apeatling e.g. phone number, default message and toggle? Would that be easier and more intuitive than jamming it into a single tool?

That probably does make sense - I have added a separate issue for this in the View backlog so we can keep this PR related to the addition of the new testing framework - 212-gh-Automattic/view-design

andrewserong

This refactor is looking really nice to me! Love the clean, easy to read tests, and it also feels much cleaner having the configuration broken out to a separate component.

Confirmed that the tests pass when running the following:

yarn test-extensions-single extensions/blocks/send-a-message/whatsapp-button/test/edit.js
yarn test-extensions-single extensions/blocks/send-a-message/whatsapp-button/test/configuration.js

Tested the block in a dotcom simple site to confirm it's still working as expected (I'm not that familiar with the block, but didn't run into any issues). Also tested on a local .org install, and it works correctly, opening up WhatsApp if I have it installed on my phone, otherwise opening the WhatsApp website. 👍

I noticed an unrelated styling issue when the focus is active on the front end of the button when using the twentytwentyone theme, so I've raised it separately in #18688

anomiex · 2021-02-04T21:45:39Z

Kraft asked me to look this over with an eye towards whether it fits in with the work we're doing to improve the testing infrastructure of the monorepo as a whole. I'm happy to say that I see nothing to be concerned about here from that perspective, as this is only adding to the existing extension tests within Jetpack the plugin (versus something like creating a whole new kind of test or test runner).

I haven't reviewed the specific changes here in detail, but 👍 for adding better tests!

projects/plugins/jetpack/package.json

brbrr

It looks good to me. Although it would need merge from master/rebase

Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>

…ettings

…alidation (#18643) Co-authored-by: Glen Davies <glen.davies@a8c.com>

glendaviesnz · 2021-02-17T04:14:17Z

It looks good to me. Although it would need merge from master/rebase

Thanks @brbrr, it is rebased now if you are able to give it a tick please.

glendaviesnz · 2021-02-21T19:29:34Z

@jeherve, @brbrr I think this is good to go, would be good to get it in before the code freeze today, just needs a crew approval.

github-actions · 2021-02-22T09:25:36Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
🔴 Add testing instructions.
🔴 Include a changelog entry for any meaningful change.
🔴 Specify whether this PR includes any changes to data or privacy.

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

If you are an automattician, once your PR is ready for review add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.undefined

jetpack plugin:

Next scheduled release: March 2, 2021.
Scheduled code freeze: February 22, 2021

jeherve · 2021-02-22T09:48:30Z

Merging this now. I'll let you handle the matching WordPress.com diff.

jeherve · 2021-02-24T17:32:39Z

@glendaviesnz Could you commit the matching WordPress.com diff? Thank you!

glendaviesnz · 2021-02-24T20:38:34Z

Could you commit the matching WordPress.com diff? Thank you!

Done.

jeherve · 2021-02-25T07:06:19Z

r221623-wpcom

glendaviesnz added the [Status] Needs Team Review label Jan 28, 2021

glendaviesnz requested a review from a team January 28, 2021 03:19

glendaviesnz self-assigned this Jan 28, 2021

matticbot added Touches WP.com Files labels Jan 28, 2021

glendaviesnz changed the title ~~[WIP} Set up testing-library/react and add example tests to WhatsApp block~~ [WIP] Set up testing-library/react and add example tests to WhatsApp block Jan 28, 2021

glendaviesnz added the [Status] In Progress label Jan 28, 2021

jeherve added the [Block] Send a message label Jan 28, 2021

glendaviesnz commented Jan 28, 2021

View reviewed changes

projects/plugins/jetpack/tests/jest-globals.js Outdated Show resolved Hide resolved

glendaviesnz marked this pull request as ready for review January 31, 2021 19:34

glendaviesnz changed the title ~~[WIP] Set up testing-library/react and add example tests to WhatsApp block~~ Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block Jan 31, 2021

glendaviesnz removed the [Status] In Progress label Jan 31, 2021

glendaviesnz force-pushed the try/jetpack-block-additional-tests branch from 697470b to 05eca89 Compare January 31, 2021 20:00

aaronrobertshaw previously approved these changes Feb 1, 2021

View reviewed changes

glendaviesnz dismissed aaronrobertshaw’s stale review via 3f6b006 February 1, 2021 22:34

glendaviesnz mentioned this pull request Feb 2, 2021

Add a block fixtures framework to allow automated checking of block validation #18643

Merged

andrewserong previously approved these changes Feb 3, 2021

View reviewed changes

andrewserong added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Team Review labels Feb 3, 2021

jeherve requested a review from brbrr February 3, 2021 16:59

jeherve added this to the 9.5 milestone Feb 3, 2021

kraftbj requested a review from anomiex February 4, 2021 21:18

brbrr reviewed Feb 8, 2021

View reviewed changes

projects/plugins/jetpack/package.json Outdated Show resolved Hide resolved

glendaviesnz dismissed andrewserong’s stale review via 5cf8b8d February 8, 2021 21:22

brbrr reviewed Feb 16, 2021

View reviewed changes

Glen Davies and others added 10 commits February 17, 2021 15:16

Test

4e79b47

fix broken attributes prop

6c2e9ed

Finish setup of initial tests

84f44ec

Fix settings to be compat with existing tests

871e26d

fix typo

59fd0f0

Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>

Change settings to configuration to avoid confusion with base block s…

40fdd28

…ettings

remove pointless script setting

f075eeb

Change configuration to match changes in #18732

01281ae

Remove check for toolbar group

3fa52d0

Add a block fixtures framework to allow automated checking of block v…

1ee629e

…alidation (#18643) Co-authored-by: Glen Davies <glen.davies@a8c.com>

glendaviesnz force-pushed the try/jetpack-block-additional-tests branch from 5125172 to 1ee629e Compare February 17, 2021 02:19

glendaviesnz removed the DO NOT MERGE don't merge it! label Feb 17, 2021

jeherve approved these changes Feb 22, 2021

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 22, 2021

github-actions bot added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Feb 22, 2021

jeherve merged commit b6b96c3 into master Feb 22, 2021

jeherve deleted the try/jetpack-block-additional-tests branch February 22, 2021 09:48

matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Feb 22, 2021

jeherve removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Status] Needs Changelog labels Feb 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block #18582

Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block #18582

glendaviesnz commented Jan 28, 2021 •

edited by apeatling

Loading

matticbot commented Jan 28, 2021

jetpackbot commented Jan 28, 2021 •

edited

Loading

aaronrobertshaw left a comment •

edited by glendaviesnz

Loading

aaronrobertshaw Feb 1, 2021

glendaviesnz Feb 1, 2021

andrewserong Feb 3, 2021

glendaviesnz Feb 10, 2021

aaronrobertshaw Feb 1, 2021

glendaviesnz Feb 1, 2021

amamujee commented Feb 1, 2021

glendaviesnz commented Feb 1, 2021

glendaviesnz commented Feb 1, 2021

andrewserong left a comment

anomiex commented Feb 4, 2021

brbrr left a comment •

edited

Loading

glendaviesnz commented Feb 17, 2021

glendaviesnz commented Feb 21, 2021

github-actions bot commented Feb 22, 2021

jeherve commented Feb 22, 2021

jeherve commented Feb 24, 2021

glendaviesnz commented Feb 24, 2021

jeherve commented Feb 25, 2021

		@@ -200,57 +60,20 @@ export default function WhatsAppButtonEdit( { attributes, setAttributes, classNa
		<div className={ getBlockClassNames() }>
		{ ToolbarGroup && (

Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block #18582

Set up testing-library/react for testing jetpack blocks and add example tests to WhatsApp block #18582

Conversation

glendaviesnz commented Jan 28, 2021 • edited by apeatling Loading

matticbot commented Jan 28, 2021

jetpackbot commented Jan 28, 2021 • edited Loading

aaronrobertshaw left a comment • edited by glendaviesnz Loading

Choose a reason for hiding this comment

aaronrobertshaw Feb 1, 2021

Choose a reason for hiding this comment

glendaviesnz Feb 1, 2021

Choose a reason for hiding this comment

andrewserong Feb 3, 2021

Choose a reason for hiding this comment

glendaviesnz Feb 10, 2021

Choose a reason for hiding this comment

aaronrobertshaw Feb 1, 2021

Choose a reason for hiding this comment

glendaviesnz Feb 1, 2021

Choose a reason for hiding this comment

amamujee commented Feb 1, 2021

glendaviesnz commented Feb 1, 2021

glendaviesnz commented Feb 1, 2021

andrewserong left a comment

Choose a reason for hiding this comment

anomiex commented Feb 4, 2021

brbrr left a comment • edited Loading

Choose a reason for hiding this comment

glendaviesnz commented Feb 17, 2021

glendaviesnz commented Feb 21, 2021

github-actions bot commented Feb 22, 2021

jeherve commented Feb 22, 2021

jeherve commented Feb 24, 2021

glendaviesnz commented Feb 24, 2021

jeherve commented Feb 25, 2021

glendaviesnz commented Jan 28, 2021 •

edited by apeatling

Loading

jetpackbot commented Jan 28, 2021 •

edited

Loading

aaronrobertshaw left a comment •

edited by glendaviesnz

Loading

brbrr left a comment •

edited

Loading