Make WordPress Core

Opened 2 years ago

Closed 13 months ago

Last modified 13 months ago

#55571 closed task (blessed) (fixed)

Gallery block fixtures in phpunit tests need updating

Reported by: glendaviesnz's profile glendaviesnz Owned by: glendaviesnz's profile glendaviesnz
Milestone: 6.2 Priority: normal
Severity: normal Version: 5.9
Component: Build/Test Tools Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

In 5.9 the Gallery block was updated to use nested image blocks, and to be wrapped in a figure tag, but the block fixtures still us the old ul and internal image format.

The fixtures should be updated to the new block format.

Attachments (1)

Screenshot 2022-04-26 at 08.54.51.png (227.9 KB) - added by gziolo 23 months ago.

Download all attachments as: .zip

Change History (21)

This ticket was mentioned in PR #2579 on WordPress/wordpress-develop by glendaviesnz.


2 years ago
#1

  • Keywords has-unit-tests added

The Gallery block unit tests still use the old format, so this PR updates it to the new format that uses nested Image blocks.
---
Fixes: https://core.trac.wordpress.org/ticket/55571
## To test

Run:

  • npm run test:php -- --filter Tests_Blocks_wpBlockParser
  • npm run test:php -- --filter Tests_Blocks_Render

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

#2 @costdev
2 years ago

  • Component changed from Gallery to Build/Test Tools
  • Milestone changed from Awaiting Review to 6.0
  • Type changed from enhancement to task (blessed)

#3 @gziolo
23 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 53261:

Tests: Update Gallery block unit tests to new gallery format

The Gallery block unit tests still use the old format, so this updates it to the new format that uses nested Image blocks.

Props glendaviesnz.
Fixes #55571.

#5 @peterwilsoncc
23 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I missed this earlier, sorry.

I think it would be good to maintain tests for both the updated and legacy format. That will ensure that any code changes don't break the legacy format on sites that haven't upgraded.

Will putting the fixture back as core__gallery-legacy.* cause problems?

I'll reopen this for now while it's discussed.

#6 @glendaviesnz
23 months ago

Will putting the fixture back as coregallery-legacy.* cause problems?

Good question, @gziolo do you know what the process has been in the past for deprecated block formats in these fixtures - the only other deprecated fixtures here are for columns and paragraphs, and these only have a single deprecation, but there have been many other deprecations that aren't covered at all in these fixtures?

#7 @gziolo
23 months ago

I didn't think about keeping both versions, it makes a lot of sense!

I would have to double-check the implementation in the core, but in Gutenberg, we have multiple versions per block, including a few deprecated versions so what @peterwilsoncc suggested would work.

#8 @peterwilsoncc
23 months ago

Thanks folks, let's go with __deprecated-1 rather than -legacy so the naming convention is consistent between repositories.

Tests can go in at any time so this can wait until after RC 1 if you have other higher priority tasks to do.

#9 @glendaviesnz
23 months ago

Ok, will take a look at adding these back sometime in the next few days.

#10 @hellofromTonya
23 months ago

  • Milestone changed from 6.0 to 6.1
  • Version changed from trunk to 5.9

Though tests can go in at anytime, moving this to 6.1 milestone to clear 6.0 for RC1 (which is tomorrow).

#11 @desrosj
18 months ago

I came across this one scrubbing tickets today, and I'm wondering. Would this help us find and prevent backwards compatibility problems such as the ones described in #53617? Or am I misunderstanding what the tests check for?

#12 @desrosj
17 months ago

  • Milestone changed from 6.1 to 6.2

RC1 is in a few hours. Going to punt this to 6.2.

This ticket was mentioned in PR #4140 on WordPress/wordpress-develop by @peterwilsoncc.


13 months ago
#13

This reverts commit 3cc94be23b31d8eb22c81a615b749f82bd6de2ad.

And then will do more.

https://core.trac.wordpress.org/ticket/55571

#14 @peterwilsoncc
13 months ago

In the linked pull request I have:

  • added all the deprecated gallery tests included in the Gutenberg repo to duplicate them in wp-develop
  • generated the additional .server.html files used in wp-dev but not the gutenberg repo
  • modified the server versions so they pass, however I don't think they are correct as I've noted on the pull request.

#15 @gziolo
13 months ago

In 55439:

Editor: Only add layout classes to inner wrapper if block is a container

Backport of https://github.com/WordPress/gutenberg/issues/48606, which fixes an issue found in adding fixtures for deprecated Gallery block versions in #55571.

See #55571.
Fixes #57831.
Props isabel_brison, Mamaduka, dasnitesh780.

@peterwilsoncc commented on PR #4140:


13 months ago
#16

I think the null blocks come from end of file empty lines.
I tried using trim to remove them but it broke a bunch of other tests that already had trailing null blocks recorded.

This ticket was mentioned in Slack in #core by costdev. View the logs.


13 months ago

This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.


13 months ago

#19 @peterwilsoncc
13 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 55471:

Build/Tests Tools: Add unit tests for Gallery blocks.

Introduces unit tests for the following blocks

  • Gallery block with caption
  • Gallery block, deprecations 1 thru 7

Updates the unit tests for the following blocks to match the counterparts stored in the Gutenberg repository:

  • Gallery block
  • Gallery block with columns

Modifies Tests_Blocks_Render::test_do_block_output() to ignore white space at the end of lines to account for whitespace equivalence in HTML.

Props peterwilsoncc, isabel_brison, gziolo.
Fixes #55571.

Note: See TracTickets for help on using tickets.