Update some tests to account for WP6.9 changes to wp_json_encode flags. #1293
Conversation
Specifically the addition of JSON_UNESCAPED_SLASHES as a flag. See: WordPress/wordpress-develop#9557
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughUpdates two unit tests in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.php📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
tests/phpunit/**/*.php📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Comment |
Summary of ChangesHello @pattonwebz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates existing PHPUnit tests to maintain their validity and functionality across different WordPress versions, particularly in light of changes introduced in WordPress 6.9. The primary goal is to prevent test failures that would arise from a modification in how Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly updates the tests to handle changes in wp_json_encode flags in WordPress 6.9. The approach is sound. I've provided a couple of suggestions to refactor the duplicated logic in the tests, which will improve code readability and maintainability by reducing repetition.
| if ( version_compare( get_bloginfo( 'version' ), '6.9', '>=' ) ) { | ||
| $this->assertStringContainsString( esc_url_raw( get_permalink( $post->ID ) ), $localized_data ); | ||
| } else { | ||
| $this->assertStringContainsString( str_replace( '/', '\\/', esc_url_raw( get_permalink( $post->ID ) ) ), $localized_data ); | ||
| } |
There was a problem hiding this comment.
To improve readability and reduce code repetition, you can determine the expected URL format first and then perform the assertion once. This avoids repeating the assertStringContainsString call and makes the test's intent clearer.
$expected_url = esc_url_raw( get_permalink( $post->ID ) );
if ( version_compare( get_bloginfo( 'version' ), '6.9', '<' ) ) {
$expected_url = str_replace( '/', '\\/', $expected_url );
}
$this->assertStringContainsString( $expected_url, $localized_data );| if ( version_compare( get_bloginfo( 'version' ), '6.9', '>=' ) ) { | ||
| $this->assertStringContainsString( esc_url_raw( get_permalink( $post->ID ) ), $localized_data ); | ||
| } else { | ||
| $this->assertStringContainsString( str_replace( '/', '\\/', esc_url_raw( get_permalink( $post->ID ) ) ), $localized_data ); | ||
| } |
There was a problem hiding this comment.
Similar to the previous test, this logic can be simplified to avoid code repetition and improve readability. By preparing the expected URL string before the assertion, you only need to call assertStringContainsString once.
$expected_url = esc_url_raw( get_permalink( $post->ID ) );
if ( version_compare( get_bloginfo( 'version' ), '6.9', '<' ) ) {
$expected_url = str_replace( '/', '\\/', $expected_url );
}
$this->assertStringContainsString( $expected_url, $localized_data );![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/phpunit/Admin/EnqueueAdminTest.php (2)
119-126: Version-aware assertion looks correct; consider minor DRY + WPCS cleanupsThe conditional assertion correctly handles both escaped and unescaped slash behavior across WP versions, so the test logic itself looks good.
Optionally, you could:
- Store the permalink in a local variable to avoid recomputing it twice.
- Tweak inline comments to follow WPCS style (capitalize “WordPress”/“WP”, start sentences with a capital letter).
For example:
- // In wp 6.9 there were changes to the flags passed to wp_json_encode() that made slashes no longer get escaped by default. - // we should check for both possibilities here to ensure compatibility across versions. - // see: https://github.com/WordPress/wordpress-develop/pull/9557 for more details. - if ( version_compare( get_bloginfo( 'version' ), '6.9', '>=' ) ) { - $this->assertStringContainsString( esc_url_raw( get_permalink( $post->ID ) ), $localized_data ); - } else { - $this->assertStringContainsString( str_replace( '/', '\\/', esc_url_raw( get_permalink( $post->ID ) ) ), $localized_data ); - } + // In WordPress 6.9, the flags passed to wp_json_encode() changed so slashes are no longer escaped by default. + // We should check for both possibilities here to ensure compatibility across versions. + // See: https://github.com/WordPress/wordpress-develop/pull/9557 for more details. + $permalink = esc_url_raw( get_permalink( $post->ID ) ); + if ( version_compare( get_bloginfo( 'version' ), '6.9', '>=' ) ) { + $this->assertStringContainsString( $permalink, $localized_data ); + } else { + $this->assertStringContainsString( str_replace( '/', '\\/', $permalink ), $localized_data ); + }As per coding guidelines, this keeps comments WPCS-compliant and the test body a bit cleaner.
138-158: Duplicate version-gated assertion block; consider extracting a small helperThis block mirrors the front page test logic and is also functionally correct for handling the wp_json_encode behavior change.
To reduce duplication and make future adjustments safer, you might extract the shared assertion into a private helper on the test class, e.g.:
private function assertPermalinkInLocalizedDataForCurrentWpVersion( int $post_id, string $localized_data ): void { $permalink = esc_url_raw( get_permalink( $post_id ) ); if ( version_compare( get_bloginfo( 'version' ), '6.9', '>=' ) ) { $this->assertStringContainsString( $permalink, $localized_data ); } else { $this->assertStringContainsString( str_replace( '/', '\\/', $permalink ), $localized_data ); } }Then both tests can just call that helper. While not required for correctness, it keeps the tests more maintainable and avoids two places to touch if WP behavior shifts again. As per coding guidelines, this also helps keep test code simple and consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
tests/phpunit/Admin/EnqueueAdminTest.php(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Follow WordPress Coding Standards (WPCS) in all PHP files
Class names use CamelCase (ClassNameConvention) for new classes
Use edac_ prefix for all custom action/filter hook names
Ensure PHP 7.4+ compatibility
Use type hints where appropriate (parameters, return types, properties)
Sanitize inputs, validate data, and escape outputs following WordPress security best practices; use nonces for forms/AJAX
Use the WordPress database API ($wpdb) for all database operations
Prefix functions and classes in the global namespace with edac_
Use WordPress transients for caching temporary data where appropriate
All user-facing text in PHP must be translatable using the accessibility-checker text domain
Use PHPDoc for all public classes, methods, and properties
Document all custom hooks (actions/filters) with docblocks including parameters and types
Files:
tests/phpunit/Admin/EnqueueAdminTest.php
tests/phpunit/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Place and write PHPUnit tests under /tests/phpunit
Files:
tests/phpunit/Admin/EnqueueAdminTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Integration Test: PHP 8.2 | WP latest (+ ms)
There was a problem hiding this comment.
Pull request overview
This PR updates unit tests to handle WordPress 6.9's changes to wp_json_encode() flags, specifically the addition of JSON_UNESCAPED_SLASHES which prevents forward slashes from being escaped by default.
- Adds version-specific assertions to accommodate both WP 6.9+ (unescaped slashes) and earlier versions (escaped slashes)
- Updates two test methods that verify localized script data containing URLs
| $this->assertStringContainsString( 'edac_pageScanner', $localized_data ); | ||
| $this->assertStringContainsString( str_replace( '/', '\\/', esc_url_raw( get_permalink( $post->ID ) ) ), $localized_data ); | ||
| // In wp 6.9 there were changes to the flags passed to wp_json_encode() that made slashes no longer get escaped by default. | ||
| // we should check for both possibilities here to ensure compatibility across versions. |
There was a problem hiding this comment.
Comment capitalization should be consistent with WordPress coding standards. The first word after the comment marker should be capitalized: "we" should be "We" at the start of the sentence.
| // we should check for both possibilities here to ensure compatibility across versions. | |
| // We should check for both possibilities here to ensure compatibility across versions. |
| $this->assertStringContainsString( str_replace( '/', '\\/', esc_url_raw( get_permalink( $post->ID ) ) ), $localized_data ); | ||
| // In wp 6.9 there were changes to the flags passed to wp_json_encode() that made slashes no longer get escaped by default. | ||
| // we should check for both possibilities here to ensure compatibility across versions. | ||
| // see: https://github.com/WordPress/wordpress-develop/pull/9557 for more details. |
There was a problem hiding this comment.
Comment capitalization should be consistent with WordPress coding standards. The first word after the comment marker should be capitalized: "see" should be "See" at the start of the sentence.
| // see: https://github.com/WordPress/wordpress-develop/pull/9557 for more details. | |
| // See: https://github.com/WordPress/wordpress-develop/pull/9557 for more details. |
|
@copilot you have a lot of things to say about what I put in my comment. Fix it for me in a new PR. |
|
@pattonwebz I've opened a new pull request, #1294, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Specifically the addition of JSON_UNESCAPED_SLASHES as a flag.
See: WordPress/wordpress-develop#9557
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.