Skip to content

Tests: Improve unit test coverage for WP_Sitemaps_Renderer. - #13537

Open
huzaifaalmesbah wants to merge 2 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests-sitemaps-renderer
Open

huzaifaalmesbah wants to merge 2 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests-sitemaps-renderer

Conversation

@huzaifaalmesbah

Copy link
Copy Markdown
Member

This PR adds comprehensive unit test coverage for WP_Sitemaps_Renderer (src/wp-includes/sitemaps/class-wp-sitemaps-renderer.php) in tests/phpunit/tests/sitemaps/wpSitemapsRenderer.php.

Prior to this PR, several methods, supported specification tags, and edge cases in WP_Sitemaps_Renderer lacked dedicated unit tests.

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

Changes & New Tests Added

  1. WP_Sitemaps_Renderer::render_index():

    • test_render_index(): Verifies that index XML output is rendered and echoed properly.
    • test_render_index_empty(): Verifies that nothing is echoed when index XML is empty.
  2. WP_Sitemaps_Renderer::render_sitemap():

    • test_render_sitemap(): Verifies that sitemap XML output is rendered and echoed properly.
    • test_render_sitemap_empty(): Verifies that nothing is echoed when sitemap XML is empty.
  3. WP_Sitemaps_Renderer::get_sitemap_xml():

    • test_get_sitemap_xml_with_all_supported_fields(): Tests all supported sitemap tags (loc, lastmod, changefreq, and priority) to ensure full coverage of the tags block.
    • test_get_sitemap_xml_empty(): Verifies empty URL array returns valid <urlset .../> XML without child <url> entries.
    • test_get_sitemap_xml_escaping(): Tests proper XML entity escaping for special characters (&, <, >, ") across supported fields.
  4. WP_Sitemaps_Renderer::get_sitemap_index_xml():

    • test_get_sitemap_index_xml_empty(): Verifies empty entries array returns valid <sitemapindex .../> XML without child <sitemap> entries.
    • test_get_sitemap_index_xml_escaping(): Tests XML entity escaping for index entries.
  5. Stylesheet Filter Tests:

    • test_get_sitemap_stylesheet_url_filter(): Tests filtering wp_sitemaps_stylesheet_url to a custom URL and confirms the <?xml-stylesheet ?> processing instruction reflects the filtered URL.
    • test_get_sitemap_index_stylesheet_url_filter(): Tests filtering wp_sitemaps_stylesheet_index_url to a custom URL and confirms the <?xml-stylesheet ?> processing instruction reflects the filtered URL.

Testing Instructions

Run the targeted test suite:

npm run test:php -- --filter Tests_Sitemaps_wpSitemapsRenderer

Run all sitemaps tests to ensure no regressions:

npm run test:php -- --group sitemaps

Use of AI Tools

AI assistance: Yes
Model(s): Gemini 3.8 Flash High
Used for: Identifying uncovered branches in class-wp-sitemaps-renderer.php (specifically render_index(), render_sitemap(), empty dataset fallbacks, and supported XML tag mappings), drafting unit test assertions with proper error handling for CLI HTTP header constraints, and preparing this PR description. All generated test cases, assertions, PHPUnit execution (22 tests, 46 assertions), and WordPress Coding Standards compliance were thoroughly reviewed, executed, and validated by me in a local Docker environment.

@github-actions

github-actions Bot commented Sep 15, 2026

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 huzaifaalmesbah, lancewillett.

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

@lancewillett lancewillett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting one change before landing: preserve PHPUnit’s error handling in the rendering tests. The other three inline comments are optional improvements.

Local validation passed on PHP 8.3 / MySQL 9.7:

  • Renderer class: 22 tests / 46 assertions in single-site and multisite.
  • Full sitemaps group: 161 tests / 238 assertions in both modes.
  • Two additional random-order class runs and all 11 new methods run individually passed.
  • PHPCS passed.

The separate-process alternative was also validated locally. No production or test changes from the review probes remain.


Adversarial review · gpt-6

if ( str_contains( $errstr, 'Cannot modify header information' ) ) {
return true;
}
return false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requested change: please preserve PHPUnit’s error handling in these four rendering tests.

Returning false here invokes PHP’s built-in handler, not PHPUnit’s previous handler. An unrelated warning injected into render_index() failed as an output mismatch instead of being reported as a PHPUnit error.

Please remove these handler/try/finally blocks and use @runInSeparateProcess with @preserveGlobalState disabled on the retained rendering tests. The same pattern exists in Tests_Media_wpCrossOriginIsolation for calls that send headers.

I tried that replacement: all four rendering tests passed in both single-site and multisite, and the injected warning was correctly reported as a PHPUnit error.


Adversarial review · gpt-6

->getMock();

$renderer->method( 'get_sitemap_index_xml' )
->willReturn( '' );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional simplification, also applicable to test_render_sitemap_empty(): consider dropping these two mocked empty-output cases.

The real getters produce a root XML document even with no entries; their documented failure result is false. Returning '' here creates an artificial case with little additional protection. Removing the non-empty output guard also leaves these tests green because echoing '' emits nothing. That is equivalent behavior, not evidence of a missed functional regression.

The two real empty-input getter tests above are useful and should stay. If serialization failure is the intended scenario here, use false and describe that intent instead.


Adversarial review · gpt-6

$actual = $renderer->get_sitemap_xml( $url_list );

$this->assertStringContainsString( '<loc>http://' . WP_TESTS_DOMAIN . '/?foo=1&amp;bar=2</loc>', $actual );
$this->assertStringContainsString( '<lastmod>2020-01-01 &amp; "quotes"</lastmod>', $actual );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional improvement for both escaping tests: use the existing XML helper to verify the document is well formed, then assert the decoded element values.

These string fragments also constrain serialization choices. For example, literal quotes and &quot; produce the same element text; > and &gt; are equivalent in the priority fixture. The current assertions reject those valid alternatives.

Parsed-value assertions would retain protection against lost, malformed, or double-escaped content while allowing equivalent XML serialization. The special-character inputs are useful and can stay.


Adversarial review · gpt-6

$sitemap_renderer = new WP_Sitemaps_Renderer();
$stylesheet_url = $sitemap_renderer->get_sitemap_stylesheet_url();

$this->assertSame( $custom_url, $stylesheet_url );

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional simplification for both stylesheet filter tests: remove the direct getter call and assertion, and keep the generated stylesheet processing-instruction assertion below.

That output check already proves the custom filter result reaches the renderer. It preserves the useful integration coverage while removing a redundant second call to the getter.


Adversarial review · gpt-6

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.

2 participants