Skip to content

Remove test_global_styles_user_cpt_change_invalidates_cached_stylesheet#45993

Merged
oandregal merged 1 commit intotrunkfrom
remove/global-styles-cache-test-filter
Nov 24, 2022
Merged

Remove test_global_styles_user_cpt_change_invalidates_cached_stylesheet#45993
oandregal merged 1 commit intotrunkfrom
remove/global-styles-cache-test-filter

Conversation

@mmtr
Copy link
Copy Markdown
Contributor

@mmtr mmtr commented Nov 23, 2022 •
edited
Loading

Follows up #45679

What?

Removes the test_global_styles_user_cpt_change_invalidates_cached_stylesheet test that was added in #45679.

Why?

Because the test was added to check that the cached global stylesheet is cleared up after changing the Global Styles CPT. But after removing the filter that short-circuits the cache (see #45679 (comment)), the test is unable to check such behavior anymore, given that the cache is never used when test runs because WP_DEBUG is enabled.

@mmtr mmtr added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Nov 23, 2022
@mmtr mmtr self-assigned this Nov 23, 2022
@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Nov 23, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@felixarntz
Copy link
Copy Markdown
Member

felixarntz commented Nov 23, 2022

@mmtr Is this test causing failures in trunk?

It's of course not great to remove tests, so I want to make sure we bring this test back once we can (for example as part of #45912, since then there likely will be some filter again).

@mmtr
Copy link
Copy Markdown
Contributor Author

mmtr commented Nov 24, 2022

@felixarntz It is not! But the test is not checking anything since it was intended for verifying that gutenberg_get_global_stylesheet wasn't returning a cached value after updating the wp_global_styles CPT. Since the cache is not used when WP_DEBUG is enabled, and given that the test environment has WP_DEBUG enabled, the test is pointless (gutenberg_get_global_stylesheet won't return a cached value ever) – hence why I opted for removing it.

I agree we should add it back as part of #45912 once we have a way for the tests to use the cache.

oandregal
oandregal approved these changes Nov 24, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@oandregal oandregal left a comment

Choose a reason for hiding this comment

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

This is unfortunate, but I don't see another way forward at the moment.

@oandregal oandregal merged commit 5cf5068 into trunk Nov 24, 2022
@oandregal oandregal deleted the remove/global-styles-cache-test-filter branch November 24, 2022 11:35
@github-actions github-actions bot added this to the Gutenberg 14.7 milestone Nov 24, 2022
@felixarntz felixarntz mentioned this pull request Nov 28, 2022
Closed
@DaisyOlsen DaisyOlsen added the [Type] Performance Related to performance efforts label Nov 30, 2022
@oandregal oandregal mentioned this pull request Dec 2, 2022
Closed
felixarntz pushed a commit that referenced this pull request Dec 26, 2022
Reintroduce test removed in #45993.
a45fc65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oandregal oandregal oandregal approved these changes

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

@felixarntz felixarntz Awaiting requested review from felixarntz

Assignees

@mmtr mmtr

Labels

Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Performance Related to performance efforts

Projects

None yet

Milestone

Gutenberg 14.7

Development

Successfully merging this pull request may close these issues.

4 participants

@mmtr @felixarntz @oandregal @DaisyOlsen