Make WordPress Core

Opened 6 months ago

Last modified 6 months ago

#58650 new enhancement

Reset cache in development mode

Reported by: ramonopoly's profile ramonopoly Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Cache API Keywords:
Focuses: Cc:

Description

https://core.trac.wordpress.org/ticket/57487 added support for development mode in order that the WordPress code is aware of different methods and levels of caching.

Tests in the Gutenberg plugin utilize methods such as wp_theme_has_theme_json.

In Core, this method checks the current development mode and deal with the cache accordingly.

Because Gutenberg had not set the required constants, PHP unit tests were failing, probably because the cache was not being cleared between tests ($can_use_cached was always true).

In order to get the tests running on the Gutenberg CI, the constant WP_RUN_CORE_TESTS needed to be declared in [Gutenberg's PHPUnit bootstrap file]https://github.com/WordPress/gutenberg/pull/51950/files#diff-a51f75ea7a86ddd18edea3fe94a0482427795fcd7b247e6959149cbac60964dfR27.

PR: https://github.com/WordPress/gutenberg/pull/51950

An alternative way suggested by @noisysocks was for Core to update Core's implementation of wp_theme_has_theme_json to cache the return value in a way that is reset by wp_clean_theme_json_cache.

cc @flixos90

Related discussion:

https://github.com/WordPress/wordpress-develop/commit/4a16702090984caef24af5a07d598cc5afff2fdc#r119747488

Change History (1)

#1 @flixos90
6 months ago

  • Reporter changed from ramonopoly, noisysocks to ramonopoly
  • Type changed from feature request to enhancement

(fixing the double issue reporter here, cc'ing @noisysocks instead 🙂)

Reviewing this more closely, it seems at this point Gutenberg setting the WP_RUN_CORE_TESTS constant is the most appropriate solution indeed - and since arguably Gutenberg can be considered part of core in a certain way, it may even make sense to handle its own unit tests the same way as core tests. So potentially this isn't even just a workaround, but a long-term acceptable solution.

That said, I acknowledge that using a static variable here makes cache clearing impossible. If I remember correctly, it was decided to go with that approach since the function is called so many times, the value should never change within a single page load, and even using an object cache would have a higher overhead so that was avoided here.

Changing this production code just because a test needs it, in a way that is less performant is questionable. On the other hand, we may only be nit-picking about an irrelevant difference. Maybe we could use a non-persistent cache group? Aren't those typically just using a variable or class property in PHP anyway, even on sites that use a persistent object cache layer? If so, I think that would be a good solution.

cc @spacedmonkey @oandregal since I believe you were involved in the original discussions

Note: See TracTickets for help on using tickets.