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

Add block pattern dom validation tests #45747

Merged
merged 4 commits into from
Nov 3, 2020

Conversation

deBhal
Copy link
Contributor

@deBhal deBhal commented Sep 18, 2020

Changes proposed in this Pull Request

  • Add some automatic tests for block-patterns

Testing instructions

cd apps/editing-toolkit/editing-toolkit-plugin/
yarn run test:php

Check the README for setup instructions

In addition to verifying that these tests run green with the current patterns, it would also be good to check that they fail with broken patterns by editing some of the patterns in the block-patterns/patterns directory, as sometimes the tests fail silently (You might also want to run npx wp-env run phpunit 'phpunit --debug -c /var/www/html/wp-content/plugins/editing-toolkit-plugin/phpunit.xml.dist' in apps/editing-toolkit to verify that the couple of pages as the new test_patterns_pass_dom_validation runs on each pattern.)

In particular, it should warn about the missing closing double-quotes that started this off in #43817, as well as catching generally broken html (incorrect/unbalanced tags etc).

@matticbot
Copy link
Contributor

@deBhal deBhal requested a review from a team September 18, 2020 10:44
@deBhal deBhal self-assigned this Sep 18, 2020
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Sep 18, 2020
@deBhal deBhal added the Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin label Sep 18, 2020
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@deBhal deBhal changed the title Add/block pattern dom validation tests Add block pattern dom validation tests Sep 18, 2020
@deBhal
Copy link
Contributor Author

deBhal commented Sep 18, 2020

error wp-calypso@0.17.0: The engine "node" is incompatible with this module. Expected version "^v12.18.4". Got "12.18.3"
error Found incompatible module.

Looks like we forgot to update CI to match the new required node version.

@deBhal deBhal force-pushed the add/block-pattern-dom-validation-tests branch 2 times, most recently from 50779d8 to 3272c21 Compare September 20, 2020 22:53
@deBhal deBhal requested a review from a team as a code owner September 20, 2020 22:53
@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D49908-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@deBhal deBhal force-pushed the add/block-pattern-dom-validation-tests branch from 3272c21 to fa7098d Compare September 20, 2020 23:55
Copy link
Contributor

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

Given that the phpunit check:

  1. Ran successfully
  2. Reported a many more tests in the results than there were previously

It looks like this is working well :)

Copy link
Member

@vindl vindl left a comment

Choose a reason for hiding this comment

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

Nice! 🚀

@ramonjd ramonjd merged commit 6f8c28f into master Nov 3, 2020
@ramonjd ramonjd deleted the add/block-pattern-dom-validation-tests branch November 3, 2020 23:52
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Editing Toolkit For issues and PRs that affect the Editing Toolkit plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants