Make WordPress Core

Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#61371 closed enhancement (fixed)

Improve `class WP_Theme_JSON` tests

Reported by: isabel_brison's profile isabel_brison Owned by: isabel_brison's profile isabel_brison
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.6
Component: Editor Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Currently, any change made to styles generated by class WP_Theme_JSON cause multiple tests from its test suite to fail, mostly due to diffs in CSS strings being checked for equality.

These tests haven't proven very useful to detect bugs in the class, but they have been incredibly painful to maintain due to the constant updates of test strings needed every time a change is made to the output styles.

There's more context in https://github.com/WordPress/gutenberg/issues/60842.

It would be good to at the very least test the output of the functions called by get_stylesheet separately. Ideally we'd minimise comparing full CSS strings, because they change quite often.

Further ideas for improvement welcome! I'm thinking to work on this core-first in hopes the changes can still make it into 6.6 and be of benefit during this cycle.

Change History (5)

This ticket was mentioned in PR #6734 on WordPress/wordpress-develop by @isabel_brison.


8 months ago
#1

  • Keywords has-patch has-unit-tests added

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

WIP: still going through the tests and working out how to make them more efficient and maintainable.

I'm using 3 types of approach so far:

  • refactoring tests that call use_stylesheet when they really want to test the behaviour of one of its underlying functions to instead call that underlying function, using a reflection class where needed;
  • skipping output of base layout styles with 'skip_root_layout_styles' => true where the test is checking multiple parts of use_stylesheet output but base layout styles aren't relevant;
  • deleting a test that is already covered by other tests

@ramonopoly commented on PR #6734:


8 months ago
#2

LGTM

I like how this change brings more specificity to coverage, e.g., testing the method that outputs block CSS. I think it'll make life easier when debugging.

And also thanks for deleting a bunch of those CSS expectations 🙃

#3 @isabel_brison
8 months ago

  • Milestone changed from Awaiting Review to 6.6

Adding this to the 6.6 milestone because it only touches the unit tests.

#4 @isabel_brison
8 months ago

  • Owner set to isabel_brison
  • Resolution set to fixed
  • Status changed from new to closed

In 58378:

Editor: Improve maintainability of class WP_Theme_JSON tests.

Updates tests calling get_stylesheet to not output layout styles if they’re not relevant to the test or to call get_styles_for_block instead where more appropriate.

Props isabel_brison, andrewserong, ramonopoly.
Fixes #61371.

@isabel_brison commented on PR #6734:


8 months ago
#5

Committed in r58378.

Note: See TracTickets for help on using tickets.