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

Improve maintainability of theme json class tests. #62463

Merged
merged 4 commits into from
Jun 17, 2024

Conversation

tellthemachines
Copy link
Contributor

What?

Partially addresses #60842.

This PR syncs WordPress/wordpress-develop#6734 to Gutenberg. I also adjusted a couple tests that don't exist in core yet to not output base layout styles.

Testing Instructions

  1. Run the PHP tests and check that everything passes.

Testing Instructions for Keyboard

Screenshots or screencast

@tellthemachines tellthemachines added the Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core label Jun 11, 2024
@tellthemachines tellthemachines self-assigned this Jun 11, 2024
Copy link

github-actions bot commented Jun 11, 2024

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: tellthemachines <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: andrewserong <[email protected]>

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

Copy link

github-actions bot commented Jun 11, 2024

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core GitHub repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ phpunit/bootstrap.php
❔ phpunit/class-wp-theme-json-test.php

@@ -1099,12 +1044,12 @@ public function test_get_stylesheet_with_deprecated_feature_level_selectors() {
)
);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm reading the comments above this test and the next one correctly, we should be able to update them to use core blocks now?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's my understanding. We should be able to update these tests to core blocks and remove the filter registering the test block from the phpunit bootstrap.

I can look a little deeper on this tomorrow if that helps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for confirming, I'll give it a go and see if it works!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does the "deprecated" in the test name refer to defining selectors with __experimentalSelector inside the supports array (like Calendar or Button do) as opposed to outside (like Image does)?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe so, it looks like this test was renamed from test_get_stylesheet_with_block_support_feature_level_selectors in #46496 which stabilised that feature and moved it outside of supports.

Copy link
Contributor

Choose a reason for hiding this comment

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

The selectors API feature got merged by a good samaritan before it was actually ready so following threads like #46486 will be tricky as the individual PRs only contain part of the "story".

To answer the question, yes, the deprecated __experimentalSelector properties are what the test was about as we have to maintain backwards compatibility for them unfortunately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks folks, all done! I trimmed the properties output to match what each block supports, and that also makes the strings a more manageable size.

Copy link

Flaky tests detected in 0077769.
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/9475662692
📝 Reported issues:

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Looking good — very cool that real core blocks could be used for both cases of the deprecated and proper selectors, too, that helps simplify things a fair bit.

Just left a couple of tiny nit comments in the strings that are output on test failures, but other than that this LGTM! 🚀

phpunit/class-wp-theme-json-test.php Outdated Show resolved Hide resolved
phpunit/class-wp-theme-json-test.php Outdated Show resolved Hide resolved
@tellthemachines tellthemachines merged commit 4011f4b into trunk Jun 17, 2024
62 checks passed
@tellthemachines tellthemachines deleted the fix/theme-josn-test branch June 17, 2024 05:25
@github-actions github-actions bot added this to the Gutenberg 18.7 milestone Jun 17, 2024
huubl pushed a commit to huubl/gutenberg that referenced this pull request Jun 17, 2024
Co-authored-by: tellthemachines <[email protected]>
Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: andrewserong <[email protected]>
@kevin940726 kevin940726 added [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. CSS Styling Related to editor and front end styles, CSS-specific issues. labels Jun 25, 2024
@tellthemachines tellthemachines removed the Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core label Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [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.

4 participants