-
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
Added unit test for post excerpt block render function #43451
Added unit test for post excerpt block render function #43451
Conversation
@draganescu Can you please review this PR ? |
waiting for PR review |
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've reviewed this PR. You can find my notes below.
Thanks!
$block = new stdClass(); | ||
|
||
// call render method with block context. | ||
$rendered = gutenberg_render_block_core_Post_Excerpt( self::$attributes, '', $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.
$rendered = gutenberg_render_block_core_Post_Excerpt( self::$attributes, '', $block ); | |
$rendered = gutenberg_render_block_core_post_excerpt( self::$attributes, '', $block ); |
Why does this function use capital letters in its name?
Is it a typo?
$this->assertEmpty( $rendered ); | ||
|
||
$block->context = array( 'postId' => 0 ); | ||
$rendered = gutenberg_render_block_core_Post_Excerpt( self::$attributes, '', $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.
} | ||
|
||
/** | ||
* Test gutenberg_render_block_core_Post_Excerpt() method |
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
$block->context = array( 'postId' => self::$post_empty->ID ); | ||
|
||
$rendered = gutenberg_render_block_core_post_excerpt( self::$attributes, '', $block ); | ||
$this->assertEmpty( $rendered ); |
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.
When a test method contains more than 1 assertion, a failure message should be added to the assertion method.
Please see https://make.wordpress.org/core/handbook/testing/automated-testing/writing-phpunit-tests/#using-assertions
|
||
$rendered = gutenberg_render_block_core_post_excerpt( self::$attributes, '', $block ); | ||
$this->assertNotEmpty( $rendered ); | ||
$this->assertStringContainsString( 'Post Expert content', $rendered ); |
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.
Please add failure messages.
@anton-vlasenko Feedback addressed. Can you please review it again. |
… add-test-for-post-excerpt-block-render
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Test files should be named using camelCase.
… add-test-for-post-excerpt-block-render
… add-test-for-post-excerpt-block-render
I have updated phpunit config temporary until we change all filenames to camel case as per wordpress core standard. And Addressed all feedback provided by reviewer. |
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.
These comments are mostly minor (sorry for being so meticulous). Thank you for considering my review.
protected static $post; | ||
|
||
/** | ||
* Array of Attributes. |
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.
* Array of Attributes. | |
* Array of attributes. |
@anton-vlasenko I have addressed your feedbacks. |
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.
Since each test method initializes the $block
variable, I think it could be moved to the set_up()
method.
However, I don't think this should be a blocker.
LGTM.
Can we merge this PR ? |
@draganescu if you have space, can you review this PR too #42435 |
I apologize for assuming you could merge it yourself. |
No worries |
* Added unittest for post excerpt block render function * Updated wpSetUpBeforeClass method with argument * Addressed feedbacks * fix unit test failure * Fix coding standards * Fix phpcs issue * Update config to allow files with new name standard * Change test file name to camel case * Update assertion comments and formatting --------- Co-authored-by: Sunil Prajapati <61308756+sunil25393@users.noreply.github.com> Co-authored-by: akasunil <sunil25393@git.wordpress.org> Co-authored-by: anton-vlasenko <antonvlasenko@git.wordpress.org>
* Added unittest for post excerpt block render function * Updated wpSetUpBeforeClass method with argument * Addressed feedbacks * fix unit test failure * Fix coding standards * Fix phpcs issue * Update config to allow files with new name standard * Change test file name to camel case * Update assertion comments and formatting --------- Co-authored-by: Sunil Prajapati <61308756+sunil25393@users.noreply.github.com> Co-authored-by: akasunil <sunil25393@git.wordpress.org> Co-authored-by: anton-vlasenko <antonvlasenko@git.wordpress.org>
What?
Added unit test for Post excerpt core block render function.