Skip to content

Tests: Add unit test coverage for editing plugin files with wp_edit_theme_plugin_file() - #14600

Open
huzaifaalmesbah wants to merge 3 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests/edit-plugin-file
Open

huzaifaalmesbah wants to merge 3 commits into
WordPress:trunkfrom
huzaifaalmesbah:tests/edit-plugin-file

Conversation

@huzaifaalmesbah

@huzaifaalmesbah huzaifaalmesbah commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Description

Adds tests for editing plugin files with wp_edit_theme_plugin_file(), the function behind the plugin file editor. Its only existing test checks the loopback request for a network-active plugin; the request validation and saving had no tests. Theme edits are left for a separate PR.

The tests create a temporary plugin with a PHP file, a text file and an image, and answer the loopback requests with a scrape result chosen per test. They cover:

  • the missing_file, bad_file, missing_content, missing_nonce and missing_theme_or_plugin errors, without changing the file;
  • unauthorized with its message for a user who cannot edit plugins;
  • nonce_failure, invalid_plugin, and bad_plugin_file_path for a file of another plugin;
  • file_does_not_exist for a plugin whose file is missing on disk;
  • illegal_file_type for an image, and for a text file once the editable_extensions filter, which receives the plugin, removes txt;
  • a text file and the main PHP file of an inactive plugin being saved without a loopback check, and a text file of an active plugin too;
  • the PHP file of an active plugin being saved when the editor and home page loopback checks both pass;
  • the edit being reverted with a php_error when the editor or home page request fails, reports an error, or returns no JSON, with the matching message and error data, and the scrape key being deleted.

Removing or changing each branch of the plugin path makes at least one test fail.

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_wpEditThemePluginFilePlugin
  2. Run them on multisite:
    npm run test:php -- --filter Tests_Admin_wpEditThemePluginFilePlugin -c tests/phpunit/multisite.xml
  3. Run them in random order:
    npm run test:php -- --filter Tests_Admin_wpEditThemePluginFilePlugin --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/wpEditThemePluginFilePlugin.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

Copy link
Copy Markdown
Member Author

Fixed in 6a07640: the first commit accidentally included a changed tests/phpunit/data/plugins/hello.php fixture, left behind by a local check that wrote to it. It is now restored to trunk, so the PR only adds the new test file, and the plugins group passes again on single site and multisite.

@huzaifaalmesbah
huzaifaalmesbah marked this pull request as ready for review October 21, 2026 03:11
@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. 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 huzaifaalmesbah, mukesh27.

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

@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 tests match wp_edit_theme_plugin_file(), and CI passes. set_up() removing a leftover fixture folder is a good pattern. One coverage suggestion inline.

* @param string $url The request URL.
* @return false|array The response.
*/
public function record_loopback( $response, $args, $url ) {

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 class blocks loopback requests but never tests the loopback path. That path runs when the PHP file of an active plugin is edited. Core sends a loopback request to check for fatal errors, and if the check fails, it writes the previous content back to the file. This is the riskiest branch of the function, and it has no coverage yet.

Since record_loopback() already returns a WP_Error, a test could:

  1. Activate the plugin and edit its main PHP file.
  2. Assert a loopback_request_failed error.
  3. Assert that the file still has its original content.
  4. Assert that one loopback request was made.

A second test could return a body with the wp_scraping_result_start and wp_scraping_result_end markers around true, for both requests, and assert that the edit is saved.

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.

Thanks for this, done in 819ec04. One detail: the function returns a php_error, with loopback_request_failed in the error data, so the tests assert those. There is now a test for an edit that is saved when both checks pass, and a data-provided test for an edit that is reverted when the editor request fails, reports an error or returns no JSON, or when the home page request fails or reports an error. Each case checks the message, the error data, the restored content, the request paths, and that the scrape key is deleted.

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