Add feature tests for checking plugin with addon enabled

[ernilambar]

• Add feature tests with addon enabled (which includes new category and extra checks under that category)

[ernilambar]

Note: Behat tests are passing in my local setup with following specs.

PHP: 8.2.19
Node: 20.11.1

[ernilambar]

Problem I am facing is classes on main plugin are not loading in the addon. Since WordPress not necessarily loads plugin files in our required order, how can I make sure that all classes of main plugin are available in the addon?

CC @swissspidy

[github-actions]

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ernilambar <rabmalin@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: davidperezgar <davidperez@git.wordpress.org>

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

[ernilambar]

May be we should also add instructions in our docs on how to create addon for Plugin Check.

[swissspidy]

Yeah simply defining the classes like that would be error prone.

First, you'll want to use the Requires Plugins header.

Second, I would recommending moving the classes to separate files and then load them only when needed and when you know the parent classes exist. For example like so:

add_filter(
        'wp_plugin_check_checks',
        function ( array $checks ) {
          require_once './class-prohibited-text-check.php';
          require_once './class-postsperpage-check.php';

          return array_merge(
            $checks,
            array(
              'prohibited_text' => new Prohibited_Text_Check(),
              'postsperpage'    => new PostsPerPage_Check(),
            )
          );
        }
      );

[swissspidy]

May be we should also add instructions in our docs on how to create addon for Plugin Check.

Yeah @davidperezgar brought that up too. First we can start small, explaining the filters and giving a short example. We can reuse some of the code you're writing here for that.

Not really urgent though, as I doubt there is huge demand for writing those 😄

[davidperezgar]

Yes! That would be very helpful.

[ernilambar]

When I plugin check command is run, may be there needs to cleaned up. Because when same command run twice, in the second time external checks are not detected. For example, in line 463 and 477 I have kept same command but output is not as expected.

[swissspidy]

That sounds odd 🤔 Sounds like some debugging is in order

[ernilambar]

@swissspidy I tried this to reproduce this in separate setup. After first command run, those checks are not included until I delete file object-cache.php.

[swissspidy]

So it's an issue with Plugin Check / the object-cache.php drop-in and not with the test?

Want me to take a look at it to see if I can find the issue, or what's the next step?

[ernilambar]

Yes, it does not seem to be related to Behat test. I tried to debug but could not find the exact issue. May be you can give a shot to find issue quicker as you are more familiar with the architecture 😊

[swissspidy]

Well, I'm not that familiar with that part 😅

The gist is thatobject-cache.php is used as a "hook" to short-circuit regular WordPress loading order and manually execute the PCP CLI command before WordPress has fully loaded. That also means before any plugins have actually loaded...

From what I can see, this hack is used so we can perform runtime checks for a plugin without the plugin having to be active on the site. But not entirely sure.

When I activate pcp-addon and foo-plugin and remove all of this object-cache.php stuff, the runtime checks and the custom addon checks work just fine. See #533 for where I attempted this.

Related: #153

[ernilambar]

I removed before_wp_load hook and all tests are passing. Did we miss to add test for CLI_Runner or it is not needed now?

[swissspidy]

It's still needed and used by Plugin_Check_Command. Now it's just instantiated later and not during the object-cache setup. See also #533.

[davidperezgar]

Behat tests are not passing.

[ernilambar]

Behat tests are not passing.

Yes. See #518 (comment) above and related conversation. That needs to be fixed first.

[ernilambar]

I have created separate feature file to pass the feature test. For blocker, I have created separate issue for tracking. #555

[swissspidy]

But separating the tests like this now gives the false impression that it works, when in reality it actually doesn't (well, unless you delete the drop-in between runs).

The object-cache drop-in simply prevents add-ons from easily integrating with PCP, due to the change in execution/loading order. That blocks this PR.

[davidperezgar]

So, is it ok?

[swissspidy]

No, IMHO this PR is currently blocked because the feature it is testing is not actually working as expected. It is blocked by #555