Skip to content

Tests: Add unit test coverage for validate_active_plugins() - #14608

Open
huzaifaalmesbah wants to merge 3 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests/validate-active-plugins
Open

huzaifaalmesbah wants to merge 3 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests/validate-active-plugins

Conversation

@huzaifaalmesbah

Copy link
Copy Markdown
Member

Description

Adds tests for validate_active_plugins(), which deactivates active plugins that are missing or invalid when the Plugins screen loads, and for the validate_plugin() errors it reports. The existing tests only check a removed plugin being reported and the empty case.

The tests use a temporary plugin file without a header and the plugins in the test data directory. They cover:

  • an active_plugins option that is not an array (a string or an integer) being reset;
  • valid active plugins being kept;
  • the plugin_invalid, plugin_not_found and no_plugin_header errors with their messages, and the invalid plugins being deactivated silently while the valid one stays active;
  • on multisite, network-active plugins being validated and deactivated for a user who can manage network plugins, and left alone for other users.

Removing or changing each branch of the two functions makes at least one test fail on single site or multisite.

This is a test-only change; no production code is modified.

Testing Instructions

  1. Run the tests on single site:
    npm run test:php -- --filter Tests_Admin_Includes_Plugin_ValidateActivePlugins
  2. Run them on multisite:
    npm run test:php -- --filter Tests_Admin_Includes_Plugin_ValidateActivePlugins -c tests/phpunit/multisite.xml
  3. Run them in random order:
    npm run test:php -- --filter Tests_Admin_Includes_Plugin_ValidateActivePlugins --order-by=random
  4. Run the plugins group:
    npm run test:php -- --group plugins
  5. Run PHPCS on the test file:
    node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpcs tests/phpunit/tests/admin/includes/plugin/validateActivePlugins.php

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5.5
Used for: Brainstorming edge-case scenarios and drafting test boilerplate; test architecture, assertions, and local PHPUnit verification were written and validated by me.

@huzaifaalmesbah
huzaifaalmesbah marked this pull request as ready for review October 21, 2026 04:27
@github-actions

github-actions Bot commented Oct 21, 2026 •

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props huzaifaalmesbah, lancewillett, mukesh27.

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


Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

  • @propsbot add <username> to give props to a GitHub user. Requires write access.
  • @propsbot remove <username> to leave a user out of the props. Requires write access.
  • @propsbot refresh to refresh this list. Adding the props-bot label works too.

@mukeshpanchal27 mukeshpanchal27 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 assertions match the source, and CI passes. One note about fixture cleanup inline.

parent::set_up();

$dir = WP_PLUGIN_DIR . '/' . self::NO_HEADER_DIR;
$this->assertDirectoryDoesNotExist( $dir, 'The test directory should not exist yet.' );

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.

In core tests, WP_PLUGIN_DIR is tests/phpunit/data/plugins, which is a tracked directory. If a run stops partway, such as on a fatal error or Ctrl+C, tear_down() never runs and this fixture folder stays there. The folder name is fixed, so this assertion fails on the next run. So every test in the class fails here until someone deletes the folder by hand. The leftover also shows up in git status.

Could this helper, or set_up(), remove a leftover folder first? #14600 already does this: its remove_plugin() helper runs in both set_up() and tear_down().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks! Added a remove_plugin() helper that runs in both set_up() and tear_down(), in 7d99d5d. I checked it with a leftover folder in place: the old code failed and the new code passes.

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.

🟢 Approval recommended

No active review issues were identified in the test-only expansion of plugin-validation coverage.

No open issues 🎉

What changed in this PR

Adds unit tests for validate_active_plugins() and the validate_plugin() errors it reports, expanding coverage without changing production code.

Changes:

  • Covers malformed active-plugin options and preservation of valid plugins.
  • Verifies invalid-plugin error codes, messages, and silent deactivation.
  • Tests permission-dependent validation of network-active plugins on multisite.
File Description
tests/​phpunit/​tests/​admin/​includes/​plugin/​validateActivePlugins.php Adds plugin-validation tests with temporary fixture setup and cleanup.

Review effort: Balanced

@lancewillett

Copy link
Copy Markdown
Member

Pushed f8932ec to create the headerless plugin fixture only in the test that uses it. The existing network-permission test now uses a logged-in site administrator without super-admin privileges, so it distinguishes network permission from merely being logged in. All methods and provider cases remain.

Two randomized seeds passed on single-site (four tests, 12 assertions) and multisite (six tests, 16 assertions); PHPCS passed. Temporarily replacing the network permission gate with manage_options or a logged-in check passed the original negative test and failed the revised test.

AI review · gpt-6

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.

🟢 Approval recommended

No active review issues are reported for this test-only expansion of WordPress plugin-validation coverage.

No open issues 🎉

Review effort: Balanced

@lancewillett lancewillett 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 fixtures exercise validation and the authenticated non-super-admin boundary without leaving unrelated filesystem setup in every case.

Reviewed the final tested revision. Randomized single-site/multisite validation, PHPCS, and current CI passed.

AI review · gpt-6

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.

4 participants