Skip to content

SSO: Reuse the test install's admin in the authentication tests - #965

Closed
obenland wants to merge 1 commit into
WordPress:trunkfrom
obenland:fix/sso-tests-existing-admin
Closed

obenland wants to merge 1 commit into
WordPress:trunkfrom
obenland:fix/sso-tests-existing-admin

Conversation

@obenland

@obenland obenland commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

The WordPress.org SSO unit tests fail on trunk (example run):

WPOrg_SSO_Authentication_Test::test_admin_refusal_cannot_be_overridden
WPOrg_SSO_Authentication_Test::test_admin_cannot_authenticate
WP_UnitTest_Factory_Exception: Unable to create the user: Sorry, that username already exists!

Both tests create an admin account, but WordPress's test install already has one. Core's user factory used to return that error quietly, and the tests passed with the existing admin. Since r63925 (#66111), factories throw on failure. The suite runs against core master, so it broke without a change here.

make_account() now gives an existing account the tests' known password instead of creating it again. The suite passes locally against core master (231 tests).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Test setup now handles cases where the requested test account already exists, allowing authentication tests to proceed with the expected test credentials. No end-user-facing behavior changes.

The tests that turn `admin` away created that account, although WordPress's test install already has it. Core's user factory used to return the error quietly, which left the existing `admin` in place; since [63925] it throws on failure, so both tests error. `make_account()` now gives an existing account the known password instead.

See https://core.trac.wordpress.org/ticket/66111.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings September 30, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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

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

@coderabbitai

coderabbitai Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a784c963-992d-48d8-b03c-dfe7bfac4351

📥 Commits

Reviewing files that changed from the base of the PR and between c8edecf and 5bebd78.

📒 Files selected for processing (1)
  • common/includes/wporg-sso/phpunit/tests/WPOrg_SSO_Authentication_Test.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The SSO authentication test helper now reuses an account with the requested login, sets its password to the test password, and returns it. If no matching account exists, the helper uses the existing creation path.

Changes

SSO test account setup

Layer / File(s) Summary
Reuse existing test accounts
common/includes/wporg-sso/phpunit/tests/WPOrg_SSO_Authentication_Test.php
make_account() now returns a matching account after setting its password to PASSWORD. If no account matches, it continues through the existing creation path. The docblock describes this behavior.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 5bebd

The helper reuses the existing test account and resets its password; the tests roll back database changes. No material merge-blocking regression is established.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reusing the existing test install administrator in SSO authentication tests.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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