Skip to content

test: cover the admin bar, active-posts widget and Users list edge paths - #813

Merged
josephfusco merged 12 commits into
WordPress:mainfrom
noruzzamans:test/admin-surfaces-edge-paths
Oct 13, 2026
Merged

josephfusco merged 12 commits into
WordPress:mainfrom
noruzzamans:test/admin-surfaces-edge-paths

Conversation

@noruzzamans

@noruzzamans noruzzamans commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

WP_Test_Presence_Admin_Bar's class-level @covers list narrows coverage for every test without its own tag, so wp_presence_refresh_screen_token() and wp_presence_get_entry_screen() were already exercised here without being counted.

Relates to #550.

Use of AI Tools

AI assistance: Yes
Tool(s): Google Gemini
Model(s): Gemini 3.8 Flash
Used for: Identifying uncovered branches and drafting the test cases; reviewed and verified locally by me.

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.55%. Comparing base (f6a926b) to head (5c79e8f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #813      +/-   ##
============================================
+ Coverage     97.12%   97.55%   +0.43%     
+ Complexity      271      269       -2     
============================================
  Files            30       30              
  Lines          4375     4372       -3     
============================================
+ Hits           4249     4265      +16     
+ Misses          126      107      -19     
Flag Coverage Δ
multisite 97.55% <ø> (+0.43%) ⬆️
phpunit 77.58% <ø> (+0.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@noruzzamans
noruzzamans marked this pull request as ready for review October 9, 2026 06:23
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

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 noruzzaman, huzaifaalmesbah, iamchitti, rejaulalomkhan, joefusco.

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

@huzaifaalmesbah huzaifaalmesbah 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.

The front-end test and the subscriber markup test read well, and the class-level @covers reasoning checks out. A few of the added rows raise coverage without guarding anything, and some cases sit in tests named for something else; details inline.

AI review: Claude Opus 5.5

Comment thread plugins/presence-api/tests/test-admin-bar.php Outdated
Comment thread plugins/presence-api/tests/widgets/test-widget-active-posts.php Outdated
Comment thread plugins/presence-api/tests/widgets/test-widget-active-posts.php Outdated
Comment thread plugins/presence-api/tests/widgets/test-widget-active-posts.php Outdated
Comment thread plugins/presence-api/tests/test-user-list.php
@josephfusco josephfusco added [Type] Enhancement A suggestion for improvement of an existing feature [Area] Admin UI Issues for the admin UI integrations [Area] Widgets Issues for the dashboard widgets php Pull requests that update php code labels Oct 9, 2026
@i-am-chitti

Copy link
Copy Markdown
Collaborator

@noruzzamans This PR isn't linked to an issue. Could you link one first?

@rejaulalomkhan

Copy link
Copy Markdown
Contributor

Thanks for adding these test edge paths @noruzzamans.

A few recommendations following up on the maintainers' review:

  1. Issue linking: As @i-am-chitti noted, please link this PR to its tracking issue (e.g., 📌 Tracking: Presence API path to Core #550) or open a focused edge-path coverage issue so it can be tracked.
  2. Assertion fidelity:
    • In \ est-admin-bar.php: Ensure added unparseable room rows have explicit assertions testing that the unparseable room is skipped while valid ones render.
    • In \ est-widget-active-posts.php: When exercising \post_type_supports( ..., 'presence' ), register a real dummy post type that explicitly omits 'presence'\ support rather than relying on attachment post access rules.
    • In \ est-user-list.php: Split fragment request checks into dedicated test methods reflecting the scenario under test rather than adding them to existing unrelated tests.

@noruzzamans

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review and recommendations, @huzaifaalmesbah, @i-am-chitti, and @rejaulalomkhan.

I've addressed the feedback in 3ad7344:

  • Linked the PR to tracking issue 📌 Tracking: Presence API path to Core #550.
  • Extracted unparseable room handling into test_unparseable_post_room_is_skipped_and_first_post_room_wins() with explicit assertions that the unparseable room is skipped and the first post room wins.
  • Registered a dummy post type (no_presence_support) to genuinely exercise the post_type_supports() check in test_active_posts_excludes_posts_without_presence_support().
  • Separated deleted user skips and unauthorized subscriber heartbeat handling into dedicated test methods.
  • Extracted empty fragment request checks in test-user-list.php into their own test method.

All linters and PHPStan Level 8 checks pass cleanly.

@josephfusco josephfusco left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the rework! I removed each guard in turn to see which tests fail, and most are covered now. Three tests still need a change:

  1. test_unparsable_post_room_is_skipped_and_first_post_room_wins
    It never reaches the ! $parsed check, because the valid row comes first and the bad one is skipped as a duplicate. Putting the unparsable row on a second user with no other post room fixes that.

  2. test_heartbeat_received_returns_empty_without_fragment_request
    It passes with both fragment checks removed, because nobody is signed in and the capability check returns first. Signing in a user who can list users fixes that.

  3. test_heartbeat_received_ignores_unauthorised_user
    It catches nothing test_register_skips_a_user_who_cannot_edit_posts doesn't already, so it can go.

@github-actions github-actions Bot added the Needs WP.org Link Author hasn't linked a WordPress.org account yet — confirm before merging so props credit correctly label Oct 13, 2026
@noruzzamans

Copy link
Copy Markdown
Contributor Author

Thanks @josephfusco, addressed all three in 99ba54c:

  • Placed the unparsable postType/bad row on a second user with no other post room so ! $parsed is reached directly.
  • Signed in an administrator with list_users capability in test_heartbeat_received_returns_empty_without_fragment_request.
  • Removed test_heartbeat_received_ignores_unauthorised_user.

@noruzzamans

Copy link
Copy Markdown
Contributor Author

Tightened in 5c79e8f: isolated the unparsable room to a dedicated user so it directly guards the ! $parsed check, with all CI checks passing.

@josephfusco josephfusco left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the changes! I removed each guard again and the unparsable room and online count tests now fail without them. The users-list assertion in test_heartbeat_received_returns_empty_without_fragment_request still passes with its fragment check removed, because the nonce check returns first, so that line and its @covers can go. That was my suggestion that didn't work out, not yours. Happy to merge either way.

@josephfusco
josephfusco merged commit 4cdedde into WordPress:main Oct 13, 2026
25 checks passed
@github-actions github-actions Bot added this to the 0.18.1 milestone Oct 13, 2026
@josephfusco josephfusco added props-bot Manually runs the Props Bot to update the contributor list. and removed Needs WP.org Link Author hasn't linked a WordPress.org account yet — confirm before merging so props credit correctly labels Oct 13, 2026
@github-actions github-actions Bot removed the props-bot Manually runs the Props Bot to update the contributor list. label Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Area] Admin UI Issues for the admin UI integrations [Area] Widgets Issues for the dashboard widgets php Pull requests that update php code [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants