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

Remove phpunit tests for features backported to Core #58776

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

youknowriad
Copy link
Contributor

What?

There are some features that start in Gutenberg and make their way into Core and eventually their code gets removed from Gutenberg. For these features, we generally unit test them within the plugin but when the code makes its way into Core (with its tests), there's no need to keep the tests in Core anymore. This PR just removes some of these old tests that are not relevant for the Gutenberg plugin anymore.

cc @WordPress/gutenberg-core

@youknowriad youknowriad added [Type] Build Tooling Issues or PRs related to build tooling Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts labels Feb 7, 2024
@youknowriad youknowriad self-assigned this Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 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.

Core SVN

Core Committers: Use this line as a base for the props when committing in SVN:

Props youknowriad, gziolo, jsnajdr, ntsekouras, ramonopoly, andraganescu.

GitHub Merge commits

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: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: draganescu <andraganescu@git.wordpress.org>

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

* @group restapi
* @group navigation
*/
class Gutenberg_REST_Navigation_Fallback_Controller_Test extends WP_Test_REST_Controller_Testcase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is not in Gutenberg anymore. cc @getdave for confirmation. Let's also confirm that the tests were backported properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

They have been removed from Gutenberg compat now I believe yes 👍

@@ -1,276 +0,0 @@
<?php

class Gutenberg_REST_Templates_Controller_Test extends WP_Test_REST_Controller_Testcase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is not in Gutenberg anymore. cc @ntsekouras @ramonjd for confirmation. Let's also confirm that the tests were backported properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The ones I had added have been backported: WordPress/wordpress-develop#3221

* @package WordPress
*/

class WP_Get_Block_CSS_Selector_Test extends WP_UnitTestCase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is not in Gutenberg anymore. cc @aaronrobertshaw for confirmation. Let's also confirm that the tests were backported properly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the ping 👍

I can confirm wp_get_block_css_selector is now only defined in core. This test was also backported in WordPress/wordpress-develop#4655 and the corresponding file in core still matches the test file being removed in this PR.

*
* @group blocks
*/
class Tests_Blocks_RenderHookedBlocks extends WP_UnitTestCase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is in Core now. cc @gziolo for confirmation. Let's also confirm that the tests were backported properly.

Copy link
Member

Choose a reason for hiding this comment

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

This class is testing the code that's bridging the gaps between WP core and Gutenberg. We can remove it unless @ockham plans to apply any changes in the compat layer.

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 not wrong, the test hasn't been touched since 6.4 which means, that if we want to make any changes now, we'll have to do add a file to 6.5 folder and modify this file. It seems actually better to remove it that way, it clarifies for the implementor what part of the test need to be backported to Core for the "new" changes.

I can restore it though, just wanted to explain my reasoning here.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a good reasoning 👍🏻

@youknowriad youknowriad force-pushed the remove/backported-tests branch from 5d34aa7 to d0ff985 Compare February 8, 2024 07:45
Copy link

github-actions bot commented Feb 8, 2024

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

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

This looks good according to my knowledge of what has been landed in core.

@youknowriad youknowriad merged commit 9560e41 into trunk Feb 8, 2024
57 checks passed
@youknowriad youknowriad deleted the remove/backported-tests branch February 8, 2024 08:45
@github-actions github-actions bot added this to the Gutenberg 17.7 milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts [Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants