Skip to content

Conversation

@sirreal
Copy link
Member

@sirreal sirreal commented Nov 3, 2025

This PR must wait until the previous WordPress version is 6.9.

What?

Instead of using fragile string equality comparisons, this relies on HTML comparisons with Core's assertEqualHTML. This is a semantic comparison of the HTML that is resilient to syntactic differences between equivalent text like & vs ' (which caused problems in the tests).

This prevents the issue that appeared in Gutenberg tests after WordPress/wordpress-develop#10143 was merged.

This reverts commit 7e69ee6 from PR #72236 and implements an alternate approach.

Testing Instructions

CI passes.

@sirreal sirreal added the [Type] Flaky Test Auto-generated flaky test report issue label Nov 3, 2025
@sirreal sirreal self-assigned this Nov 3, 2025
@sirreal sirreal added the No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core label Nov 3, 2025
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

Flaky tests detected in 3d624fb.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19041545962
📝 Reported issues:

@sirreal sirreal requested review from ntsekouras and talldan November 3, 2025 17:38
@sirreal sirreal marked this pull request as ready for review November 3, 2025 17:38
@github-actions
Copy link

github-actions bot commented Nov 3, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sirreal <[email protected]>
Co-authored-by: ntsekouras <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

),
),
'expected_wrapper' => '<div class="has-background" style="background-image:url(' . $apos . 'https://example.com/image.jpg' . $apos . ');background-size:cover;">Content</div>',
'expected_wrapper' => '<div class="has-background" style="background-image:url(&#039;https://example.com/image.jpg&#039;);background-size:cover;">Content</div>',
Copy link
Member Author

Choose a reason for hiding this comment

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

These could become the ' character for more legible tests:

Suggested change
'expected_wrapper' => '<div class="has-background" style="background-image:url(&#039;https://example.com/image.jpg&#039;);background-size:cover;">Content</div>',
'expected_wrapper' => '<div class="has-background" style="background-image:url(\'https://example.com/image.jpg\');background-size:cover;">Content</div>',

@sirreal sirreal force-pushed the fix/php-tests-html-with-expect-html branch from 3d624fb to 558893d Compare November 3, 2025 17:42
@ntsekouras
Copy link
Contributor

Thanks for the PR! I thought we had tried that before when investigating and the CI didn't pass. Wasn't that the case?

CI passes.

The difference between now and back then is that the previous failures were from PHP previous major version, which as of #72506 they are not run on GB PR before merged. Have you tested those jobs somehow?

@sirreal sirreal requested a review from desrosj as a code owner November 4, 2025 11:09
@sirreal
Copy link
Member Author

sirreal commented Nov 4, 2025

The difference between now and back then is that the previous failures were from PHP previous major version, which as of #72506 they are not run on GB PR before merged. Have you tested those jobs somehow?

Thank you for noticing that! Indeed, the test method was unavailable in the previous version and we see an error:

Error: Call to undefined method WP_Block_Supports_Background_Test::assertEqualHTML()

This change would be good to make but unfortunately we'll have to wait 🙂

@sirreal sirreal marked this pull request as draft November 4, 2025 11:50
@sirreal sirreal removed the request for review from desrosj November 4, 2025 11:51
@github-actions github-actions bot added the Stale label Nov 20, 2025
@github-actions github-actions bot closed this Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No Core Sync Required Indicates that any changes do not need to be synced to WordPress Core Stale [Type] Flaky Test Auto-generated flaky test report issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants