Skip to content

52988: Adds tests for get bookmark - #1180

Closed
hellofromtonya wants to merge 24 commits into
WordPress:masterfrom
hellofromtonya:add/52988/get_bookmark
Closed

52988: Adds tests for get bookmark#1180
hellofromtonya wants to merge 24 commits into
WordPress:masterfrom
hellofromtonya:add/52988/get_bookmark

Conversation

@hellofromtonya

@hellofromtonyahellofromtonya commented Apr 7, 2021

Copy link
Copy Markdown
Contributor

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

Adds unit tests for get_bookmark().

The tests are broken up by the logic path through the if/elseif/else branches.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@hellofromtonyahellofromtonya self-assigned this Apr 7, 2021
@hellofromtonya
hellofromtonya marked this pull request as ready for review April 8, 2021 18:54
@hellofromtonya
hellofromtonya requested review from SergeyBiryukov and removed request for jrfnlApril 12, 2021 14:54

@peterwilsonccpeterwilsoncc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As always, this is lovely.

As always, I've added a few minor questions and comments inline but nothing major.

Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated
Comment threadtests/phpunit/tests/bookmark/getBookmark.php Outdated

@peterwilsonccpeterwilsoncc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good to me.

It looks like there was an accidental change to package-lock.json included in 3111683 but I'll preapprove as it's really the committers responsibility to check against these things.

@hellofromtonya

Copy link
Copy Markdown
ContributorAuthor

It looks like there was an accidental change to package-lock.json included in 3111683 but I'll preapprove as it's really the committers responsibility to check against these things.

Yup, I did. Doh. Way too easy to accidentally commit lock file. Luckily, it's straightforward for the core committer to skip over that file and only commit the test file.

Thanks @peterwilsoncc!

@peterwilsoncc

Copy link
Copy Markdown
Contributor

@hellofromtonya
hellofromtonya deleted the add/52988/get_bookmark branch January 5, 2022 16:51
Sign up for freeto 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.

3 participants

@hellofromtonya@peterwilsoncc@jrfnl