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

Testing: Add integration tests for blocks with deprecations #15268

Merged
merged 5 commits into from
May 7, 2019

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Apr 30, 2019

Description

Inspired by the comment from @ellatrix in #15057 (comment):

It's a bit hard to review this without any tests for deprecated blocks. Would it be possible to add some simple tests to see if deprecated block structures are converted correctly?

This PR adds integration tests for blocks with deprecations:

  • Gallery block
  • Image block
  • Pullquote block
  • Quote block

Paragraph block was covered previously.

Implementation details

I used #3996 to replicate the example for deprecated Gallery block. I figured out there are two issues:

  • we use now align hook which filters out none value but injects other values otherwise
  • images attribute uses a newer definition which makes it useless

I fixed both issues in this PR. You can test it manually by opening Gutenberg's Code Editor and pasting the following snippet used in the test:

<!-- wp:core/gallery {"ids":[null,null],"columns":2} -->
<div class="wp-block-gallery alignnone columns-2 is-cropped">
	<figure class="blocks-gallery-image">
		<img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==" alt="title" />
	</figure>
	<figure class="blocks-gallery-image">
		<img src="data:image/jpeg;base64,/9j/2wBDAAMCAgICAgMCAgIDAwMDBAYEBAQEBAgGBgUGCQgKCgkICQkKDA8MCgsOCwkJDRENDg8QEBEQCgwSExIQEw8QEBD/yQALCAABAAEBAREA/8wABgAQEAX/2gAIAQEAAD8A0s8g/9k=" alt="title" />
	</figure>
</div>
<!-- /wp:core/gallery -->

How has this been tested?

npm run test-unit

