Commit 07139aa7e08 for woocommerce
commit 07139aa7e0882b1dda88d7c2fefd0c39655ac296
Author: Vlad Olaru <vlad.olaru@automattic.com>
Date: Tue Sep 8 16:02:09 2026 +0300
[tests] Move classic theme block style loading coverage from E2E to PHP (#68249)
* test(blocks): Move classic style loading coverage to PHP
Classic-theme style loading was covered by two Playwright tests that only inspected server-emitted stylesheet identifiers. On current WordPress versions, Core's on-demand asset path can satisfy those assertions without exercising WooCommerce's compatibility fallback.
Move the request decision cache onto the container-owned controller instance so the fallback can be isolated without changing its request lifecycle. Add integration coverage that rebuilds the real Woo block registry under WordPress's supported classic-asset settings, proves styles stay absent before render, and verifies Product Filters queues its metadata style when rendered.
Remove the redundant browser spec now that PHP owns both the generic absence and render-time enqueue contracts.
Refs TESTOPS-234
* test(blocks): run the classic style loading test in-process
@runInSeparateProcess with preserveGlobalState disabled re-runs the
WordPress test bootstrap in the child, which drops and recreates every
table in the phpunit database mid-suite and costs 3.7s per run. The
test only needed it as a substitute for cleanup.
Snapshot the WooCommerce block registry and the style queue in setUp
and restore them in tearDown, reset the queue at the start of the test
so earlier renders cannot leak into its assertions, and drop the
assertions that could not fail: the instanceof guards after
assertInstanceOf, the render_callback check that is always true for a
closure, and the post-render print capture that the recorded mutation
never reached. Replace the two markup regexes with one assertion on the
style queue, which covers every block style rather than one file
naming convention. Rename the method to match its testdox claim.
Refs TESTOPS-234
* test(blocks): Pin a classic theme for the style loading test
The test guarded on the ambient theme already being classic rather
than selecting one itself. Which theme the PHPUnit install activates
depends on the WordPress build under test: locally it resolves to the
suite's classic `default` theme, while every CI job resolves a block
theme, so the guard failed on all three PHP/WP matrix jobs.
Switch to Storefront before the guard, as class-wc-post-types-test.php
already does. The compatibility fallback under test only runs for
classic themes, so the test has to choose the theme instead of
inheriting it. WP_UnitTestCase's transaction rolls the switch back, so
no teardown is needed.
Refs TESTOPS-234
* test(blocks): Assert the exact handles that must not be queued
The pre-render assertion matched the style queue against a handle
naming pattern. A Product Filters style queued under a name that
pattern does not describe would pass it, and the post-render assertion
that follows would pass in that case too, so the pair could both hold
while the behaviour under test was broken.
Assert that none of the captured metadata handles are queued, the
precise complement of the post-render check, and keep the pattern
assertion as the broader guard that a classic page queues no block
style at all.
Refs TESTOPS-234
* test(blocks): Drop redundant filter cleanup from tearDown
tearDown removed the two asset filters the test adds. WP_UnitTestCase
snapshots $wp_filter in set_up and restores it wholesale in tear_down,
so both were already removed by the time any later test ran.
They also had no bearing on the registry restore that follows them:
passing a WP_Block_Type instance to WP_Block_Type_Registry::register()
stores that instance as-is without re-running set_props(), so
register_block_type_args never fires during the restore.
Refs TESTOPS-234
* test(blocks): Restore the script queue after rendering Product Filters
The test restored only wp_styles()->queue in tearDown. Rendering
Product Filters through do_blocks() also enqueues a script, because
ProductFilters::render() calls wp_enqueue_script( 'wc-settings' ).
phpunit.xml sets backupGlobals="false", and neither WP_UnitTestCase nor
WC_Unit_Test_Case resets $wp_scripts, so that handle stayed queued for
every test that ran afterwards in the same process.
Capture wp_scripts()->queue alongside the style queue and restore it in
tearDown, so the class leaves the asset state it found.
Refs TESTOPS-234
* Simplify the classic theme block style test around one probe block
The classic-theme test reached enqueue_block_style_for_classic_themes()
by unregistering every WooCommerce block and calling register_blocks()
to rebuild the catalogue. Everything awkward about the test followed
from that: the registry snapshot and restore, the scripts-queue restore
that ProductFilters::render() made necessary, and a dependency on the
suite's bootstrap theme that the comment above switch_theme() did not
mention.
Register one probe block through register_block_type() instead. The
register_block_type_args filter still fires on it from
WP_Block_Type::set_props(), and do_blocks() still renders it through
WP_Block::render(), so the two seams the removed browser spec covered
stay covered. The probe declares a source-less style handle, so no real
block's assets are involved and the bootstrap theme no longer matters.
A second test covers the block-theme branch, which unhooks the filter,
directly on the method.
What this gives up is the assertion that Core skips blocks with empty
style_handles across the whole catalogue; that is Core's contract.
Mutation-checked: no-op'ing the render enqueue, removing both
style-stripping lines, and removing the remove_filter() call each turn
exactly one assertion red.
* Make the on-demand premise explicit in the classic theme style tests
The classic-theme test forced should_load_separate_core_block_assets
and should_load_block_assets_on_demand to false at PHP_INT_MAX without
saying what that overrides. Since WordPress 6.9,
wp_load_classic_theme_block_styles_on_demand() opts classic themes into
on-demand block assets at priority 0, so on every WordPress version
WooCommerce supports the fallback is only live on sites that opt out.
A reader could not tell from the test whether the forced configuration
was realistic or invented.
Name the Core function in a comment and assert the opt-out before
registering the probe, so the test states which sites it simulates.
Turn the block-theme test into a @testWith over Storefront and Twenty
Twenty-Two that asserts Core is already loading on demand, then checks
the fallback passes args through and unhooks. The Storefront row is
the premise the PR rests on, made executable: on a classic theme with
Core's defaults the fallback stands down, which is why the removed
browser spec never exercised it. If a future WordPress flips that
default back, the row fails and says the fallback is live again.
* test: Isolate each clause of the classic theme style fallback guard
The stand-down test activated Storefront and Twenty Twenty-Two under
Core's defaults and asserted that the fallback passes args through and
unhooks itself. Both rows pass for reasons the test name did not claim:
Storefront reports on_demand=1 and separate=1, so the separate-assets
clause masks the on-demand one, and Twenty Twenty-Two short-circuits at
wp_is_block_theme(). Removing the on-demand clause from the guard left
both rows green, so the test named for that condition never covered it.
Keep the defaults rows, which assert the real-world outcome the E2E
migration rests on, and rename them to match. Add one test per clause
that forces the other asset filters to the values which make that clause
the only one able to stand the fallback down. Each new test now fails
when its clause is removed from the guard.
Refs TESTOPS-234
diff --git a/plugins/woocommerce/changelog/testops-234-classic-style-loading-cache b/plugins/woocommerce/changelog/testops-234-classic-style-loading-cache
new file mode 100644
index 00000000000..81306a99d01
--- /dev/null
+++ b/plugins/woocommerce/changelog/testops-234-classic-style-loading-cache
@@ -0,0 +1,4 @@
+Significance: patch
+Type: dev
+
+Move the classic-theme block style decision cache from a function-local static to a BlockTypesController instance property so it can be isolated in PHP tests.
diff --git a/plugins/woocommerce/src/Blocks/BlockTypesController.php b/plugins/woocommerce/src/Blocks/BlockTypesController.php
index 13646e13738..5dfe9fedeab 100644
--- a/plugins/woocommerce/src/Blocks/BlockTypesController.php
+++ b/plugins/woocommerce/src/Blocks/BlockTypesController.php
@@ -53,6 +53,13 @@ final class BlockTypesController {
*/
private static $register_blocks_has_run = false;
+ /**
+ * Whether WooCommerce block styles should be enqueued on demand for classic themes.
+ *
+ * @var bool|null
+ */
+ private $should_enqueue_block_style_for_classic_themes = null;
+
/**
* Constructor.
*
@@ -595,17 +602,16 @@ final class BlockTypesController {
*/
public function enqueue_block_style_for_classic_themes( $args, $block_name ) {
- // Repeatedly checking the theme is expensive. So statically cache this logic result and remove the filter if not needed.
- static $should_enqueue_block_style_for_classic_themes = null;
- if ( null === $should_enqueue_block_style_for_classic_themes ) {
- $should_enqueue_block_style_for_classic_themes = ! (
+ // Repeatedly checking the theme is expensive. Cache this logic result and remove the filter if not needed.
+ if ( null === $this->should_enqueue_block_style_for_classic_themes ) {
+ $this->should_enqueue_block_style_for_classic_themes = ! (
is_admin() ||
wp_is_block_theme() ||
( function_exists( 'wp_should_load_block_assets_on_demand' ) && wp_should_load_block_assets_on_demand() ) ||
wp_should_load_separate_core_block_assets()
);
}
- if ( ! $should_enqueue_block_style_for_classic_themes ) {
+ if ( ! $this->should_enqueue_block_style_for_classic_themes ) {
remove_filter( 'register_block_type_args', array( $this, 'enqueue_block_style_for_classic_themes' ), 10 );
return $args;
diff --git a/plugins/woocommerce/tests/e2e/tests/blocks/style.classic_theme.spec.ts b/plugins/woocommerce/tests/e2e/tests/blocks/style.classic_theme.spec.ts
deleted file mode 100644
index dc6e8f8c453..00000000000
--- a/plugins/woocommerce/tests/e2e/tests/blocks/style.classic_theme.spec.ts
+++ /dev/null
@@ -1,98 +0,0 @@
-/**
- * External dependencies
- */
-import { Page } from '@playwright/test';
-import {
- expect,
- test as base,
- CLASSIC_THEME_SLUG,
-} from '@woocommerce/e2e-utils';
-
-const test = base.extend( {} );
-
-async function getStylesheets( page: Page ) {
- const styleLocators = page.locator(
- 'link[rel="stylesheet"][href*="assets/client/blocks"]:not([href*="wc-blocks.css"])'
- );
- return await styleLocators.evaluateAll( ( links ) =>
- links.map( ( link ) => ( link as HTMLLinkElement ).href )
- );
-}
-
-async function getInlineStyles( page: Page ) {
- const styleLocators = page.locator(
- 'style[id^="woocommerce-"][id$="-style-inline-css"]'
- );
- return await styleLocators.evaluateAll( ( styles ) =>
- styles.map( ( style ) => style.id )
- );
-}
-
-test.describe( 'Block Style Loading in Classic Themes', () => {
- test.beforeEach( async ( { requestUtils } ) => {
- // Activate classic theme for all tests
- await requestUtils.activateTheme( CLASSIC_THEME_SLUG );
- } );
-
- test( 'should not load unnecessary block styles on pages without WooCommerce blocks', async ( {
- page,
- admin,
- editor,
- } ) => {
- await admin.createNewPost( { postType: 'page' } );
- await editor.canvas
- .getByRole( 'textbox', { name: 'Add title' } )
- .fill( 'Test Page Without Blocks' );
-
- await editor.insertBlock( {
- name: 'core/paragraph',
- attributes: {
- content: 'This is a regular page with no WooCommerce blocks.',
- },
- } );
-
- await editor.publishAndVisitPost();
-
- const blockStylesheets = await getStylesheets( page );
- const inlineBlockStyles = await getInlineStyles( page );
-
- expect( blockStylesheets ).toHaveLength( 0 );
- expect( inlineBlockStyles ).toHaveLength( 0 );
- } );
-
- test( 'should load base WooCommerce styles when blocks are present', async ( {
- page,
- admin,
- editor,
- } ) => {
- await admin.createNewPost( { postType: 'page' } );
- await editor.canvas
- .getByRole( 'textbox', { name: 'Add title' } )
- .fill( 'Test Page With WooCommerce Block' );
-
- await editor.insertBlock( {
- name: 'woocommerce/product-filters',
- } );
-
- await editor.publishAndVisitPost();
-
- const blockStylesheets = await getStylesheets( page );
- const inlineBlockStyles = await getInlineStyles( page );
-
- // Ensure styles are loaded (either as files or inline)
- const hasFileStyles = blockStylesheets.length > 0;
- const hasInlineStyles = inlineBlockStyles.length > 0;
-
- expect( hasFileStyles || hasInlineStyles ).toBeTruthy();
-
- const hasProductFilterStyle = blockStylesheets.some( ( href ) =>
- href.includes( 'product-filters' )
- );
- const hasProductFilterInlineStyle = inlineBlockStyles.some( ( id ) =>
- id.includes( 'product-filters' )
- );
- expect(
- hasProductFilterStyle || hasProductFilterInlineStyle
- ).toBeTruthy();
- } );
-} );
diff --git a/plugins/woocommerce/tests/php/src/Blocks/BlockTypesController.php b/plugins/woocommerce/tests/php/src/Blocks/BlockTypesController.php
index 06cd5041e96..72d69d90337 100644
--- a/plugins/woocommerce/tests/php/src/Blocks/BlockTypesController.php
+++ b/plugins/woocommerce/tests/php/src/Blocks/BlockTypesController.php
@@ -21,6 +21,16 @@ class BlockTypesController extends WC_Unit_Test_Case {
*/
private $block_types_controller;
+ /**
+ * Block registered through the real registration path, so register_block_type_args fires on it.
+ */
+ private const PROBE_BLOCK = 'woocommerce/classic-theme-fallback-probe';
+
+ /**
+ * Style handle the probe block declares, registered without a source.
+ */
+ private const PROBE_STYLE = 'wc-classic-theme-fallback-probe';
+
/**
* Sets up a new TestedBlockTypesController so it can be tested.
*
@@ -36,6 +46,142 @@ class BlockTypesController extends WC_Unit_Test_Case {
);
}
+ /**
+ * Removes the probe block and style; the base class does not reset the block registry or the style queue.
+ */
+ public function tearDown(): void {
+ try {
+ if ( \WP_Block_Type_Registry::get_instance()->is_registered( self::PROBE_BLOCK ) ) {
+ unregister_block_type( self::PROBE_BLOCK );
+ }
+ wp_dequeue_style( self::PROBE_STYLE );
+ wp_deregister_style( self::PROBE_STYLE );
+ } finally {
+ parent::tearDown();
+ }
+ }
+
+ /**
+ * @testdox Should defer a block's style until the block renders on a classic theme.
+ */
+ public function test_classic_theme_defers_block_style_until_render(): void {
+ switch_theme( 'storefront' );
+ $this->assertFalse( wp_is_block_theme(), 'The test must run with a classic theme.' );
+
+ // Since WordPress 7.0, wp_load_classic_theme_block_styles_on_demand() opts classic themes into
+ // on-demand block assets at wp_default_styles priority 0, so the fallback is only live on sites that opt out.
+ add_filter( 'should_load_separate_core_block_assets', '__return_false', PHP_INT_MAX );
+ add_filter( 'should_load_block_assets_on_demand', '__return_false', PHP_INT_MAX );
+ $this->assertFalse( wp_should_load_block_assets_on_demand(), 'The test must simulate a site that opted out of on-demand block assets.' );
+
+ $block_type = $this->register_probe_block();
+
+ $this->assertSame( array(), $block_type->style_handles, 'Registration must strip the style so Core does not queue it on every page.' );
+ $this->assertFalse( wp_style_is( self::PROBE_STYLE, 'enqueued' ), 'The style must not be queued before the block renders.' );
+
+ $this->assertStringContainsString( 'class="probe"', do_blocks( '<!-- wp:' . self::PROBE_BLOCK . ' /-->' ) );
+ $this->assertTrue( wp_style_is( self::PROBE_STYLE, 'enqueued' ), 'Rendering the block must queue its style.' );
+ }
+
+ /**
+ * Both themes satisfy more than one clause of the fallback's guard under Core's defaults, so this asserts the
+ * real-world outcome that makes the fallback dormant. The three tests below isolate the individual clauses.
+ *
+ * @testdox Should stand down under Core's default block asset settings.
+ * @testWith ["storefront"]
+ * ["twentytwentytwo"]
+ *
+ * @param string $theme Theme to activate before the decision is made.
+ */
+ public function test_stands_down_under_core_default_asset_settings( string $theme ): void {
+ switch_theme( $theme );
+ $this->assertTrue( wp_should_load_block_assets_on_demand(), 'WordPress must already be loading block assets on demand.' );
+
+ $this->assert_fallback_stands_down();
+ }
+
+ /**
+ * @testdox Should stand down when only on-demand block assets are enabled.
+ */
+ public function test_stands_down_when_only_on_demand_assets_are_enabled(): void {
+ switch_theme( 'storefront' );
+ $this->assertFalse( wp_is_block_theme(), 'The test must run with a classic theme.' );
+
+ // Turn the separate-assets clause off so on-demand is the only clause that can stand the fallback down.
+ add_filter( 'should_load_separate_core_block_assets', '__return_false', PHP_INT_MAX );
+ add_filter( 'should_load_block_assets_on_demand', '__return_true', PHP_INT_MAX );
+ $this->assertFalse( wp_should_load_separate_core_block_assets(), 'Separate core block assets must be off.' );
+ $this->assertTrue( wp_should_load_block_assets_on_demand(), 'On-demand block assets must be on.' );
+
+ $this->assert_fallback_stands_down();
+ }
+
+ /**
+ * @testdox Should stand down when only separate core block assets are enabled.
+ */
+ public function test_stands_down_when_only_separate_assets_are_enabled(): void {
+ switch_theme( 'storefront' );
+ $this->assertFalse( wp_is_block_theme(), 'The test must run with a classic theme.' );
+
+ // Turn the on-demand clause off so separate assets is the only clause that can stand the fallback down.
+ add_filter( 'should_load_separate_core_block_assets', '__return_true', PHP_INT_MAX );
+ add_filter( 'should_load_block_assets_on_demand', '__return_false', PHP_INT_MAX );
+ $this->assertTrue( wp_should_load_separate_core_block_assets(), 'Separate core block assets must be on.' );
+ $this->assertFalse( wp_should_load_block_assets_on_demand(), 'On-demand block assets must be off.' );
+
+ $this->assert_fallback_stands_down();
+ }
+
+ /**
+ * @testdox Should stand down under a block theme even when block assets are not loaded on demand.
+ */
+ public function test_stands_down_under_a_block_theme_without_on_demand_assets(): void {
+ switch_theme( 'twentytwentytwo' );
+ $this->assertTrue( wp_is_block_theme(), 'The test must run with a block theme.' );
+
+ // Turn both asset clauses off so the block theme check is the only clause that can stand the fallback down.
+ add_filter( 'should_load_separate_core_block_assets', '__return_false', PHP_INT_MAX );
+ add_filter( 'should_load_block_assets_on_demand', '__return_false', PHP_INT_MAX );
+ $this->assertFalse( wp_should_load_separate_core_block_assets(), 'Separate core block assets must be off.' );
+ $this->assertFalse( wp_should_load_block_assets_on_demand(), 'On-demand block assets must be off.' );
+
+ $this->assert_fallback_stands_down();
+ }
+
+ /**
+ * Asserts that the fallback passed the block args through untouched and unhooked itself.
+ */
+ private function assert_fallback_stands_down(): void {
+ $args = array( 'style_handles' => array( self::PROBE_STYLE ) );
+
+ $result = $this->block_types_controller->enqueue_block_style_for_classic_themes( $args, self::PROBE_BLOCK );
+
+ $this->assertSame( $args, $result, 'Core queues the style on render already, so the args must pass through untouched.' );
+ $this->assertFalse(
+ has_filter( 'register_block_type_args', array( $this->block_types_controller, 'enqueue_block_style_for_classic_themes' ) ),
+ 'The fallback must unhook itself once it decides it is not needed.'
+ );
+ }
+
+ /**
+ * Registers the probe block and its style through the real registration path.
+ *
+ * @return \WP_Block_Type The registered block type.
+ */
+ private function register_probe_block(): \WP_Block_Type {
+ wp_register_style( self::PROBE_STYLE, false, array(), '1' );
+ $block_type = register_block_type(
+ self::PROBE_BLOCK,
+ array(
+ 'style_handles' => array( self::PROBE_STYLE ),
+ 'render_callback' => static fn() => '<div class="probe"></div>',
+ )
+ );
+ $this->assertInstanceOf( \WP_Block_Type::class, $block_type );
+
+ return $block_type;
+ }
+
/**
* @testdox Should identify blocks that should have data attributes.
*/