Skip to content

Commit

Permalink
General: Introduce WP_DEVELOPMENT_MODE constant to signify context-…
Browse files Browse the repository at this point in the history
…specific development mode.

In recent releases, WordPress core added several instances of cache usage around specific files. While those caches are safe to use in a production context, in development certain nuances apply for whether or not those caches make sense to use. Initially, `WP_DEBUG` was used as a temporary workaround, but it was clear that a more granular method to signify a specific development mode was required: For example, caches around `theme.json` should be disabled when working on a theme as otherwise it would disrupt the theme developer's workflow, but when working on a plugin or WordPress core, this consideration does not apply.

This changeset introduces a `WP_DEVELOPMENT_MODE` constant which, for now, can be set to either "core", "plugin", "theme", or an empty string, the latter of which means no development mode, which is also the default. A new function `wp_get_development_mode()` is the recommended way to retrieve that configuration value.

With the new function available, this changeset replaces all existing instances of the aforementioned `WP_DEBUG` workaround to use `wp_get_development_mode()` with a more specific check.

Props azaozz, sergeybiryukov, peterwilsoncc, spacedmonkey.
Fixes #57487.


git-svn-id: https://develop.svn.wordpress.org/trunk@56042 602fd350-edb4-49c9-b593-d223f7449a82
  • Loading branch information
felixarntz committed Jun 26, 2023
1 parent 88de9a9 commit 4a16702
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 21 deletions.
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ LOCAL_WP_DEBUG_LOG=true
LOCAL_WP_DEBUG_DISPLAY=true
LOCAL_SCRIPT_DEBUG=true
LOCAL_WP_ENVIRONMENT_TYPE=local
LOCAL_WP_DEVELOPMENT_MODE=core

# The URL to use when running e2e tests.
WP_BASE_URL=http://localhost:${LOCAL_PORT}
11 changes: 10 additions & 1 deletion src/wp-includes/default-constants.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,18 @@ function wp_initial_constants() {
define( 'WP_CONTENT_DIR', ABSPATH . 'wp-content' ); // No trailing slash, full paths only - WP_CONTENT_URL is defined further down.
}

/*
* Add define( 'WP_DEVELOPMENT_MODE', 'core' ) or define( 'WP_DEVELOPMENT_MODE', 'plugin' ) or
* define( 'WP_DEVELOPMENT_MODE', 'theme' ) to wp-config.php to signify development mode for WordPress core, a
* plugin, or a theme respectively.
*/
if ( ! defined( 'WP_DEVELOPMENT_MODE' ) ) {
define( 'WP_DEVELOPMENT_MODE', '' );
}

