Skip to content

Tests: Expand canonical coverage for feed-like slugs - #1

Open
AceMedia wants to merge 1 commit into
irozum:fix/21700-canonical-tag-slug-matching-feed-endpointfrom
AceMedia:ticket/21700-feed-slug-tests
Open

AceMedia wants to merge 1 commit into
irozum:fix/21700-canonical-tag-slug-matching-feed-endpointfrom
AceMedia:ticket/21700-feed-slug-tests

Conversation

@AceMedia

Copy link
Copy Markdown

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

This adds extra regression tests on top of irozum's PR, WordPress#14305. It is a tests-only commit on their exact head (bfcddbc) and does not touch src/. Props irozum.

What the tests cover

All additions are in tests/phpunit/tests/canonical.php, tagged @ticket 21700.

  • Tags, categories and a custom taxonomy, each with slugs feed, rss, rss2, rdf and atom, requested with a real trailing feed segment: /feed/, /feed/atom/, /feed/rdf/, /feed/rss/ and /feed/rss2/. The slug must be kept and the real feed segment resolved to the expected canonical feed URL. The /atom/ and /rss2/ shorthand is covered too, except for the slug feed, where /tag/feed/atom/ is ambiguous.
  • Single posts in a category with those slugs (/%category%/%postname%/) combined with /comment-page-2/, /feed/ and /feed/atom/. Post slugs themselves cannot match a feed endpoint, because wp_unique_post_slug() renames them, so the feed-like segment is the category.

That is 99 term cases and 15 post cases (114 new cases). All 114, plus the PR's own 5 feed cases, fail on trunk (PR source change reverted temporarily) and pass on the PR head. I removed cases that passed either way and did not enter the changed loop (plain archive URLs, ?cpage= and ?feed= on posts).

What the tests do not cover

Extra cases I tried fail on the PR head as well as on trunk, so I left them out of the commit: a term slug with a real second page (/tag/rss/page/2/, ?paged=2) and a term slug with ?feed=atom or ?feed=rss2, where the slug is still stripped (for example /tag/rss/page/2/ becomes /tag/page/2/). The PR's own paged cases pass only because page 2 does not exist with one post. I can share those cases if useful, but I have not changed the PR's production code.

Testing and limits

  • PHP 8.4.25, MariaDB 10.11.18, PHPUnit 9.6.35.
  • tests/phpunit/tests/canonical.php: 212 tests, 238 assertions, passing in single site and multisite configurations.
  • PHPCS and php -l are clean on the changed file.
  • Trunk comparison was a temporary local revert of the PR's src/wp-includes/canonical.php change, restored afterwards.
  • I did not run the wider Core suite, other databases or PHP versions.
  • WordPress Playground was not used: no browser was connected to it, so there is no live reproduction on stock WordPress.

AI assistance: Codex and Claude helped me assess the issue, write the tests and run the checks. I have reviewed the code myself. The test results are from automated runs through my local test setup.

Adds regression tests for `redirect_canonical()` where a term slug is named like a feed endpoint (`feed`, `rss`, `rss2`, `rdf`, `atom`). They cover tags, categories and a custom taxonomy with a real trailing feed segment (`/feed/`, `/feed/atom/`, `/feed/rdf/`, `/feed/rss/`, `/feed/rss2/` and the `/atom/` and `/rss2/` shorthand), and single posts in a category with such a slug combined with `/comment-page-2/` and comment feeds.

Props irozum.

See https://core.trac.wordpress.org/ticket/21700.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props shanerounce.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Sign up for free to 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.

1 participant