Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Ignore] Check if tests detect missing injected theme attribute #5155

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Sep 6, 2023

We have a dedicated test for this:

public function test_inject_theme_attribute_in_block_template_content() {
$theme = get_stylesheet();
$content_without_theme_attribute = '<!-- wp:template-part {"slug":"header","align":"full", "tagName":"header","className":"site-header"} /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_without_theme_attribute,
$theme
);
$expected = sprintf(
'<!-- wp:template-part {"slug":"header","align":"full","tagName":"header","className":"site-header","theme":"%s"} /-->',
get_stylesheet()
);
$this->assertSame( $expected, $template_content );
$content_without_theme_attribute_nested = '<!-- wp:group --><!-- wp:template-part {"slug":"header","align":"full", "tagName":"header","className":"site-header"} /--><!-- /wp:group -->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_without_theme_attribute_nested,
$theme
);
$expected = sprintf(
'<!-- wp:group --><!-- wp:template-part {"slug":"header","align":"full","tagName":"header","className":"site-header","theme":"%s"} /--><!-- /wp:group -->',
get_stylesheet()
);
$this->assertSame( $expected, $template_content );
// Does not inject theme when there is an existing theme attribute.
$content_with_existing_theme_attribute = '<!-- wp:template-part {"slug":"header","theme":"fake-theme","align":"full", "tagName":"header","className":"site-header"} /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_with_existing_theme_attribute,
$theme
);
$this->assertSame( $content_with_existing_theme_attribute, $template_content );
// Does not inject theme when there is no template part.
$content_with_no_template_part = '<!-- wp:post-content /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_with_no_template_part,
$theme
);
$this->assertSame( $content_with_no_template_part, $template_content );
}

If I remove theme attribute injection (as done by this branch), the test still seems to pass locally 🤔

To reproduce:

  • Check out this branch
  • npm run test:php -- --group=block-templates

@ockham ockham self-assigned this Sep 6, 2023
@ockham
Copy link
Contributor Author

ockham commented Sep 6, 2023

Ah nevermind 😅

image

@ockham ockham closed this Sep 6, 2023
@ockham ockham reopened this Sep 6, 2023
@ockham
Copy link
Contributor Author

ockham commented Sep 6, 2023

Ah, I realized the line I changed got rid of the entire template content, not just the theme attribute injection. Let's try once more...

@ockham
Copy link
Contributor Author

ockham commented Sep 6, 2023

Okay, tests are still failing:

There were 2 failures:

1) Tests_Blocks_Editor::test_wp_get_post_content_block_attributes
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 (
-    'layout' => Array &1 (
-        'type' => 'constrained'
-    )
-)
+Array &0 ()

/var/www/tests/phpunit/tests/blocks/editor.php:454
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106
/var/www/vendor/bin/phpunit:118

2) Tests_REST_WpRestTemplatesController::test_get_template_fallback
Should fallback to `page.html`.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'page'
+'index'

/var/www/tests/phpunit/tests/rest-api/wpRestTemplatesController.php:839
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106
/var/www/vendor/bin/phpunit:118

Still a bit surprised it's not the following test that seems to be failing:

public function test_inject_theme_attribute_in_block_template_content() {
$theme = get_stylesheet();
$content_without_theme_attribute = '<!-- wp:template-part {"slug":"header","align":"full", "tagName":"header","className":"site-header"} /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_without_theme_attribute,
$theme
);
$expected = sprintf(
'<!-- wp:template-part {"slug":"header","align":"full","tagName":"header","className":"site-header","theme":"%s"} /-->',
get_stylesheet()
);
$this->assertSame( $expected, $template_content );
$content_without_theme_attribute_nested = '<!-- wp:group --><!-- wp:template-part {"slug":"header","align":"full", "tagName":"header","className":"site-header"} /--><!-- /wp:group -->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_without_theme_attribute_nested,
$theme
);
$expected = sprintf(
'<!-- wp:group --><!-- wp:template-part {"slug":"header","align":"full","tagName":"header","className":"site-header","theme":"%s"} /--><!-- /wp:group -->',
get_stylesheet()
);
$this->assertSame( $expected, $template_content );
// Does not inject theme when there is an existing theme attribute.
$content_with_existing_theme_attribute = '<!-- wp:template-part {"slug":"header","theme":"fake-theme","align":"full", "tagName":"header","className":"site-header"} /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_with_existing_theme_attribute,
$theme
);
$this->assertSame( $content_with_existing_theme_attribute, $template_content );
// Does not inject theme when there is no template part.
$content_with_no_template_part = '<!-- wp:post-content /-->';
$template_content = _inject_theme_attribute_in_block_template_content(
$content_with_no_template_part,
$theme
);
$this->assertSame( $content_with_no_template_part, $template_content );
}

@ockham
Copy link
Contributor Author

ockham commented Sep 6, 2023

Ah nevermind, that test literally just covers the _inject_theme_attribute_in_block_template_content function -- not _build_block_template_result_from_file (from which it is called) 🤦‍♂️

@ockham ockham closed this Sep 6, 2023
@ockham ockham deleted the try/do-tests-detect-missing-theme-attribute-injection branch September 6, 2023 13:50
@ockham ockham restored the try/do-tests-detect-missing-theme-attribute-injection branch September 7, 2023 18:40
@ockham ockham reopened this Sep 7, 2023
@ockham
Copy link
Contributor Author

ockham commented Sep 13, 2023

Closing. I've added test coverage for this in #5186 (https://core.trac.wordpress.org/changeset/56562/).

@ockham ockham closed this Sep 13, 2023
@ockham ockham deleted the try/do-tests-detect-missing-theme-attribute-injection branch September 13, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant