Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 16 months ago

#57197 closed enhancement (fixed)

Add some basic test coverage for the Gutenberg plugin

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

Description

During preparation for the 6.1 release, we saw an issue with duplicated function names in Core and the Gutenberg plugin which would cause fatal errors when running the stable version of the Gutenberg plugin on a WordPress install that was running Core trunk.

Specifically, this was caused by [54118] (which was based on this PR): It introduced wp_add_global_styles_for_blocks to Core, while the same function was also still declared in Gutenberg (with no function_exists guards or the like). Thus, it lead to a naming collision.

We already have some safeguards in place -- e.g. all PRs in the Gutenberg repo are run against Core trunk. While that did cause all subsequent PRs filed in the Gutenberg repo to fail (and eventually prompted @wildworks to file a fix), it went unnoticed by Core developers for a while, since there are no safeguards on the Core side.

I thus propose adding minimal coverage to wordpress-develop's CI that attempts to activate and install the Gutenberg plugin. That would be enough to catch any fatal errors as described above, while adding minimal overhead on the Core/wordpress-develop side.

IMO, it's reasonable to add this for a feature plugin whose code is by definition planned to be eventually merged into Core.

Change History (11)

#1 @Bernhard Reiter
17 months ago

cc/ @desrosj @hellofromTonya

This ticket was mentioned in PR #3679 on WordPress/wordpress-develop by @Bernhard Reiter.


17 months ago
#2

  • Keywords has-patch has-unit-tests added

Try to activate and install the Gutenberg plugin. This will catch naming collisions between Core and GB (see Core ticket 57197 for an example).

To test:

  1. Check out this branch and run npm run env:start.
  2. Run npm run test:e2e -- tests/e2e/specs/gutenberg-plugin.test.js locally and verify that it passes.
  3. Add a function definition to Core that's also present in Gutenberg (without function_exists guards or the like). I use the following:

{{{diff
diff --git a/src/wp-settings.php b/src/wp-settings.php
index 3ed93b2f36..d2fdafc74b 100644
--- a/src/wp-settings.php
+++ b/src/wp-settings.php
@@ -8,6 +8,10 @@

  • @package WordPress */


+function gutenberg_style_engine_get_styles() {
+ die();
+}
+

/

  • Stores the location of the WordPress directory of functions, classes, and core content. *

}}}

  1. Run npm run test:e2e -- tests/e2e/specs/gutenberg-plugin.test.js again and watch it fail.

Trac ticket: https://core.trac.wordpress.org/ticket/57197

@Bernhard Reiter commented on PR #3679:


17 months ago
#3

As an alternative, we could consider making a WP CLI based test.

Pro:

  • Probably runs faster than an e2e test.

Con:

  • Doesn't really seem to fit into any of our current test frameworks (neither PHP/JS unit tests nor e2e tests).

@fullofcaffeine commented on PR #3679:


17 months ago
#4

This is great! Do you think it's a better approach than running core tests in Gutenberg PRs (like I originally discussed here: https://wordpress.slack.com/archives/C03B0H5J0/p1650590706137509), or perhaps they're not mutually exclusive? I'd say they are complementary, so we could also try a similar approach in the GB repo.

@Bernhard Reiter commented on PR #3679:


17 months ago
#5

This is great! Do you think it's a better approach than running core tests in Gutenberg PRs (like I originally discussed here: https://wordpress.slack.com/archives/C03B0H5J0/p1650590706137509), or perhaps they're not mutually exclusive? I'd say they are complementary, so we could also try a similar approach in the GB repo.

EDIT: I now see this is not really for running core GB PHPUnit tests in core, but only for activating the plugin, which is also great but not the same thing I was talking about above when I mentioned core tests in GB. Still, this is a great step forward!

Thank you! Yeah, so this would be complementary to running Core's tests on Gutenberg PRs. (The latter sounds a bit familiar BTW: https://github.com/WordPress/gutenberg/issues/26418 😬)

@fullofcaffeine commented on PR #3679:


17 months ago
#6

(The latter sounds a bit familiar BTW: https://github.com/WordPress/gutenberg/issues/26418 grimacing)

Oh gosh! I didn't recall this PR, thanks for pointing that out! We should definitely resume work there!

#7 @Bernhard Reiter
17 months ago

  • Owner set to Bernhard Reiter
  • Resolution set to fixed
  • Status changed from new to closed

In 54913:

Build/Test Tools: Add basic e2e coverage for Gutenberg.

Add some minimal e2e test coverage to install and activate the Gutenberg plugin.

This will catch naming collisions between Core and Gutenberg and help avoid crashing WordPress installations that run the stable version of the Gutenberg plugin on top of Core trunk.

Props costdev.
Fixes #57197.

@Bernhard Reiter commented on PR #3679:


17 months ago
#8

Thank you for reviewing @costdev!

Committed to Core in https://core.trac.wordpress.org/changeset/54913/.

#9 @Bernhard Reiter
17 months ago

In 54914:

Build/Test Tools: Stylistic changes to Gutenberg e2e test.

Make some stylistic changes (multiline comment formatting, test description) that had been suggested during code review and that didn't make it into the previous commit.

Props costdev.
Follows [54913].
See #57197.

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


17 months ago

#11 @desrosj
16 months ago

  • Milestone changed from Awaiting Review to 6.2
Note: See TracTickets for help on using tickets.