![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
#56266 closed task (blessed) (fixed)
Backport missing Gutenberg tests to Core
| Reported by: | antonvlasenko | Owned by: | hellofromTonya |
|---|---|---|---|
| Priority: | normal | Milestone: | 6.1 |
| Component: | Editor | Version: | 5.9 |
| Severity: | minor | Keywords: | has-patch has-unit-tests commit |
| Cc: | Focuses: |
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.
4 years 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
@
4 years 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
@
4 years 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:
4 years 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
messageparameters. - 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> 😂
4 years 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:
4 years 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.
4 years ago
robinwpdeveloper commented on PR #3005:
4 years ago
#10
PR looks good to me! 👍
#12
@
4 years ago
- Type enhancement → task (blessed)
Let's convert this to a task instead of an enhancement.
hellofromtonya commented on PR #3005:
4 years 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:
4 years 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.