Skip to content

Ensure block style variation is available in the cached global stylesheet - #81852

Closed
tellthemachines wants to merge 1 commit into
add/list-layout-supportfrom
fix/stupid-tests
Closed

tellthemachines wants to merge 1 commit into
add/list-layout-supportfrom
fix/stupid-tests

Conversation

@tellthemachines

@tellthemachines tellthemachines commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

What?

Addresses test failures on #68002 by ensuring the block style variation used in the failing test is added to the global stylesheet at the point it's cached. Caching happens in layout as soon as a block with layout is encountered; when #68002 added layout to the List block, the caching started happening in phpunit/block-bindings-test.php which runs before layout, so the block style variation registered in the layout tests never made it into the stylesheet.

I should add that running the failing test in a separate process won't work in Gutenberg because our test setup explicitly sets WP_DEBUG=true and that doesn't seem to go well with spinning up a new process (because we're redeclaring it)

Testing Instructions

The PHP tests should pass.

Use of AI Tools

Debugged and implemented with kimi 3.0/copilot.

@github-actions

github-actions Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

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 <isabel_brison@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>

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

@andrewserong andrewserong added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Aug 20, 2026

@andrewserong andrewserong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I should add that running the failing test in a separate process won't work in Gutenberg because our test setup explicitly sets WP_DEBUG=true and that doesn't seem to go well with spinning up a new process (because we're redeclaring it)

With that in mind, this seems okay to add as a short-term fix to me. Longer-term, it'd be good to either get the tests working in separate processes or (if it's appropriate) see if the layout code should allow the cache to be reset.

Something that came up when running this through a Claude session: this might not work with the multisite tests that run on trunk as they use a different xml file for configuration. Also, it looks like BeforeFirstTestHook is deprecated as of PHPUnit 10, so I'm wondering if this would be better to be inlined in bootstrap.php or called as a normal function defined in the fixture file, or does it need to run at a particular time in the testing lifecycle?

@ramonjd

ramonjd commented Aug 20, 2026 •

Copy link
Copy Markdown
Member

Is the static var in gutenberg_render_layout_support_flag() the problem I wonder.

Claude told me this:

gutenberg_get_global_settings() caches the same kind of merged theme.json data, but it puts it in the object cache under the theme_json group, and _gutenberg_clean_theme_json_caches() clears it on switch_theme and start_previewing_theme (global-styles-and-settings.php#L345-L355). So we have two caches of the same data on the same code path, and only one of them can be cleared. So a request that switches theme or blog and then renders a layout block uses the old theme’s blockGap

So an alternative is to cache it in the theme json caches then clear it in _gutenberg_clean_theme_json_caches()

 function gutenberg_render_layout_support_flag( $block_content, $block ) {
-	static $global_styles = null;
-
-		if ( null === $global_styles ) {
-			$global_styles = gutenberg_get_global_styles();
-		}
+		$global_styles = wp_cache_get( 'gutenberg_layout_global_styles', 'theme_json' );
+		if ( false === $global_styles || WP_DEBUG ) {
+			$global_styles = gutenberg_get_global_styles();
+			wp_cache_set( 'gutenberg_layout_global_styles', $global_styles, 'theme_json' );
+		}
 	wp_cache_delete( 'gutenberg_get_global_styles_base_custom_css', 'theme_json' );
+	wp_cache_delete( 'gutenberg_layout_global_styles', 'theme_json' );

Sorry for the dump. I'll come back and test this tommorrow.

@tellthemachines

Copy link
Copy Markdown
Contributor Author

this might not work with the multisite tests that run on trunk as they use a different xml file for configuration.

Do these run on CI? The PHP tests are all passing on this branch.

So an alternative is to cache it in the theme json caches then clear it in _gutenberg_clean_theme_json_caches()

I can give that a go and see if it works! The important thing is we don't have to call gutenberg_get_global_styles repeatedly.

@andrewserong

Copy link
Copy Markdown
Contributor

Do these run on CI? The PHP tests are all passing on this branch.

Yes, they run on trunk. PRs only run a limited set of PHP tests (for speed I think?) whereas the full suite runs on trunk. Here's where that's defined:

exclude:
# On PRs: Only test PHP 7.4 and 8.5, single-site, latest WP
- event: 'pull_request'
php: '8.0'
- event: 'pull_request'
php: '8.1'
- event: 'pull_request'
php: '8.2'
- event: 'pull_request'
php: '8.3'
- event: 'pull_request'
php: '8.4'
- event: 'pull_request'
multisite: true
- event: 'pull_request'
wordpress: 'previous major version'

@tellthemachines

Copy link
Copy Markdown
Contributor Author

Ok I've put up an alternative as per Ramon's suggestion above: #81889

@tellthemachines

Copy link
Copy Markdown
Contributor Author

Closing this in favour of #81889, thanks folks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[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.

3 participants