Not sure if this is the right way to fix this problem but can't think of any other solution right now. With more and more API changes we might have this problem more often in the future and hiding output can't be the solution. It probably makes it harder to debug errors in the future in case a column is actually missing and you only see an empty line in the diff? Best practice is usually to compare all values (apart from date/subtable which is random).

On the other side I am thinking we sometimes actually want this build to fail as there is a different behaviour with Piwik versions and some plugins might actually need to be sure this column exist whereas for some it is not relevant.

Maybe a stupid idea that adds unnecessary overhead and is not really practical, haven't found so much about it: In Android you can define something like a targetSdkVersion. If we'd define such a version in plugins - optionally maybe - could we compare all API output for Piwik versions having this version and for all other Piwik versions we remove the zeros? Eg targetPiwikVersion >= 2.10. More practical would be maybe to do this only on Travis when running tests for minimum_required_piwik.

I think best solution in general is to not use SystemTests but IntegrationTests / UnitTests instead. We should always keep this in mind and try not to add any SystemTests in the future until needed so not comparing the whole output doesn't matter as we can be sure there are IntegrationTests

Best practice is usually to compare all values (apart from date/subtable which is random).

Problem w/ this is that currently system tests don't compare values, they compare rendered output. Comparing values wouldn't be a problem because in that case it would do 0 == false. I suggested not rendering output in system tests just comparing datatables, but @mattab seems opposed to that...

to prevent this type of system build failures, besides rewriting the tests to integration/unit as Thomas suggested, maybe in some cases it would work to use showColumns and only keep a few known columns (preventing future columns from failing the test?)

(we couldn't merge this PR, as removing 0 values may hide real bugs in the future...).

Using showColumns in the ApiGetWithSitesInfo test would be counter-intuitive since you won't be testing the whole API result. But it would be a simpler hack than this pull request.

It would not always be counter-intuitive. Eg when adding a system test for bandwidth one is not necessarily interested in fields for content tracking, browser, etc. Ideally, one would only write Integration tests instead of system tests and it would be solved automatically. Unfortunately, we won't be able to rewrite all the existing system tests soonish (but I think it would be worth to "forbid" writing any further system tests unless there is a really good reason for it which would be very rare).

As we probably need a solution for this now... Can we maybe compare all values for the latest Piwik version (unless a lower max_required_piwik in plugin.json is defined then we'd compare all values for that max Piwik version) and for all lower/other versions we would not compare all values? Would that work? It would probably only work for plugin tests that run on Travis otherwise we don't know which plugin.json to read

I mean it would be counter-intuitive for ApiGetWithSitesInfo since the whole point of the plugin is to add columns to a single report. We could hide metrics that are 0, but then that might mask bugs where those columns are incorrectly removed. An unlikely bug, though.

True for ApiGetWithSitesInfo it might be counter-intuitive... to keep things easy maybe we could compare all values when running against master and do not compare those zero values when running against a fixed version eg when running against minimum_required_piwik?

The build's passing right now so I think it's no longer necessary (maybe due to a new release or something). Anyway, I'm going to lose this pull request.