-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Comment Template: Reorganize block tests to ease backport process #40739
Conversation
@@ -45,8 +45,7 @@ public function setUp() { | |||
); | |||
} | |||
|
|||
function test_build_comment_query_vars_from_block_with_context_no_pagination() { | |||
update_option( 'page_comments', false ); | |||
function test_build_comment_query_vars_from_block_with_context() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the order of test_build_comment_query_vars_from_block_with_context
and test_build_comment_query_vars_from_block_with_context_no_pagination
.
As the content of both functions is pretty similar, git
thinks I just modified the name.
'number' => 5, | ||
'paged' => 1, | ||
), | ||
build_comment_query_vars_from_block( $block ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I changed the order of the arguments. It should be like this:
$this->assertEquals( $expected, $actual );
Here, build_comment_query_vars_from_block( $block )
should be the $actual
value.
'paged' => 1, | ||
) | ||
), | ||
build_comment_query_vars_from_block( $block ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing I mentioned previously with the order of the arguments. ☝️
); | ||
$actual = build_comment_query_vars_from_block( $block ); | ||
$this->assertSame( $comment_query_max_num_pages, $actual['paged'] ); | ||
$this->assertSame( $comment_query_max_num_pages, get_query_var( 'cpage' ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this code was placed in a different position in wordpress-develop
, so I moved it.
'comments/inherit' => true, | ||
) | ||
$expected, | ||
str_replace( array( "\n", "\t" ), '', $block->render() ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguments in the wrong order (again).
@@ -309,8 +307,6 @@ function test_render_block_core_comment_content_converts_to_html() { | |||
|
|||
$expected_content = "<p>Paragraph One</p>\n<p>P2L1<br />\nP2L2</p>\n<p><a href=\"https://example.com/\" rel=\"nofollow ugc\">https://example.com/</a></p>\n"; | |||
|
|||
// Here we use the function prefixed with 'gutenberg_*' because it's added | |||
// in the build step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced gutenberg_render_block_core_comment_template
with $block->render()
some time ago, so this comments are not relevant anymore.
@@ -391,8 +387,6 @@ function test_rendering_comment_template_unmoderated_preview() { | |||
|
|||
add_filter( 'wp_get_current_commenter', $commenter_filter ); | |||
|
|||
// Here we use the function prefixed with 'gutenberg_*' because it's added | |||
// in the build step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. ☝️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as on the other PR, more precise assertions.
Similar changes may be needed elsewhere in the file but outside the diff.
@@ -59,20 +58,22 @@ function test_build_comment_query_vars_from_block_with_context_no_pagination() { | |||
); | |||
|
|||
$this->assertEquals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->assertEquals( | |
$this->assertSameSetsWithIndex( |
@@ -85,18 +86,17 @@ function test_build_comment_query_vars_from_block_with_context() { | |||
); | |||
|
|||
$this->assertEquals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->assertEquals( | |
$this->assertSameSetsWithIndex( |
@@ -107,7 +107,6 @@ function test_build_comment_query_vars_from_block_no_context() { | |||
$block = new WP_Block( $parsed_blocks[0] ); | |||
|
|||
$this->assertEquals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->assertEquals( | |
$this->assertSameSetsWithIndex( |
@@ -391,8 +387,6 @@ function test_rendering_comment_template_unmoderated_preview() { | |||
|
|||
add_filter( 'wp_get_current_commenter', $commenter_filter ); | |||
|
|||
// Here we use the function prefixed with 'gutenberg_*' because it's added | |||
// in the build step. | |||
$this->assertEquals( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$this->assertEquals( | |
$this->assertSame( |
Thanks @DAreRodz, this should make things much easier! Generally speaking, we'll want to start removing unit tests from GB once they have been backported to WP, in order to avoid the hassle that the duplication of the tests might cause. (This is established practice in GB, see e.g. #37159.) The idea is that any further changes to the implementation or test should then happen in This should be quite straight-forward for tests for library functions such as |
@peterwilsoncc, I just saw your proposed changes. I'll add them tomorrow. 👍 And thanks, @ockham, for the clarification. 🙂 |
I've done the requested changes, @peterwilsoncc (sorry for the delay here 🐌 ). |
Thanks a lot, @DAreRodz -- this makes it a lot easier to compare the current test with the one in I made a diff: --- wordpress-develop/tests/phpunit/tests/blocks/renderCommentTemplate.php 2022-05-10 11:45:56.000000000 +0200
+++ gutenberg/phpunit/class-block-library-comment-template-test.php 2022-05-10 11:46:21.000000000 +0200
@@ -1,27 +1,22 @@
<?php
/**
- * Comment Template block rendering tests.
+ * Tests the Comment Template block.
*
- * @package WordPress
- * @subpackage Blocks
- * @since 6.0.0
+ * @package Gutenberg
+ * @subpackage block-library
*/
/**
- * Tests for the Comment Template block.
- *
- * @since 6.0.0
- *
- * @group blocks
+ * Tests for rendering a list of comments
*/
-class Tests_Blocks_RenderReusableCommentTemplate extends WP_UnitTestCase {
+class Block_Library_Comment_Template_Test extends WP_UnitTestCase {
private static $custom_post;
private static $comment_ids;
private static $per_page = 5;
- public function set_up() {
- parent::set_up();
+ public function setUp() {
+ parent::setUp();
update_option( 'page_comments', true );
update_option( 'comments_per_page', self::$per_page );
@@ -50,10 +45,6 @@
);
}
- /**
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_with_context() {
$parsed_blocks = parse_blocks(
'<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
@@ -66,7 +57,7 @@
)
);
- $this->assertEquals(
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -81,10 +72,6 @@
);
}
- /**
- * @ticket 55567
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_with_context_no_pagination() {
update_option( 'page_comments', false );
$parsed_blocks = parse_blocks(
@@ -112,10 +99,6 @@
update_option( 'page_comments', true );
}
- /**
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_no_context() {
$parsed_blocks = parse_blocks(
'<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
@@ -123,7 +106,7 @@
$block = new WP_Block( $parsed_blocks[0] );
- $this->assertEquals(
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -138,68 +121,9 @@
}
/**
- * Test that if pagination is set to display the last page by default (i.e. newest comments),
- * the query is set to look for page 1 (rather than page 0, which would cause an error).
- *
- * Regression: https://github.com/WordPress/gutenberg/issues/40758.
- *
- * @ticket 55658
- * @covers ::build_comment_query_vars_from_block
- */
- function test_build_comment_query_vars_from_block_pagination_with_no_comments() {
- $comments_per_page = get_option( 'comments_per_page' );
- $default_comments_page = get_option( 'default_comments_page' );
-
- update_option( 'comments_per_page', 50 );
- update_option( 'previous_default_page', 'newest' );
-
- $post_without_comments = self::factory()->post->create_and_get(
- array(
- 'post_type' => 'post',
- 'post_status' => 'publish',
- 'post_name' => 'fluffycat',
- 'post_title' => 'Fluffy Cat',
- 'post_content' => 'Fluffy Cat content',
- 'post_excerpt' => 'Fluffy Cat',
- )
- );
-
- $parsed_blocks = parse_blocks(
- '<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
- );
-
- $block = new WP_Block(
- $parsed_blocks[0],
- array(
- 'postId' => $post_without_comments->ID,
- )
- );
-
- $this->assertSameSetsWithIndex(
- array(
- 'orderby' => 'comment_date_gmt',
- 'order' => 'ASC',
- 'status' => 'approve',
- 'no_found_rows' => false,
- 'post_id' => $post_without_comments->ID,
- 'hierarchical' => 'threaded',
- 'number' => 50,
- ),
- build_comment_query_vars_from_block( $block )
- );
-
- update_option( 'comments_per_page', $comments_per_page );
- update_option( 'default_comments_page', $default_comments_page );
- }
-
-
- /**
* Test that both "Older Comments" and "Newer Comments" are displayed in the correct order
* inside the Comment Query Loop when we enable pagination on Discussion Settings.
* In order to do that, it should exist a query var 'cpage' set with the $comment_args['paged'] value.
- *
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
*/
function test_build_comment_query_vars_from_block_sets_cpage_var() {
@@ -235,8 +159,6 @@
/**
* Test rendering a single comment
- *
- * @ticket 55567
*/
function test_rendering_comment_template() {
$parsed_blocks = parse_blocks(
@@ -263,8 +185,6 @@
* └─ comment 2
* └─ comment 4
* └─ comment 3
- *
- * @ticket 55567
*/
function test_rendering_comment_template_nested() {
$first_level_ids = self::factory()->comment->create_post_comments(
@@ -364,8 +284,6 @@
/**
* Test that line and paragraph breaks are converted to HTML tags in a comment.
- *
- * @ticket 55643
*/
function test_render_block_core_comment_content_converts_to_html() {
$comment_id = self::$comment_ids[0];
@@ -397,9 +315,6 @@
/**
* Test that unapproved comments are included if it is a preview.
- *
- * @ticket 55634
- * @covers ::build_comment_query_vars_from_block
*/
function test_build_comment_query_vars_from_block_with_comment_preview() {
$parsed_blocks = parse_blocks(
@@ -421,8 +336,7 @@
add_filter( 'wp_get_current_commenter', $commenter_filter );
- $this->assertEquals(
- build_comment_query_vars_from_block( $block ),
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -433,14 +347,13 @@
'hierarchical' => 'threaded',
'number' => 5,
'paged' => 1,
- )
+ ),
+ build_comment_query_vars_from_block( $block )
);
}
/**
* Test rendering an unapproved comment preview.
- *
- * @ticket 55643
*/
function test_rendering_comment_template_unmoderated_preview() {
$parsed_blocks = parse_blocks(
JSDoc and class names aside, AFAICS, the remaining differences in
So let's get this PR merged. As a follow-up, I'll file a PR against |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misspoke; after merging, the diff between GB's and --- /Users/bernhardreiter/src/wordpress-develop/tests/phpunit/tests/blocks/renderCommentTemplate.php 2022-05-10 13:32:26.000000000 +0200
+++ phpunit/class-block-library-comment-template-test.php 2022-05-10 13:29:23.000000000 +0200
@@ -1,27 +1,22 @@
<?php
/**
- * Comment Template block rendering tests.
+ * Tests the Comment Template block.
*
- * @package WordPress
- * @subpackage Blocks
- * @since 6.0.0
+ * @package Gutenberg
+ * @subpackage block-library
*/
/**
- * Tests for the Comment Template block.
- *
- * @since 6.0.0
- *
- * @group blocks
+ * Tests for rendering a list of comments
*/
-class Tests_Blocks_RenderReusableCommentTemplate extends WP_UnitTestCase {
+class Block_Library_Comment_Template_Test extends WP_UnitTestCase {
private static $custom_post;
private static $comment_ids;
private static $per_page = 5;
- public function set_up() {
- parent::set_up();
+ public function setUp() {
+ parent::setUp();
update_option( 'page_comments', true );
update_option( 'comments_per_page', self::$per_page );
@@ -50,10 +45,6 @@
);
}
- /**
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_with_context() {
$parsed_blocks = parse_blocks(
'<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
@@ -66,7 +57,7 @@
)
);
- $this->assertEquals(
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -81,10 +72,6 @@
);
}
- /**
- * @ticket 55567
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_with_context_no_pagination() {
update_option( 'page_comments', false );
$parsed_blocks = parse_blocks(
@@ -112,10 +99,6 @@
update_option( 'page_comments', true );
}
- /**
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
- */
function test_build_comment_query_vars_from_block_no_context() {
$parsed_blocks = parse_blocks(
'<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
@@ -123,7 +106,7 @@
$block = new WP_Block( $parsed_blocks[0] );
- $this->assertEquals(
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -138,12 +121,48 @@
}
/**
+ * Test that both "Older Comments" and "Newer Comments" are displayed in the correct order
+ * inside the Comment Query Loop when we enable pagination on Discussion Settings.
+ * In order to do that, it should exist a query var 'cpage' set with the $comment_args['paged'] value.
+ */
+ function test_build_comment_query_vars_from_block_sets_cpage_var() {
+
+ // This could be any number, we set a fixed one instead of a random for better performance.
+ $comment_query_max_num_pages = 5;
+ // We subtract 1 because we created 1 comment at the beginning.
+ $post_comments_numbers = ( self::$per_page * $comment_query_max_num_pages ) - 1;
+ self::factory()->comment->create_post_comments(
+ self::$custom_post->ID,
+ $post_comments_numbers,
+ array(
+ 'comment_author' => 'Test',
+ 'comment_author_email' => '[email protected]',
+ 'comment_author_url' => 'http://example.com/author-url/',
+ 'comment_content' => 'Hello world',
+ )
+ );
+ $parsed_blocks = parse_blocks(
+ '<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
+ );
+
+ $block = new WP_Block(
+ $parsed_blocks[0],
+ array(
+ 'postId' => self::$custom_post->ID,
+ 'comments/inherit' => true,
+ )
+ );
+ $actual = build_comment_query_vars_from_block( $block );
+ $this->assertSame( $comment_query_max_num_pages, $actual['paged'] );
+ $this->assertSame( $comment_query_max_num_pages, get_query_var( 'cpage' ) );
+ }
+
+ /**
* Test that if pagination is set to display the last page by default (i.e. newest comments),
* the query is set to look for page 1 (rather than page 0, which would cause an error).
*
* Regression: https://github.com/WordPress/gutenberg/issues/40758.
*
- * @ticket 55658
* @covers ::build_comment_query_vars_from_block
*/
function test_build_comment_query_vars_from_block_pagination_with_no_comments() {
@@ -175,7 +194,7 @@
)
);
- $this->assertSameSetsWithIndex(
+ $this->assertEquals(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -192,51 +211,8 @@
update_option( 'default_comments_page', $default_comments_page );
}
-
- /**
- * Test that both "Older Comments" and "Newer Comments" are displayed in the correct order
- * inside the Comment Query Loop when we enable pagination on Discussion Settings.
- * In order to do that, it should exist a query var 'cpage' set with the $comment_args['paged'] value.
- *
- * @ticket 55505
- * @covers ::build_comment_query_vars_from_block
- */
- function test_build_comment_query_vars_from_block_sets_cpage_var() {
-
- // This could be any number, we set a fixed one instead of a random for better performance.
- $comment_query_max_num_pages = 5;
- // We subtract 1 because we created 1 comment at the beginning.
- $post_comments_numbers = ( self::$per_page * $comment_query_max_num_pages ) - 1;
- self::factory()->comment->create_post_comments(
- self::$custom_post->ID,
- $post_comments_numbers,
- array(
- 'comment_author' => 'Test',
- 'comment_author_email' => '[email protected]',
- 'comment_author_url' => 'http://example.com/author-url/',
- 'comment_content' => 'Hello world',
- )
- );
- $parsed_blocks = parse_blocks(
- '<!-- wp:comment-template --><!-- wp:comment-author-name /--><!-- wp:comment-content /--><!-- /wp:comment-template -->'
- );
-
- $block = new WP_Block(
- $parsed_blocks[0],
- array(
- 'postId' => self::$custom_post->ID,
- 'comments/inherit' => true,
- )
- );
- $actual = build_comment_query_vars_from_block( $block );
- $this->assertSame( $comment_query_max_num_pages, $actual['paged'] );
- $this->assertSame( $comment_query_max_num_pages, get_query_var( 'cpage' ) );
- }
-
/**
* Test rendering a single comment
- *
- * @ticket 55567
*/
function test_rendering_comment_template() {
$parsed_blocks = parse_blocks(
@@ -263,8 +239,6 @@
* └─ comment 2
* └─ comment 4
* └─ comment 3
- *
- * @ticket 55567
*/
function test_rendering_comment_template_nested() {
$first_level_ids = self::factory()->comment->create_post_comments(
@@ -364,8 +338,6 @@
/**
* Test that line and paragraph breaks are converted to HTML tags in a comment.
- *
- * @ticket 55643
*/
function test_render_block_core_comment_content_converts_to_html() {
$comment_id = self::$comment_ids[0];
@@ -397,9 +369,6 @@
/**
* Test that unapproved comments are included if it is a preview.
- *
- * @ticket 55634
- * @covers ::build_comment_query_vars_from_block
*/
function test_build_comment_query_vars_from_block_with_comment_preview() {
$parsed_blocks = parse_blocks(
@@ -421,8 +390,7 @@
add_filter( 'wp_get_current_commenter', $commenter_filter );
- $this->assertEquals(
- build_comment_query_vars_from_block( $block ),
+ $this->assertSameSetsWithIndex(
array(
'orderby' => 'comment_date_gmt',
'order' => 'ASC',
@@ -433,14 +401,13 @@
'hierarchical' => 'threaded',
'number' => 5,
'paged' => 1,
- )
+ ),
+ build_comment_query_vars_from_block( $block )
);
}
/**
* Test rendering an unapproved comment preview.
- *
- * @ticket 55643
*/
function test_rendering_comment_template_unmoderated_preview() {
$parsed_blocks = parse_blocks(
That is:
|
What?
Move some changes made in
phpunit/tests/blocks/renderCommentTemplate.php
(wordpress-develop) tophpunit/class-block-library-comment-template-test.php
(gutenberg).Why?
Although both files should contain (almost) the same code, they have been differentiated as development progressed. This PR makes them more similar and will ease the backporting of future changes.