Output theme palette presets when appearance tools are enabled

[tellthemachines]

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

Syncs the changes from WordPress/gutenberg#57190.

Added a unit test and had to tweak the WP_Test_REST_Themes_Controller teardown function to remove an unexpected side-effect.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[tellthemachines]

I was unaware of this until now, but added theme support for a feature in a test will persist in other tests even across theme switches, unless explicitly removed.

The remove_theme_support( 'editor-color-palette' ); in the teardown function of WpGetGlobalStylesheet below was causing a failure in one of the wpThemeJsonResolver tests, because the theme support for 'editor-gradient-presets' added in this test was changing the order of the color settings output from theme.json, which gets merged with theme supports here. I decided to remove them both explicitly in this test to avoid future issues.

[noisysocks]

Does it work if we call remove_theme_support at the end of the test?

So like this:

	public function test_theme_supports_editor_gradient_presets_array() {
		$gradient = array(
			'name'     => __( 'Vivid cyan blue to vivid purple', 'themeLangDomain' ),
			'gradient' => 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)',
			'slug'     => 'vivid-cyan-blue-to-vivid-purple',
		);
		add_theme_support( 'editor-gradient-presets', array( $gradient ) );
		$response = self::perform_active_theme_request();
		$result   = $response->get_data();
		$this->assertArrayHasKey( 'theme_supports', $result[0] );
		$this->assertSame( array( $gradient ), $result[0]['theme_supports']['editor-gradient-presets'] );
		remove_theme_support( 'editor-gradient-presets' );
	}

No big deal, happy with whatever works, it's just that usually wpTearDownAfterClass only contains the "mirror image" of what's in set_up or wpSetUpBeforeClass.

[tellthemachines]

It should work! Lemme try that

[andrewserong]

Just a question, what happens in PHPUnit if you call a function after a failing assertion? Would remove_theme_support still get called? If not, then we might want to have remove_theme_support be called just before the first assertion so that the absence of calling remove_theme_support doesn't affect other tests 🤔

My main thinking there is so that if this test "really" fails, and someone's trying to investigate, they don't get distracted by another failing test that's only failing if this test didn't manage to do all the cleanup needed.

[tellthemachines]

Great question! I had to go and try it out 😄 and can confirm a failing assertion means the rest of the block won't get executed.

I guess that's a good reason to not remove_theme_support inline then. I'd better revert the latest commit.

[noisysocks]

Similar to my other comment, it would be a lot simpler (no need for the member variable) if we can call remove_theme_support() at the end of the test.

[tellthemachines]

I was following the pattern of $remove_theme_support_at_teardown in this file. I agree simpler is better if there are no negative consequences to removing supports added per test.

[noisysocks]

Would prefer to remove that as well if it's not necessary. Try it and see if tests pass ✅

[noisysocks]

Thanks 👍

[tellthemachines]

Thanks for the reviews folks! Committed in r57259.

[mukeshpanchal27]

@tellthemachines This needs to fixed.

/*
 * text
 */