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

Added unit test for post excerpt block render function #43451

Merged

Conversation

akasunil
Copy link
Member

What?

Added unit test for Post excerpt core block render function.

@akasunil
Copy link
Member Author

@draganescu Can you please review this PR ?

@akasunil
Copy link
Member Author

akasunil commented Jan 2, 2023

waiting for PR review

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a 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!

phpunit/blocks/render-block-post-excerpt-test.php Outdated Show resolved Hide resolved
phpunit/blocks/render-block-post-excerpt-test.php Outdated Show resolved Hide resolved
$block = new stdClass();

// call render method with block context.
$rendered = gutenberg_render_block_core_Post_Excerpt( self::$attributes, '', $block );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$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 );
Copy link
Contributor

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
Copy link
Contributor

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 );
Copy link
Contributor

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 );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add failure messages.

@draganescu draganescu changed the title Added unittest for post excerpt block render function Jan 6, 2023
@skorasaurus skorasaurus added the [Block] Post Excerpt Affects the Post Excerpt Block label Apr 10, 2023
@akasunil
Copy link
Member Author

@anton-vlasenko Feedback addressed. Can you please review it again.

anton-vlasenko

This comment was marked as outdated.

Copy link

github-actions bot commented Feb 26, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: akasunil <sunil25393@git.wordpress.org>
Co-authored-by: anton-vlasenko <antonvlasenko@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@anton-vlasenko anton-vlasenko added Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core [Type] Code Quality Issues or PRs that relate to code quality Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release and removed Backport from WordPress Core Pull request that needs to be backported to a Gutenberg release from WordPress Core labels Feb 27, 2024
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phpunit/blocks/render-block-post-excerpt-test.php Outdated Show resolved Hide resolved
phpunit/blocks/render-block-post-excerpt-test.php Outdated Show resolved Hide resolved
phpunit/blocks/render-block-post-excerpt-test.php Outdated Show resolved Hide resolved
@youknowriad youknowriad removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Apr 3, 2024
@akasunil akasunil marked this pull request as draft April 19, 2024 08:55
@akasunil
Copy link
Member Author

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.

@akasunil akasunil marked this pull request as ready for review April 28, 2024 08:35
Copy link
Contributor

@anton-vlasenko anton-vlasenko left a 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Array of Attributes.
* Array of attributes.
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
phpunit/blocks/renderBlockCorePostExcerpt.php Outdated Show resolved Hide resolved
@akasunil
Copy link
Member Author

akasunil commented May 9, 2024

@anton-vlasenko I have addressed your feedbacks.

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a 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.

@akasunil
Copy link
Member Author

Can we merge this PR ?

@draganescu draganescu merged commit 7988696 into WordPress:trunk May 24, 2024
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 18.5 milestone May 24, 2024
@akasunil akasunil deleted the add-test-for-post-excerpt-block-render branch May 24, 2024 13:50
@akasunil
Copy link
Member Author

@draganescu if you have space, can you review this PR too #42435

@anton-vlasenko
Copy link
Contributor

Can we merge this PR ?

I apologize for assuming you could merge it yourself.
I'm glad to see it has been merged now.

@akasunil
Copy link
Member Author

I apologize for assuming you could merge it yourself.

No worries

carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jun 4, 2024
* 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>
patil-vipul pushed a commit to patil-vipul/gutenberg that referenced this pull request Jun 17, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Excerpt Affects the Post Excerpt Block [Type] Code Quality Issues or PRs that relate to code quality
5 participants