Opened 6 days ago
Last modified 6 days ago
#66494 new enhancement
Tests: Add unit coverage for the comment functions in comment.php
| Reported by: | hmbashar | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Awaiting Review |
| Component: | Comments | Version: | |
| Severity: | normal | Keywords: | has-patch has-unit-tests |
| Cc: | Focuses: | tests |
Description
src/wp-includes/comment.php has several functions that protect the comment form and keep the comment counts and statuses in sync, but they have no direct PHPUnit coverage. A search of tests/phpunit/tests finds no references to:
wp_check_comment_flood(),wp_throttle_comment_flood()andcheck_comment_flood_db(), which block comments that are posted too quickly.wp_check_comment_data_max_lengths(), which rejects comment data that is too long for the database columns.wp_defer_comment_counting()andwp_update_comment_count(), which defer and flush the updates of the comment counts of posts.wp_transition_comment_status(), which fires the actions of a comment status change.get_comment_statuses()andsanitize_comment_cookies().
Some of these are only exercised indirectly through wp_allow_comment() and wp_new_comment(), so a change in their behavior is hard to spot and to attribute. Direct tests describe the current behavior of each function.
Proposed scope
This ticket is meant to be handled with several small patches, one for each group of related functions, so each one is easy to review. The tests go in tests/phpunit/tests/comment/, one class per function, with data providers where it helps.
- Flood protection.
wp_throttle_comment_flood(): an earlier block is kept, and the 15 second limit.wp_check_comment_flood(): a flood that another callback already declared is kept, administrators and moderators are not throttled, logged-in users are matched by user and visitors by IP address or email, the one hour window, thecomment_flood_filterandcomment_flood_triggerhooks, and$avoid_die.check_comment_flood_db(): it adds thewp_is_comment_floodfilter. - Limits and counts.
wp_check_comment_data_max_lengths(): each of the four fields at and over its limit, with multibyte characters, and the error codes and data.wp_defer_comment_counting()andwp_update_comment_count(): counts are not updated while deferred, the deferred posts are flushed once, and the return values. - Statuses and cookies.
wp_transition_comment_status(): the actions that fire, with the status aliases (hold,approve,0,1) and without a change.get_comment_statuses().sanitize_comment_cookies(): the filters, unslashing and escaping of the author, email and URL cookies.
Out of scope
This ticket only adds tests and does not change behavior. wp_set_comment_cookies() is left out, because it calls setcookie(), which cannot be asserted from the test suite. Changes to how the counts are updated are discussed in #58156, and the tests describe the current behavior only.
Change History (3)
This ticket was mentioned in PR #15529 on WordPress/wordpress-develop by @hmbashar.
6 days ago
#1
- Keywords has-patch has-unit-tests added; needs-patch removed
This ticket was mentioned in PR #15530 on WordPress/wordpress-develop by @hmbashar.
6 days ago
#2
Adds unit tests for the comment data length check and for the deferred comment counts in src/wp-includes/comment.php, which had no direct test coverage. This is the second group of the work described in the ticket, and is independent from the other groups. No behavior is changed.
Functions covered, each in its own test class in tests/phpunit/tests/comment/:
wp_check_comment_data_max_lengths()- Data within the limits, or without the fields, is accepted.
- For each of the four fields, a value as long as the limit is accepted, and a longer one is rejected with the error code, the message and the
200status of that field. - The author and the content are counted in bytes, so multibyte characters count more than once.
- The first field that is too long is the one that is reported (author, email, URL, then content).
- The limits come from the
wp_get_comment_fields_max_lengthsfilter.
wp_defer_comment_counting()- Counting is not deferred by default, the state can be turned on and off, and a value that is not a boolean does not change it.
- The counts of the posts are updated when deferring is turned off, and not updated while it is on.
wp_update_comment_count()- Nothing is done without a post ID, and the count is updated at once when counting is not deferred.
- When counting is deferred the post is queued and its count is not changed.
- Each queued post is updated once, in order, when the queue is flushed, and the queue is emptied.
The state of wp_defer_comment_counting() is kept in a static variable, so the tests that use it turn deferring off before and after each test. The behavior of wp_update_comment_count() is not changed here; ideas to change it are in #58156, and these tests describe the current behavior only.
Testing:
vendor/bin/phpunit --filter 'Tests_Comment_(WpCheckCommentDataMaxLengths|WpDeferCommentCounting|WpUpdateCommentCount)'vendor/bin/phpunit -c tests/phpunit/multisite.xml --filter 'Tests_Comment_(WpCheckCommentDataMaxLengths|WpDeferCommentCounting|WpUpdateCommentCount)'vendor/bin/phpunit --group commentvendor/bin/phpcs tests/phpunit/tests/comment/
I also temporarily broke the related lines in src/wp-includes/comment.php (the comparison and the byte counting of the author and content, each of the email, URL and content checks, the status of the error, the boolean check of the deferring, the flush, the removal of duplicates from the queue, emptying the queue, the check for a missing post ID, and the queueing itself) and confirmed that a new test fails for each one.
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5.5
Used for: Written with AI and manually reviewed by me.
This ticket was mentioned in PR #15531 on WordPress/wordpress-develop by @hmbashar.
6 days ago
#3
Adds unit tests for the comment statuses, the actions of a status change, and the comment author cookies in src/wp-includes/comment.php, which had no direct test coverage. This is the third and last group of the work described in the ticket, and is independent from the other groups. No behavior is changed.
Functions covered, each in its own test class in tests/phpunit/tests/comment/:
get_comment_statuses()- The four statuses and their labels, in order.
wp_transition_comment_status()- The
transition_comment_status,comment_{old}_to_{new}andcomment_{new}_{type}actions fire in order, with the expected arguments. - The status aliases
hold,approve,0and1are mapped tounapprovedandapproved, for both the old and the new status. - When the status does not change, including when only an alias differs, only the action of the new status and the comment type fires.
- The comment type is part of the name of the last action, for
comment,pingback,trackbackand a custom type.
- The
sanitize_comment_cookies()- Cookies that are not set are not created, and values that need no change are kept.
- The name and the email address are unslashed and escaped for use in an attribute, and the URL is unslashed and not escaped.
- Each value goes through the
pre_comment_author_name,pre_comment_author_emailorpre_comment_author_urlfilter first.
wp_set_comment_cookies() is not covered, because it calls setcookie(), which cannot be asserted from the test suite. The tests of sanitize_comment_cookies() remove the default filters of the fields, so they only cover what the function does, and restore the cookies afterwards.
Testing:
vendor/bin/phpunit --filter 'Tests_Comment_(GetCommentStatuses|WpTransitionCommentStatus|SanitizeCommentCookies)'vendor/bin/phpunit -c tests/phpunit/multisite.xml --filter 'Tests_Comment_(GetCommentStatuses|WpTransitionCommentStatus|SanitizeCommentCookies)'vendor/bin/phpunit --group commentvendor/bin/phpcs tests/phpunit/tests/comment/
I also temporarily broke the related lines in src/wp-includes/comment.php (a label and the order of the statuses, the check for a changed status, each of the four aliases, the mapping of the old status, each of the actions, the unslashing and escaping of each cookie, and the filter of the name) and confirmed that a new test fails for each one.
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5.5
Used for: Written with AI and manually reviewed by me.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Adds unit tests for the comment flood protection functions in
src/wp-includes/comment.php, which had no direct test coverage. This is the first group of the work described in the ticket, and each of the groups is a separate PR. No behavior is changed.Functions covered, each in its own test class in
tests/phpunit/tests/comment/:wp_throttle_comment_flood()comment_flood_filterfilter.wp_check_comment_flood()comment_flood_filterfilter, thecomment_flood_triggeraction, thecomment_flood_messagefilter,$avoid_die, and the429response.wp_allow_comment()returns thecomment_flooderror for a flood.check_comment_flood_db()wp_check_comment_flood()to thewp_is_comment_floodfilter, runs on thecheck_comment_floodaction by default, and the check can be removed by unhooking it.Testing:
vendor/bin/phpunit --filter 'Tests_Comment_(WpThrottleCommentFlood|CheckCommentFloodDb|WpCheckCommentFlood)'vendor/bin/phpunit -c tests/phpunit/multisite.xml --filter 'Tests_Comment_(WpThrottleCommentFlood|CheckCommentFloodDb|WpCheckCommentFlood)'vendor/bin/phpunit --group commentvendor/bin/phpcs tests/phpunit/tests/comment/I also temporarily broke the related lines in
src/wp-includes/comment.php(the 15 second limit, the earlier decision, each of the two capability checks, the one hour window, matching logged-in users by IP address,$avoid_die, the trigger action, the response code, and the filter added bycheck_comment_flood_db()) and confirmed that a new test fails for each one.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5.5
Used for: Written with AI and manually reviewed by me.