Backport block template resolution algorithm unit tests

[Bernhard Reiter]

It seems like we missed backporting these as part of ​https://github.com/WordPress/wordpress-develop/pull/1796.

It seems like we missed backporting these as part of ​https://github.com/WordPress/wordpress-develop/pull/1796. Doing it now since they will give us better confidence when we need to modify the algorithm.

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

Can't remove ​`index.php` from block-theme yet because of this check: ​https://github.com/WordPress/wordpress-develop/blob/3442f776f3f56eb76eec2c85f2d42941d9db9a93/src/wp-includes/class-wp-theme.php#L341-L348

Are we going to relax this to accept block-templates/index.html instead (and soon, templates/index.html)?

3 unit test failures to go -- 1 from the newly backported tests, 2 from what seems to be a side effect of the theme switching (?):

There were 3 failures:

1) Block_Template_Test::test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/var/www/tests/phpunit/includes/../data/themedir1/block-theme-child/page-home.php'
+'/var/www/src/wp-includes/template-canvas.php'

/var/www/tests/phpunit/tests/block-template.php:120

2) Tests_General_Template::test_has_custom_logo
Failed asserting that true is false.

/var/www/tests/phpunit/tests/general/template.php:270

3) Tests_General_Template::test_get_custom_logo
Failed asserting that a string is empty.

/var/www/tests/phpunit/tests/general/template.php:315

For the remaining failure test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template, I followed the logic of the algorithm and it's behaving as expected, basically I don't see any logic that compares the specificity of the php template with the block template that comes from the parent theme so I don't really know why this test passes in Gutenberg.

For the remaining failure test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template, I followed the logic of the algorithm and it's behaving as expected, basically I don't see any logic that compares the specificity of the php template with the block template that comes from the parent theme so I don't really know why this test passes in Gutenberg.

That's a bit concerning though. The test has a note that specifically points at a regression fix: ​https://github.com/WordPress/wordpress-develop/pull/1920/files#diff-12e215aaa7e67637d3de9eae20536f56ac9248727dcdca0b6f87c09e3f256166R101-R108

The reasoning of ​that PR makes sense to me; and I think I thought to preserve that functionality when refactoring the template resolution algorithm, and ​added the test to cover against further regressions.

I'll look a bit more into this.

I think I've tracked it down: You're right that the algorithm is working as expected _here_.

In Gutenberg OTOH, something isn't quite working with test theme directory registration, and thus, we're not detecting that test-theme-child is a child theme, and test-theme is its parent. (This can be validated by echoing get_template() somewhere in the unit test file: It should return the parent theme -- i.e. test-theme -- rather than the child -- but in GB, it returns the child theme instead.)

This will require a number of different fixes:

• In the unit test in GB, we need to make sure the child theme is detected as such, and its parent is found. This will cause test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template to fail in GB.
• We need to fix the template resolution algorithm in both GB and WP Core to make test_child_theme_php_template_takes_precedence_over_equally_specific_parent_theme_block_template pass in both.

@ockham I think to avoid delaying the switch to "templates" and "parts", we should comment this test for now while we work on it separately. In parallel, I can start the "folders" patch in Core that way it will unblock the Gutenberg PR.

@ockham I've created the folders PR in ​https://github.com/WordPress/wordpress-develop/pull/1932/

Can't remove ​`index.php` from block-theme yet because of this check:

​https://github.com/WordPress/wordpress-develop/blob/3442f776f3f56eb76eec2c85f2d42941d9db9a93/src/wp-includes/class-wp-theme.php#L341-L348

Are we going to relax this to accept block-templates/index.html instead (and soon, templates/index.html)?

This was punted to 6.0:
https://core.trac.wordpress.org/ticket/54272

Are we going to relax this to accept block-templates/index.html instead (and soon, templates/index.html)?

This was punted to 6.0: https://core.trac.wordpress.org/ticket/54272

Thanks for clarifying @carolinan!

@ockham I think to avoid delaying the switch to "templates" and "parts", we should comment this test for now while we work on it separately. In parallel, I can start the "folders" patch in Core that way it will unblock the Gutenberg PR.

Okay, I'll do that now 👍

@ockham I think to avoid delaying the switch to "templates" and "parts", we should comment this test for now while we work on it separately. In parallel, I can start the "folders" patch in Core that way it will unblock the Gutenberg PR.

Okay, I'll do that now 👍

Done: 6d63d8f. Should pass now & be ready for review.

Multisite tests are failing. I'll try a rebase for starters.

Multisite tests are still failing. Specifically, tests affected are in the sidebars and widgets controller test suites (rest-sidebars-controller.php and rest-widgets-controller.php), and they're mostly getting the wrong REST API responses (e.g. some data where an empty response was expected, or extraneous fields).

I've double-checked that this is not affecting trunk, or other PRs. It's also not affecting any of the single-site tests. The only thing in this PR that I think could've caused this is the theme switching (to our block test theme(s), and back to whatever theme was active before); but apparently there's some cross-site leakage of data. Or so.

Trying to revert the theme switch, and restoring the delete_option( 'site_logo' ).

No, multisite tests are still failing 😕 (It's the same ones as before, plus a bunch more in themeMods.php.

The next thing I'll try will be to copy the theme switching strategy from tests/phpunit/tests/theme/wpThemeJsonResolver.php, since that's also using block-theme and block-theme-child, and multisite tests obviously seem to be passing there:

​https://github.com/WordPress/wordpress-develop/blob/e7c8bf8fa70680a6723cd182bed6595732f3600c/tests/phpunit/tests/theme/wpThemeJsonResolver.php#L15-L37

Having another look at the test matrix, not _all_ multisite tests are failing. The ones that are passing are:

• PHPUnit Tests / 5.6 multisite on ubuntu-latest
• PHPUnit Tests / 8.1 multisite on ubuntu-latest
• PHPUnit Tests / 5.6 multisite slow tests on ubuntu-latest

Uh, but the 8.1 jobs have continue-on-error. (Closer inspections shows that unit tests _are_ failing there.)

The 5.6 ones do seem to pass, tho.

The next thing I'll try will be to copy the theme switching strategy from tests/phpunit/tests/theme/wpThemeJsonResolver.php

This got a bit messy upon first attempt (more failures in other tests).

New/alternative hypothesis: Since we've been doing the theme switching in the _static_ wpSetUpBeforeClass (and wpTearDownAfterClass) -- maybe that's happening too early (or late, respectively) -- i.e. before multisite setup is complete? I'll try moving them into the non-static set_up and tear_down.

Hmm, looking at the test failures, it seems like the data might be coming from tests/phpunit/tests/widgets.php. I'll look into that file...

This could be relevant: #1331

Seems like the sidebar/widgets data we're seeing if written by ​`wp_install_defaults`. In a multi-site context, the latter is called by ​`wp_initialize_site`.

@jorgefilipecosta gave me the decisive tip: 5087aeb 😄

Great, going to commit this.

[youknowriad]

In 52246:

Themes: Add block template resolution algorithm unit tests.

Backports the unit tests for block templates
and hybrid themes template resolution from the Gutenberg plugin.

Props bernhard-reiter, jorgefilipecosta.
Fixes #54478.

committed in https://core.trac.wordpress.org/changeset/52246

I've filed an issue for the failing unit test that we had to disable in Core Trac: https://core.trac.wordpress.org/ticket/54515