#56266 closed task (blessed) (fixed)
Backport missing Gutenberg tests to Core
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.1 | Priority: | normal |
Severity: | minor | Version: | 5.9 |
Component: | Editor | Keywords: | has-patch has-unit-tests commit |
Focuses: | Cc: |
Description
WP_Navigation_Test and WP_Global_Styles_Test test classes haven't been backported from Gutenberg to Core.
Change History (17)
This ticket was mentioned in PR #3005 on WordPress/wordpress-develop by anton-vlasenko.
17 months ago
#3
- Keywords has-patch has-unit-tests added; needs-patch removed
This PR aims to backport WP_Navigation_Test
and WP_Global_Styles_Test
test classes to Core.
These tests haven't been back ported to Core.
Trac ticket: https://core.trac.wordpress.org/ticket/56266
#4
@
17 months ago
Testing instructions for https://github.com/WordPress/wordpress-develop/pull/3005:
Make sure that the unit tests pass.
That PR only contains unit tests.
Props to @ntsekouras for discovering this issue.
Original tests classes:
#5
@
16 months ago
- Keywords commit added
Thanks @antonvlasenko! I approved the PR and marking for commit
consideration. @SergeyBiryukov Do you have time to take a look at this one?
ironprogrammer commented on PR #3005:
16 months ago
#6
Each of these migrated tests have the same test method name, test_it_correctly_handles_different_post_types()
, which seems awkward compared with others in the project. Maybe it's just me, but an alternative might be to simplify it to test_handle_different_post_types()
. Why?
- The inclusion of
_it_
seems unnecessary in the context of these isolated tests, *UNLESS* additional clarity is desired, in which case this should include the function name under test, which seems pretty common. - We should assume that unit tests already test for "correct" results, making that word redundant. So "incorrect"/error messages for assertions should be defined in their
message
parameters. - Like with git commit messages, the imperative voice tends to read more easily.
Likewise, `test_wp_filter_global_styles_post_returns_correct_value()` mentions "correct" in its name, but the test is to "return the same value", hence _return_same_value
sounds closer to the intent.
Of course none of this is critical, and maybe only matters to those who stare at unit tests and terminal output! Just </two-cents>
😂
16 months ago
#7
@ironprogrammer Each of these migrated tests have the same test method name, test_it_correctly_handles_different_post_types(), which seems awkward compared with others in the project. Maybe it's just me, but an alternative might be to simplify it to test_handle_different_post_types().
I think test_should_
/ test_should_not_
have precedent as test method prefixes in Core and are certainly consistent with wider unit/integration/e2e testing, making it easier for experienced testers when they begin contributing to Core.
I had a longer message in mind, but as this comment will appear on the Trac ticket, I've instead put this in a Gist to keep the noise down. I'm curious to hear your thoughts on this, so feel free to DM me on Slack to discuss further.
anton-vlasenko commented on PR #3005:
16 months ago
#8
There is no need to prefix the data provider with
data_test_
.
My bad. Thanks for the review, @costdev!
I've fixed it.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
15 months ago
robinwpdeveloper commented on PR #3005:
15 months ago
#10
PR looks good to me! 👍
#12
@
15 months ago
- Type changed from enhancement to task (blessed)
Let's convert this to a task instead of an enhancement.
#13
@
15 months ago
- Owner set to hellofromTonya
- Status changed from new to reviewing
Self-assigning for review and commit.
hellofromtonya commented on PR #3005:
15 months ago
#14
Rebased on top of current trunk
to ensure the new tests cover any changes without introducing any test leaks to other tests.
hellofromtonya commented on PR #3005:
14 months ago
#17
Committed via changeset https://core.trac.wordpress.org/changeset/54382. Thank you everyone for your contributions!
I'm working on a patch for this issue.