Build/Test Tools: Use strict assertions in REST schema sanitization tests#12613
Conversation
…ests Replace the remaining assertEquals() calls with assertSame() in the REST schema sanitization tests. These assertions exist to verify that rest_sanitize_value_from_schema() coerces a value to the type declared in the schema, so the type of the returned value is the whole point of the test. A loose assertion passes even when the sanitizer returns the uncoerced value, which hides exactly the class of bug these tests are meant to catch. The file already used assertSame() for the majority of its assertions, so this makes it consistent throughout. Props thisismyurl. See #64895.
|
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 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. |
There was a problem hiding this comment.
Pull request overview
This pull request tightens the REST schema sanitization test suite by replacing remaining loose PHPUnit assertions with strict assertions, ensuring the tests actually verify that rest_sanitize_value_from_schema() returns values coerced to the schema-declared types (not just loosely equal values).
Changes:
- Replaced remaining
assertEquals()calls withassertSame()in REST schema sanitization tests. - Updated expected literals from integers to floats where the schema declares
'type' => 'number'(since sanitization returns(float)). - Kept specific values intentionally non-coerced (e.g., properties not declared in
propertiesand withadditionalPropertiesunset), making that behavior explicit under strict comparison.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Replaces the remaining
assertEquals()calls in the REST schema sanitization tests withassertSame().These assertions are a good fit for strict comparison, because the type of the returned value is the thing under test.
rest_sanitize_value_from_schema()promises to coerce a value to the type declared in the schema, and most of these cases pass a deliberately mistyped input to prove it does:With a loose assertion that test still passes if the sanitizer hands back the uncoerced
array( '1' ), so it cannot fail for the one reason it exists.Worth flagging for review, since it is the easy thing to get wrong here: every schema in these particular tests declares
'type' => 'number', which returns(float), so the expected values are floats rather than integers. That matches the convention already used bytest_type_number()at the top of this file, which assertsassertSame( 1.0, ... ).I checked each expected literal against its own schema rather than swapping the method names mechanically, which also means three values are deliberately left alone:
'd' => '1'in the nested-properties test stays a string, becausedhas no matching entry inproperties, nopatternProperties, andadditionalPropertiesis unset, so it is never coerced.'b' => 1in that same test stays an integer for the same reason.'b' => 1intest_type_object()likewise stays an integer.The strict assertions now document that behaviour instead of glossing over it.
Only assertion methods and expected literals changed. The file already used
assertSame()for 65 of its assertions, so this makes it consistent throughout.One disclosure on verification: this box has no Docker/
wp-env, so I could not run the full PHPUnit suite locally. I verified each conversion by reading the schema declared in each test and matching the expected literal to the typerest_sanitize_value_from_schema()returns for it, andphp -lis clean. I would appreciate a CI run confirming it.Trac ticket: https://core.trac.wordpress.org/ticket/64895
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Helping locate candidate files and cross-check each expected literal against the schema type. I reviewed the conversions and take responsibility for the final code.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.