Repository navigation
Tests: Add unit coverage for the_permalink_rss() - #15371
shail-mehta wants to merge 3 commits into
Conversation
Cover the the_permalink_rss filter arguments and escaped feed output. See #65819.
There was a problem hiding this comment.
🟡 Changes recommended
The permalink RSS test uses a URL unchanged by esc_url(), so it cannot detect missing escaping or escaping applied before the filter.
1 open issue
What changed in this PR
Adds dedicated PHPUnit coverage for the_permalink_rss(), complementing the existing feed tests without changing production code.
Changes:
- Adds a post fixture and direct
@coversannotation. - Checks filter call count, arguments, and captured output against an escaped expected value.
| File | Description |
|---|---|
tests/phpunit/tests/feed/thePermalinkRss.php |
Adds a focused test for the feed permalink filter and output. |
Review effort: Balanced
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
|
lancewillett
left a comment
lancewillett
left a comment
There was a problem hiding this comment.
The escaping concern from Copilot is confirmed locally: removing esc_url() from the_permalink_rss() still leaves all three assertions passing. The appended URL contains nothing that escaping changes.
Please use a filtered URL containing characters that must be escaped and assert the literal escaped output, without calling esc_url() to calculate the expectation. Keep this in the existing test; the output assertion can replace the mock call-count check. Targeted single-site/multisite runs and PHPCS pass, but the claimed escaping protection needs this correction.
AI review · gpt-6
| $filtered = null; | ||
|
|
||
| add_filter( | ||
| 'the_permalink_rss', | ||
| static function ( $url ) use ( &$filtered ) { | ||
| $filtered = $url; | ||
|
|
||
| // Include a raw ampersand so missing esc_url() would fail the assertion below. | ||
| return 'https://example.org/?a=1&b=2'; | ||
| } | ||
| ); | ||
|
|
||
| ob_start(); | ||
| the_permalink_rss(); | ||
| $output = ob_get_clean(); | ||
|
|
||
| $this->assertSame( $permalink, $filtered ); |
lancewillett
left a comment
lancewillett
left a comment
There was a problem hiding this comment.
Approved at a679a7b. The test now uses a discriminating filtered URL and a literal escaped result. Mutation check: removing esc_url() and discarding the filter result are both caught. On Copilot's thread: the call count was dropped on purpose in the earlier review.
1 test, 2 assertions in random order, single site and multisite. PHPCS clean. CI green.
AI review · claude-opus-5-5
|
Thanks, @shail-mehta. Per #65819 comment 1521, test-only changes now get their own Trac ticket in the component of the code under test, instead of #65819. That now includes open PRs like this one. Could you open a new ticket for this change (likely Feeds)? Please add the AI review · |
|
Created Ticket: #66473 |
…alink_rss()`. Add a test that the `the_permalink_rss` filter receives the post permalink, and that the filtered URL is escaped in the feed output. Developed in: #15371 Props shailu25. Fixes #66473. git-svn-id: https://develop.svn.wordpress.org/trunk@65256 602fd350-edb4-49c9-b593-d223f7449a82
…alink_rss()`. Add a test that the `the_permalink_rss` filter receives the post permalink, and that the filtered URL is escaped in the feed output. Developed in: WordPress/wordpress-develop#15371 Props shailu25. Fixes #66473. Built from https://develop.svn.wordpress.org/trunk@65256 git-svn-id: http://core.svn.wordpress.org/trunk@64405 1a063a9b-81f0-0310-95a4-ce76da25c4cd
github-project-automation
Bot
moved this from In review
to Done
in WordPress Project Build Tooling
Trac ticket: #66473
What
Adds a dedicated PHPUnit class for
the_permalink_rss(), covering thethe_permalink_rssfilter arguments and escaped feed output.Test-only; no production code changes.
Why
the_permalink_rss()had no direct@coverscoverage. Existing RSS2/Atom feed tests already assert permalink content viaget_permalink(); this keeps the feed permalink filter +esc_url()contract.How
Add
tests/phpunit/tests/feed/thePermalinkRss.phpwith@covers ::the_permalink_rss, using a local post fixture and asserting filter call count, arguments, and escaped output.Testing Instructions
Expected: OK (1 test, 3 assertions).
Use of AI Tools