@tsteur opened this issue on October 28th 2014

Plugins should only use methods that are declared in any plugins/*/API.php or any method or property tagged with @api.

We should check all PHP files within a plugin for any illicit method, constant or property usage and output any that is not public. If anything is used that is @deprecated a warning should be shown. If a plugin uses any method from another plugin (API.php) we should check whether the dependency is defined in require section of plugin.json.

This can be useful - to see if we should mark more methods, constants, properties as API - to run automated tests - when a plugin is published on the Marketplace to see if it uses any no longer existing methods, any deprecated method etc and under circumstances trigger an error and not publish the new version. See also #6150

https://github.com/nikic/PHP-Parser/ could be used for this which we already use in documentation generator.

@mattab commented on October 28th 2014

:+1: this will be so useful!

Maybe you already mean this by "Automated test" but could we add an automated test that is executed in each plugin's build and that fails the plugin's build whenever the plugin uses a function that does is not tagged with @api ?

@tsteur commented on October 28th 2014

that's meant

@mattab commented on February 19th 2015

Good news: work in progress in the phpstorm plugin: https://github.com/piwik/phpstorm-plugin-piwikstorm

Next steps is to run this in CI somehow. @diosmosis please write update here when you have one :+1:

@diosmosis commented on February 26th 2015

Status: Need to get phpstorm running on travis. To do that, we need to register phpstorm on travis using our license file. Which is not straightforward. Can either use the license file or server (according to jetbrains), but not sure how.

@mattab commented on February 26th 2015

did you try to ask Jetbrains support to explain us the exact steps to get this going? I've emailed them few times before and they were helpful, but maybe that's more complex...

@diosmosis commented on March 13th 2015

Emailed jetbrains, waiting for a reply.

@mattab commented on March 17th 2015

There is a possibility that what we want to do is not even possible... hopefully jetbrains support will be helpful and give us the solution, otherwise the work done so far may be "useless" (at least at automating the detection of deprecated code, we could still do it manually from time to time...)

@diosmosis commented on March 23rd 2015

Status: managed to get phpstorm license recognized in travis. Currently the process is killed before finishing.

@mattab commented on March 23rd 2015

Note that this code inspection ideally should run for: - Open source plugins at Piwik and Piwik PRO - Premium plugins at Piwik PRO - (optional: Third party plugins on the marketplace)

Maybe it will be necessary to add a new CI job to each build to run this code inspect?

@diosmosis commented on March 23rd 2015

I think that would require a new CI job. Though it would be good if it didn't run on EVERY commit, just maybe master/develop branches. Or maybe for these repos we can setup our own server and add a github check (like travis/scrutinizer does) so travis doesn't get clogged.

@diosmosis commented on March 24th 2015

@mattab It looks like running the inspections can exhaust the memory, can we only run one build (not job, at least not yet) at a time on piwik-tests-plugins?

@mnapoli commented on March 24th 2015

Aren't all Travis builds (even jobs) run into a separate virtual machine?

@diosmosis commented on March 24th 2015

You're right, I read the docs wrong. Not sure what to do though, I don't think it's possible to increase the memory.

@diosmosis commented on March 24th 2015

Status: inspections are created and uploaded as artifacts, ie, http://builds-artifacts.piwik.org/protected/LoginLdap.code_quality_tests/408.1/inspections/

TODO: - [x] print summary of most important inspections out in travis console output - [x] fix random failures (reduce max memory used by JVM?) (haven't seen a new one yet, crossing off for now) - [x] fix bugs in new inspection (eg, Config::getInstance() results in failure because Singleton marks it as @api, not Config) - [x] speed up code inspection build - [x] make marketplace use new latest commit to base new builds off of (pull request created) - [x] setup repo for core + pro plugins - [x] setup weekly cron to test all maintained plugins for non-api/deprecated

@mnapoli commented on March 24th 2015

Here it says the max memory builds can use is 3Gb if it helps http://docs.travis-ci.com/user/common-build-problems/#My-build-script-is-killed-without-any-error

@diosmosis commented on March 24th 2015

That's what I read ;)

@mattab commented on March 25th 2015

What are the most little amount of work left to have it already useful in production? we need to deliver small useful MVP and put it in CI already if possible and then we can improve the tool further in next sprint, as i'm afraid we don't have much more time for this sprint.

(for example for MVP I think we don't need the UI/JS part simply we could read Travis log output to see which file+line calls which deprecated / non-api function)

@diosmosis commented on March 29th 2015

Weekly report of maintained plugins' use of non-@api and @deprecated symbols is now in place & pull request for using it in piwik-tests-plugins via marketplace exists. Closing.

Follow up issue here: https://github.com/piwik/piwik/issues/7569

@mnapoli commented on March 29th 2015

@diosmosis that's interesting :) where will those reports be? E.g. do you have a link to an example?

@diosmosis commented on March 29th 2015

If you look in the protected builds artifacts server you'll see folders like inspections.all or inspections.pluginname. Check those out (inspections.all is the most readable)

@mnapoli commented on March 29th 2015

On http://builds-artifacts.piwik.org/? I can't find those folders in there (or in subfolders) :(

@diosmosis commented on March 29th 2015

Builds-artifacts.org/protected/. Doesn't get listed in directory ;)

@mattab commented on March 29th 2015

Weekly report of maintained plugins' use of non-@api and @deprecated symbols is now in place & pull request for using it in piwik-tests-plugins via marketplace exists. Closing.

Nice progress! :+1:

This issue was closed on March 29th 2015
Powered by GitHub Issue Mirror