@gziolo gziolo added [Feature] Blocks Overall functionality of blocks [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Apr 30, 2019
@gziolo gziolo self-assigned this Apr 30, 2019
@gziolo gziolo requested a review from ellatrix April 30, 2019 14:38
@gziolo
Copy link
Member Author

gziolo commented Apr 30, 2019

@aduth - I tried to add tests for List and Heading blocks but it looks like deprecated versions left for those blocks don't work anymore. It looks like #7349 was added temporary and wasn't planned to be supported for long. Does it mean we can safely remove them?

@ellatrix
Copy link
Member

ellatrix commented May 1, 2019

I don't see any code that adds it to the generation of the fixtures. How does it work?

@@ -0,0 +1,10 @@
<!-- wp:core/gallery {"ids":[null,null],"columns":2} -->
Copy link
Member

@ellatrix ellatrix May 1, 2019

Choose a reason for hiding this comment

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

There's multiple deprecations, but I only see one being added here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's time-consuming to backport all those missing fixtures.

@gziolo
Copy link
Member Author

gziolo commented May 6, 2019

I don't see any code that adds it to the generation of the fixtures. How does it work?

It's handled out of the box based on the patterns in the fixtures folder:
https://github.com/WordPress/gutenberg/blob/master/test/integration/full-content/full-content.spec.js#L30
https://github.com/WordPress/gutenberg/blob/master/packages/e2e-tests/fixtures/utils.js#L32-L44

@gziolo
Copy link
Member Author

gziolo commented May 6, 2019

I tried the following code for Quote but it doesn't work as the currently active save handler intercepts it:

<!-- wp:core/quote {"align":"left","style":2} -->
<blockquote class="wp-block-quote is-large" style="text-align:left">
	<p>Testing deprecated quote block...</p>
	<cite>...with a caption</cite>
</blockquote>
<!-- /wp:core/quote -->

@gziolo gziolo force-pushed the add/blocks-depreacated-tests branch from eb25221 to 4658ef3 Compare May 6, 2019 12:07
@gziolo
Copy link
Member Author

gziolo commented May 6, 2019

I was able to cover all Pullquote deprecations with tests in 4658ef3.

I will look into Image and Gallery blocks tomorrow.

@gziolo gziolo force-pushed the add/blocks-depreacated-tests branch from 4658ef3 to bf87e3a Compare May 7, 2019 08:12
@gziolo
Copy link
Member Author

gziolo commented May 7, 2019

I will look into Image and Gallery blocks tomorrow.

It's now covered with 9e84880 and 4658ef3.

I removed one of the deprecations from Gallery block as it will never catch block definition because of how the newer one builds on top of it. The related change was introduced in: #11540.

{"core\/archives":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"displayAsDropdown":{"type":"boolean","default":false},"showPostCounts":{"type":"boolean","default":false}}},"core\/block":{"attributes":{"ref":{"type":"number"}}},"core\/calendar":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"month":{"type":"integer"},"year":{"type":"integer"}}},"core\/categories":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"displayAsDropdown":{"type":"boolean","default":false},"showHierarchy":{"type":"boolean","default":false},"showPostCounts":{"type":"boolean","default":false}}},"core\/latest-comments":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"commentsToShow":{"type":"number","default":5,"minimum":1,"maximum":100},"displayAvatar":{"type":"boolean","default":true},"displayDate":{"type":"boolean","default":true},"displayExcerpt":{"type":"boolean","default":true}}},"core\/latest-posts":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"categories":{"type":"string"},"postsToShow":{"type":"number","default":5},"displayPostContent":{"type":"boolean","default":false},"displayPostContentRadio":{"type":"string","default":"excerpt"},"excerptLength":{"type":"number","default":55},"displayPostDate":{"type":"boolean","default":false},"postLayout":{"type":"string","default":"list"},"columns":{"type":"number","default":3},"order":{"type":"string","default":"desc"},"orderBy":{"type":"string","default":"date"}}},"core\/legacy-widget":{"attributes":{"className":{"type":"string"},"identifier":{"type":"string"},"instance":{"type":"object"},"isCallbackWidget":{"type":"boolean"}}},"core\/rss":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"columns":{"type":"number","default":2},"blockLayout":{"type":"string","default":"list"},"feedURL":{"type":"string","default":""},"itemsToShow":{"type":"number","default":5},"displayExcerpt":{"type":"boolean","default":false},"displayAuthor":{"type":"boolean","default":false},"displayDate":{"type":"boolean","default":false},"excerptLength":{"type":"number","default":55}}},"core\/search":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"label":{"type":"string","default":"Search"},"placeholder":{"type":"string","default":""},"buttonText":{"type":"string","default":"Search"}}},"core\/shortcode":{"attributes":{"text":{"type":"string","source":"html"}}},"core\/tag-cloud":{"attributes":{"align":{"type":"string","enum":["left","center","right","wide","full"]},"className":{"type":"string"},"taxonomy":{"type":"string","default":"post_tag"},"showTagCounts":{"type":"boolean","default":false}}}}
Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like npm run fixtures:regenerate wasn't executed after Latest Posts block was updated.

@@ -95,47 +142,39 @@ export const settings = {
);
},
},
{
Copy link
Member Author

Choose a reason for hiding this comment

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

The previous definition is nearly the same (it only adds one optional attribute in HTML output) making this entry redundant. It can be safely removed.

@ellatrix
Copy link
Member

ellatrix commented May 7, 2019

It's handled out of the box based on the patterns in the fixtures folder:

Then why weren't they generated before? I also see changes to fixtures. What makes it so that they are now generated in this PR?

@gziolo
Copy link
Member Author

gziolo commented May 7, 2019

Then why weren't they generated before? I also see changes to fixtures. What makes it so that they are now generated in this PR?

You need to manual craft HTML input files yourself. See all details explained in this file: https://github.com/WordPress/gutenberg/blob/master/packages/e2e-tests/fixtures/blocks/README.md

@ellatrix
Copy link
Member

ellatrix commented May 7, 2019

@gziolo Ah, I see. Is there any way we can error if someone adds a deprecation, but doesn't add a HTML input file to test the deprecated code?

Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

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

Love that deprecations are now tested!

@gziolo
Copy link
Member Author

gziolo commented May 7, 2019

@gziolo Ah, I see. Is there any way we can error if someone adds a deprecation, but doesn't add a HTML input file to test the deprecated code?

Yes, that would be nice to have. It's now automatically enforced to include one HTML file per block's folder. We probably can expand on that once we have standardized deprecated.js file for blocks.

@gziolo gziolo merged commit 31e2144 into master May 7, 2019
@gziolo gziolo deleted the add/blocks-depreacated-tests branch May 7, 2019 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants