Tests: Use assertIsNotArray() for array type assertions - #13305
Conversation
Replaces the generic `assertFalse( is_array() )` assertion in `Tests_Theme::test_get_theme()` with PHPUnit's dedicated `assertIsNotArray()`, which states the intent directly and produces a more descriptive failure message on failure.
Replaces the generic `assertFalse( is_array() )` assertion in `Tests_Theme::test_get_theme()` with PHPUnit's dedicated `assertIsNotArray()`, which states the intent directly and produces a more descriptive failure message on failure.
|
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. |
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. |
| $theme = get_theme( $name ); | ||
| // WP_Theme implements ArrayAccess. Even ArrayObject returns false for is_array(). | ||
| $this->assertFalse( is_array( $theme ) ); | ||
| // WP_Theme implements ArrayAccess, but that does not make it an array. |
There was a problem hiding this comment.
One suggestion by Claude Opus 5
The reason given for rewording is that it referred to an is_array() call that's no longer there — but assertIsNotArray() resolves to IsType(TYPE_ARRAY), which is is_array() underneath, so the reference wasn't really stale.
More usefully: the sentence that got dropped was the one carrying the information.
Even ArrayObject returns false for is_array().
That's the justification for having a comment there at all — SPL's own array-like class doesn't satisfy is_array() either, so the assertion is documenting real language behaviour rather than something self-evident. "but that does not make it an array" states the conclusion without the evidence.
Could we keep both?
// WP_Theme implements ArrayAccess, but that does not make it an array — even ArrayObject is not one.Small thing, and I'm happy either way if you'd rather keep it short.
2 participants
Tests_Theme::test_get_theme()wraps a native type check in a boolean assertion. PHPUnit has a dedicated assertion for this, which states the intent directly and reports a more useful message when it fails. The comment above it is reworded in the same change, as it referred to theis_array()call that is no longer there.This is the last remaining convertible occurrence in the test suite.
Follow-up to [62761], [63311].
Trac ticket: https://core.trac.wordpress.org/ticket/65819
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.