Re: [xwiki-devs] [Brainstorming] Loss of TPC from 2017-11-09 to 2018-04-03

On 16 Apr 2018, at 16:00, Vincent Massol <vincent(a)massol.net> wrote: Hi, I’ve posted the following on matrix/IRC. Friday: * Guys we regressed in global TPC from 2017-11-09 to 2018-04-03 :( * From 76.28% to 75.88% * we need to analyze more in details now to understand why * (and whether the report is correct or not) * PDF Report: https://up1.xwikisas.com/#mJ0loeB6nBrAgYeKA7MGGw * Examples: ** com.xpn.xwiki.internal.store from 97.6% to 89.32% ** com.xpn.xwiki.internal.file from 67.9% to 9.87% - that's huge maybe some refactoring ** org.xwiki.url.internal from 86.99% to 63.69% Today: * Note that I have 2 reasons for checking our TPC: ** Globally it has to go up every year and we've gone down since nov 2017 ** For STAMP we need to show an increase in TPC and we had a nice +2.2% increase from 2016 to 2017 but we've gone down now and we need to go up, to go to +3 or/ 4% at end of 2018 Today I’ve also started analyzing the drop from 67.9% to 9.87% for “com.xpn.xwiki.internal.file”. Here are my findings: * This is caused by TemporaryDeferredFileRepository which is no longer tested ** Before: http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platfo… ** After; http://maven.xwiki.org/site/clover/20180403/clover-commons+rendering+platfo… * There have been refactoring recently to enable FS attachment store by default that have led to TemporaryDeferredFileRepository not being used anymore (deprecated methods not used anymore) or used only in some specific circumstances (for example at https://github.com/xwiki/xwiki-platform/blob/ac85d61b0c48d7ed21a8109e964f77… the store is not defined) * The conclusion is that before the changesTemporaryDeferredFileRepository was called a lot (indirectly) leading to 545 calls to it (http://maven.xwiki.org/site/clover/20171109/clover-commons+rendering+platfo…) and now it’s not called anymore at all. * The second conclusion is that we had no unit/integration tests for it and the changes lead to loosing it being tested. So it’s no longer tested ATM. Thanks to the Clover report we noticed that it’s no longer tested. * Thus we need to do one of 2 things: ** Move the code to legacy and make sure no code uses it. That’s if we don’t really need it ** Keep it but add some unit/integration tests or even some functional tests that would trigger it. I’ll let Thomas pitch in on this. Next step: analyze some other drops in TPC.

Note: I wish it were simpler to find out why some TPC drops. Took me like 1-2 hours to figure all this out... Thanks -Vincent