Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

May plugin activation be required? #61

Closed
carstingaxion opened this issue Apr 1, 2024 · 9 comments
Closed

May plugin activation be required? #61

carstingaxion opened this issue Apr 1, 2024 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@carstingaxion
Copy link

carstingaxion commented Apr 1, 2024

Hello @swissspidy !

I suggested using this cool action for GatherPress, but we ran into a subtle problem during the first run, unfortunately.

While @mauteri suggested to call wp plugin activate gatherpress from within our workflow, I think it might be needed to this on the wp-plugin-check-action itself?

Probably somewhere around here, between 108 and 109:

https://github.com/swissspidy/wp-plugin-check-action/blob/8ee6b81574b78daa89c1487b5e3e2337b03dbf29/action.yml#L108-L109

May I be wrong? What is the expectated behaviour with plugins that make use of the activation hook AND create custom tables? Are such plugins - in theory - testable with the action?

@swissspidy
Copy link
Member

Heya, thanks for opening this!

No, plugin activation is not required.

Plugin Check performs two kinds of checks:

  • Static analysis — the code is tested by reading it instead of running it. These are mostly PHPCS tests.
  • Runtime analysis — the plugin is activated as part of this check, but on a separate WordPress install with a different database table prefix. This is automatically handled by Plugin Check.

So to answer your question:

What is the expectated behaviour with plugins that make use of the activation hook AND create custom tables? Are such plugins - in theory - testable with the action?

Yes, these plugins are testable. You should not need to do anything special to make it work.

I think what you might be running into is WordPress/plugin-check#234

If you manually install the Plugin Check plugin on a development site and run it against GatherPress, are you getting the same database errors?

I'll try to reproduce this as well on my end with just the plugin.

@swissspidy
Copy link
Member

Hmm when I'm running wp plugin check gatherpress --format=json --require=./wp-content/plugins/plugin-check/cli.php locally it works just fine.

In any case, the GitHub action is still working fine for you and there's no error, so not really urgent to look into it imho :)

@pedro-mendonca
Copy link

pedro-mendonca commented Apr 3, 2024

Hi, with the new WP 6.5 and dependencies I'm getting an error of a plugin that depends of another.

Warning: Failed to activate plugin. [Plugin Name] requires 1 plugin to be installed and activated: [plugin-dependency]. .
Error: No plugins activated.
Error: Process completed with exit code 1.

And with this, the test fails.
The current alternative is setting wp_version to 6.4, which doesn't rely on the dependencies and the plugin is correctly activated.

Maybe the action could check the plugin dependencies and activate them also, it's probably wp-cli territory as it's how the action installs the plugin to check.

@swissspidy
Copy link
Member

@pedro-mendonca Mind creating a new issue for that? It's really separate and I am basically about to close this one.

It's a great question though! Do you have a sample repo you could point me to?

I might add a plugin-dependencies input or so that allows you to specify the dependencies.

WP-CLI is a good point as well and WP-CLI will soon gain these capabilities. But in the meantime we'll have to sort this out ourselves. Even if that means writing some hacky code to find the dependencies.

@pedro-mendonca
Copy link

Sure, I'll create another issue, I'm sorry I thought it might be related :)

@swissspidy swissspidy added the question Further information is requested label Apr 4, 2024
@swissspidy swissspidy closed this as not planned Won't fix, can't repro, duplicate, stale Apr 4, 2024
@swissspidy swissspidy reopened this Apr 4, 2024
@swissspidy swissspidy added the enhancement New feature or request label Apr 4, 2024
@swissspidy
Copy link
Member

I have to circle back on this one:

Plugin activation is not required for static checks, but it is required for runtime checks.

So turns out I was not doing the latter here, which means the runtime checks were simply not running. But that's not a big deal, the rest is still working even without plugin activation.

@swissspidy
Copy link
Member

Fixed in #67

@carstingaxion
Copy link
Author

Hallo @swissspidy

I'm sorry you had to deal with all this on your own. I for sure wanted to come back to you earlier.

I haven't even looked into your changes yet. But great, you did. Will jump right into it now!

Thanks.

@carstingaxion
Copy link
Author

Cool. Happy to see your changes touched the lines 108ff. and I was not that wrong.

Thank you very much for your detailed explanation and the fast solution @swissspidy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants