@mnapoli opened this Issue on October 30th 2014 Member

From #6544: Add code coverage badge + Travis CI badge + Code quality badge in each plugin README (core+PRO plugins)

@mnapoli commented on November 4th 2014 Member

@mattab

I'm having a lot of trouble with the current .travis.yml and its generator. Furthermore I don't think Scrutinizer is a good idea after all for plugins (i.e. what we started discussing last time):

  • plugins don't have unit tests, at best a few of them have have integration tests but those are not in code coverage, per #6556
  • Scrutinizer analyzer can't do much because plugins don't use Composer, so all calls to Piwik core can't be analyzed. That also means that detecting calls to <a class='mention' href='https://github.com/deprecated'>@deprecated</a> can't be done that way.

So Scrutinizer don't provide much value in this case. If we were to set it up today on all plugins, all code coverages will be 0% (either because there are no unit tests or because there are no tests/passing tests). And all quality indicators will be very high, i.e. not useful.

So I'm suggesting we keep Scrutinizer for Piwik Core and components (as it is now). And for plugins we don't use Scrutinizer (at least until the situation improves). There is currently coveralls configuration but it seems that it's not even used! So I'd say let's drop the idea of code coverage because it doesn't work and it's useless right now.

Regarding #6544 I'd say let's start by having tests and making them pass. And let's try to have more unit tests too and clearly separate them from integration/system/ui tests (like it is in Core). Writing unit tests today is not simple, but hopefully it will become easier on the long run with DI.

@sgiehl commented on November 4th 2014 Member

There are some Plugins running code coverage using coveralls. See
http://coveralls.io/r/piwik

@mnapoli commented on November 4th 2014 Member

OK then something's wrong, I couldn't make it work. At first glance it seems that coveralls has no report in the last 14 days (https://coveralls.io/r/piwik/plugin-LoginLdap) but there are commits since then.

@mattab commented on November 4th 2014 Owner

Let's not drop code coverage. it's very useful and needed because we try to establish an engineering culture of Core plugins and PRO plugins bundled with tests. I see code coverage as a rough imperfect-yet-useful indicator of the quality of tests.

Because we need to have tests for all plugins we should maintain, and because we don't know how good tests are without investigating them in detail, then code coverage metric has a real value for a quick assessment when reviewing new plugin.

@mnapoli commented on November 5th 2014 Member

I've added the Travis badges everywhere at least. Let's keep Coveralls then.

@sgiehl commented on November 5th 2014 Member

Code coverage did not work because of an error in the .travis.yml template. I've fixed it. Seems to work again for LoginLdap plugin

@mattab commented on November 28th 2014 Owner

Sitaution: we have code coverage with Scrutinizer on Core piwik but not on plugins. For now we don't need code coverage on plugins as it's quite useless metrics because we have too few unit tests.

@mattab commented on January 6th 2015 Owner

followed up in #6931 Generate Unit tests code coverage report for Plugins

This Issue was closed on November 28th 2014
Powered by GitHub Issue Mirror