// Add define( 'WP_DEBUG', true ); to wp-config.php to enable display of notices during development.
if ( ! defined( 'WP_DEBUG' ) ) {
if ( 'development' === wp_get_environment_type() ) {
if ( wp_get_development_mode() || 'development' === wp_get_environment_type() ) {
define( 'WP_DEBUG', true );
} else {
define( 'WP_DEBUG', false );
Expand Down
30 changes: 10 additions & 20 deletions src/wp-includes/global-styles-and-settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,10 @@ function wp_get_global_settings( $path = array(), $context = array() ) {
$cache_key = 'wp_get_global_settings_' . $origin;

/*
* Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme
* Ignore cache when the development mode is set to 'theme', so it doesn't interfere with the theme
* developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
$can_use_cached = ! WP_DEBUG;
$can_use_cached = wp_get_development_mode() !== 'theme';

$settings = false;
if ( $can_use_cached ) {
Expand Down Expand Up @@ -151,12 +149,10 @@ function wp_get_global_styles( $path = array(), $context = array() ) {
*/
function wp_get_global_stylesheet( $types = array() ) {
/*
* Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme
* Ignore cache when the development mode is set to 'theme', so it doesn't interfere with the theme
* developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
$can_use_cached = empty( $types ) && ! WP_DEBUG;
$can_use_cached = empty( $types ) && wp_get_development_mode() !== 'theme';

/*
* By using the 'theme_json' group, this data is marked to be non-persistent across requests.
Expand Down Expand Up @@ -252,12 +248,10 @@ function wp_get_global_styles_custom_css() {
return '';
}
/*
* Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme
* Ignore cache when the development mode is set to 'theme', so it doesn't interfere with the theme
* developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
$can_use_cached = ! WP_DEBUG;
$can_use_cached = wp_get_development_mode() !== 'theme';

/*
* By using the 'theme_json' group, this data is marked to be non-persistent across requests.
Expand Down Expand Up @@ -303,12 +297,10 @@ function wp_get_global_styles_custom_css() {
*/
function wp_get_global_styles_svg_filters() {
/*
* Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme
* Ignore cache when the development mode is set to 'theme', so it doesn't interfere with the theme
* developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
$can_use_cached = ! WP_DEBUG;
$can_use_cached = wp_get_development_mode() !== 'theme';
$cache_group = 'theme_json';
$cache_key = 'wp_get_global_styles_svg_filters';
if ( $can_use_cached ) {
Expand Down Expand Up @@ -402,12 +394,10 @@ function wp_theme_has_theme_json() {
if (
null !== $theme_has_support &&
/*
* Ignore static cache when `WP_DEBUG` is enabled. Why? To avoid interfering with
* Ignore static cache when the development mode is set to 'theme', to avoid interfering with
* the theme developer's workflow.
*
* @todo Replace `WP_DEBUG` once an "in development mode" check is available in Core.
*/
! WP_DEBUG &&
wp_get_development_mode() !== 'theme' &&
/*
* Ignore cache when automated test suites are running. Why? To ensure
* the static cache is reset between each test.
Expand Down
44 changes: 44 additions & 0 deletions src/wp-includes/load.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,50 @@ function wp_get_environment_type() {
return $current_env;
}

/**
* Retrieves the current development mode.
*
* The development mode affects how certain parts of the WordPress application behave, which is relevant when
* developing for WordPress.
*
* Valid developer modes are 'core', 'plugin', 'theme', or an empty string to disable developer mode.
*
* Developer mode is considered separately from `WP_DEBUG` and {@see wp_get_environment_type()}. It does not affect
* debugging output, but rather functional nuances in WordPress.
*
* @since 6.3.0
*
* @return string The current development mode.
*/
function wp_get_development_mode() {
static $current_mode = null;

if ( ! defined( 'WP_RUN_CORE_TESTS' ) && null !== $current_mode ) {

This comment has been minimized.

Copy link
@ramonjd

ramonjd Jun 27, 2023

Member

@felixarntz This change has broken many Gutenberg PHP unit tests, e.g., those that rely on wp_theme_has_theme_json()

What's your suggestion to getting them running again?

Adding this to PHP unit test bootstrap.php seems to work

if ( ! defined( 'WP_DEVELOPMENT_MODE' ) ) {
	define( 'WP_DEVELOPMENT_MODE', 'theme' );
}

This comment has been minimized.

Copy link
@ramonjd

ramonjd Jun 27, 2023

Member

I think we're going to run with this for now:

if ( ! defined( 'WP_RUN_CORE_TESTS' ) ) {
	define( 'WP_RUN_CORE_TESTS', true );
}

To get the pipeline working. I suspect there's a lot of cleanup required on the Gutenberg side to remove duplicated tests as well 😅

This comment has been minimized.

Copy link
@felixarntz

felixarntz Jun 27, 2023

Author Member

@ramonjd Hmm, neither of these look like a proper fix, but works for the time being. Can you point me to one of the failed runs to see which tests specifically fail? Happy to take a closer look tomorrow and try to come up with a fix.

This comment has been minimized.

Copy link
@noisysocks

noisysocks Jun 27, 2023

Member

Here's a run that shows the failures: https://github.com/WordPress/gutenberg/actions/runs/5384051109/jobs/9772421423?pr=51894

I wrote some more info here: WordPress/gutenberg#51950

The problem is that Gutenberg contains tests that really ought to only be in Core. While it does I think we need to set WP_RUN_CORE_TESTS.

return $current_mode;
}

$development_mode = WP_DEVELOPMENT_MODE;

// Exclusively for core tests, rely on a global `$_wp_tests_development_mode`.
if ( defined( 'WP_RUN_CORE_TESTS' ) && isset( $GLOBALS['_wp_tests_development_mode'] ) ) {
$development_mode = $GLOBALS['_wp_tests_development_mode'];
}

$valid_modes = array(
'core',
'plugin',
'theme',
'',
);
if ( ! in_array( $development_mode, $valid_modes, true ) ) {
$development_mode = '';
}

$current_mode = $development_mode;

return $current_mode;
}

/**
* Don't load all of WordPress when handling a favicon.ico request.
*
Expand Down
46 changes: 46 additions & 0 deletions tests/phpunit/tests/load/wpGetDevelopmentMode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php
/**
* Unit tests for `wp_get_development_mode()`.
*
* @package WordPress
* @subpackage UnitTests
* @since 6.3.0
*
* @group load.php
* @covers ::wp_get_development_mode
*/
class Test_WP_Get_Development_Mode extends WP_UnitTestCase {

/**
* Tests that `wp_get_development_mode()` returns the value of the `WP_DEVELOPMENT_MODE` constant.
*
* @ticket 57487
*/
public function test_wp_get_development_mode_constant() {
$this->assertSame( WP_DEVELOPMENT_MODE, wp_get_development_mode() );
}

/**
* Tests that `wp_get_development_mode()` allows test overrides.
*
* @ticket 57487
*/
public function test_wp_get_development_mode_test_overrides() {
global $_wp_tests_development_mode;

$_wp_tests_development_mode = 'plugin';
$this->assertSame( 'plugin', wp_get_development_mode() );
}

/**
* Tests that `wp_get_development_mode()` ignores invalid filter values.
*
* @ticket 57487
*/
public function test_wp_get_development_mode_filter_invalid_value() {
global $_wp_tests_development_mode;

$_wp_tests_development_mode = 'invalid';
$this->assertSame( '', wp_get_development_mode() );
}
}
25 changes: 25 additions & 0 deletions tests/phpunit/tests/theme/wpGetGlobalStylesSvgFilters.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,29 @@ public function test_switching_themes_should_recalculate_svg() {
$this->assertStringContainsString( '<svg', $svg_for_default_theme, 'Block theme should contain SVG' );
$this->assertNotSame( $svg_for_default_theme, $svg_for_block_theme, 'Cache value should have changed' );
}

/**
* Tests that the function relies on the development mode for whether to use caching.
*
* @ticket 57487
*
* @covers ::wp_get_global_styles_svg_filters
*/
public function test_caching_is_used_when_developing_theme() {
global $_wp_tests_development_mode;

switch_theme( 'block-theme' );

// Store SVG in cache.
$svg = '<svg></svg>';
wp_cache_set( 'wp_get_global_styles_svg_filters', $svg, 'theme_json' );

// By default, caching should be used, so the above value will be returned.
$_wp_tests_development_mode = '';
$this->assertSame( $svg, wp_get_global_styles_svg_filters(), 'Caching was not used despite development mode disabled' );

// When the development mode is set to 'theme', caching should not be used.
$_wp_tests_development_mode = 'theme';
$this->assertNotSame( $svg, wp_get_global_styles_svg_filters(), 'Caching was used despite theme development mode' );
}
}
23 changes: 23 additions & 0 deletions tests/phpunit/tests/theme/wpGetGlobalStylesheet.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,29 @@ public function test_switching_themes_should_recalculate_stylesheet() {
$this->assertStringContainsString( $expected, $stylesheet_for_block_theme, 'Custom font size (100px) should be present for block theme' );
}

/**
* Tests that the function relies on the development mode for whether to use caching.
*
* @ticket 57487
*/
public function test_caching_is_used_when_developing_theme() {
global $_wp_tests_development_mode;

$this->maybe_switch_theme( 'block-theme' );

// Store CSS in cache.
$css = '.my-class { display: block; }';
wp_cache_set( 'wp_get_global_stylesheet', $css, 'theme_json' );

// By default, caching should be used, so the above value will be returned.
$_wp_tests_development_mode = '';
$this->assertSame( $css, wp_get_global_stylesheet(), 'Caching was not used despite development mode disabled' );

// When the development mode is set to 'theme', caching should not be used.
$_wp_tests_development_mode = 'theme';
$this->assertNotSame( $css, wp_get_global_stylesheet(), 'Caching was used despite theme development mode' );
}

/**
* Adds the 'editor-font-sizes' theme support with custom font sizes.
*
Expand Down
1 change: 1 addition & 0 deletions tools/local-env/scripts/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ wp_cli( `config set WP_DEBUG_LOG ${process.env.LOCAL_WP_DEBUG_LOG} --raw --type=
wp_cli( `config set WP_DEBUG_DISPLAY ${process.env.LOCAL_WP_DEBUG_DISPLAY} --raw --type=constant` );
wp_cli( `config set SCRIPT_DEBUG ${process.env.LOCAL_SCRIPT_DEBUG} --raw --type=constant` );
wp_cli( `config set WP_ENVIRONMENT_TYPE ${process.env.LOCAL_WP_ENVIRONMENT_TYPE} --type=constant` );
wp_cli( `config set WP_DEVELOPMENT_MODE ${process.env.LOCAL_WP_DEVELOPMENT_MODE} --type=constant` );

// Move wp-config.php to the base directory, so it doesn't get mixed up in the src or build directories.
renameSync( 'src/wp-config.php', 'wp-config.php' );
Expand Down

0 comments on commit 4a16702

Please sign in to comment.