Make WordPress Core

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() and check_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() and wp_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() and sanitize_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.

  1. 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, the comment_flood_filter and comment_flood_trigger hooks, and $avoid_die. check_comment_flood_db(): it adds the wp_is_comment_flood filter.
  2. 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() and wp_update_comment_count(): counts are not updated while deferred, the deferred posts are flushed once, and the return values.
  3. 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.

Related: #65819, #58156.

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

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()
    • Comments less than 15 seconds apart are blocked, and 15 seconds or more are allowed.
    • An earlier decision to block is kept.
    • It is the default callback of the comment_flood_filter filter.
  • wp_check_comment_flood()
    • A flood found by another callback is kept.
    • Users who can manage options, or who can moderate comments, are not throttled (each capability is tested on its own).
    • A visitor is matched by IP address or email address, and a logged-in user by user ID and not by IP address.
    • Comments older than one hour are ignored.
    • The comment_flood_filter filter, the comment_flood_trigger action, the comment_flood_message filter, $avoid_die, and the 429 response.
    • wp_allow_comment() returns the comment_flood error for a flood.
  • check_comment_flood_db()
    • It adds wp_check_comment_flood() to the wp_is_comment_flood filter, runs on the check_comment_flood action 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 comment
  • vendor/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 by check_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.

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 200 status 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_lengths filter.
  • 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 comment
  • vendor/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} and comment_{new}_{type} actions fire in order, with the expected arguments.
    • The status aliases hold, approve, 0 and 1 are mapped to unapproved and approved, 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, trackback and a custom type.
  • 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_email or pre_comment_author_url filter 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 comment
  • vendor/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.

Note: See TracTickets for help on using tickets.