Skip to content

Build/Test Tools: Use strict assertions in the interactivity-api tests. - #13528

Closed
haritpanchal wants to merge 1 commit into
WordPress:trunkfrom
haritpanchal:tests/interactivity-api-strict-assertions
Closed

haritpanchal wants to merge 1 commit into
WordPress:trunkfrom
haritpanchal:tests/interactivity-api-strict-assertions

Conversation

@haritpanchal

Copy link
Copy Markdown

What

Reviewed the 2 files claimed on the ticket under tests/phpunit/tests/interactivity-api/.
All 33 assertEquals() calls convert to assertSame().

  • wpInteractivityAPIDirectivesProcessor.php (32 assertions) — every call compared a
    string literal against $p, the WP_Interactivity_API_Directives_Processor
    instance itself, not a string. This only ever worked because PHPUnit falls
    back to __toString() when comparing a string to an object that defines
    one — assertSame() would fail on every single one of these (string !== object) if the method name were swapped without also fixing the argument.
    __toString() delegates to get_updated_html(): string
    (WP_HTML_Tag_Processor, which this class extends), the same strict
    return-type contract used in the block-bindings/ conversions from this
    ticket, so each call now explicitly asserts against
    $p->get_updated_html().
  • wpInteractivityAPI.php (1 assertion) — parse_directive_name() declares
    @phpstan-return array{..., suffix: non-empty-string|null, unique_id: non-empty-string|null} and casts every non-null branch with (string), so
    the type is a documented guarantee, not an inference. The two sibling
    assertions immediately above this one in the same test already use
    assertSame() for the same structure, so this was an inconsistency fix.

Testing

npm run test:php -- --group interactivity-api
381 tests, 999 assertions, 0 failures

node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpunit -c tests/phpunit/multisite.xml --group interactivity-api
381 tests, 999 assertions, 0 failures

PHPCS clean on both changed files. Test-only change; no production code touched.

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Helping locate candidate files and cross-check each expected literal
against the return-type contract. I reviewed the conversions and take
responsibility for the final code.

@github-actions

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 haritpanchal.

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

@lancewillett

Copy link
Copy Markdown
Member

wporg-sync pushed a commit that referenced this pull request Sep 15, 2026
Compare updated HTML strings explicitly with assertSame() in the directives processor tests. Use a strict comparison for the sorted directive suffixes and unique IDs, preserving the distinction between strings and null.

Developed in: #13528

Props haritpanchal.
See #64895.


git-svn-id: https://develop.svn.wordpress.org/trunk@63622 602fd350-edb4-49c9-b593-d223f7449a82
wporg-sync pushed a commit to WordPress/WordPress that referenced this pull request Sep 15, 2026
Compare updated HTML strings explicitly with assertSame() in the directives processor tests. Use a strict comparison for the sorted directive suffixes and unique IDs, preserving the distinction between strings and null.

Developed in: WordPress/wordpress-develop#13528

Props haritpanchal.
See #64895.

Built from https://develop.svn.wordpress.org/trunk@63622


git-svn-id: http://core.svn.wordpress.org/trunk@62798 1a063a9b-81f0-0310-95a4-ce76da25c4cd
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