Hi Adam
Ok your objection is a good enough "no" for me to back off. I will try to
think of a way to work on the integration as you suggested.
I am not picking specifically on your work. However the entire startup
logic has many problems and lengthy messy code scattered all over the
place. I am trying as much as I can to "cut out" code until some one says
no like your good self.
I will fix the build.xml to ensure correct behavior in cobertura.
Taher Alkhateeb
On Thursday, 26 May 2016, Adam Heath <doo...@brainfood.com> wrote:
> Let me restate, do not remove the code coverage tool; fix the integration.
>
> Every single time I used code coverage to design more tests, I *always*
> found bugs. Real bugs. And, I also found unreachable code. I'll give an
> example:
>
> ==
> public void printMap(Map value) {
> if (value == null) {
> return;
> }
> String foo = safeToString(value);
> System.err.println(foo);
> }
>
> private String safeToString(Object value) {
> if (value == null) {
> return null;
> }
> return value.toString();
> }
> ==
>
> Granted, the above unreachable code in safeToString *code* be discovered
> with deep study, but an *automated* tool makes it much easier to find.
> And, the above example is a *very* simple example. Please take a look at
> the test cases for UtilCache.
>
> On 05/26/2016 11:26 AM, Adam Heath wrote:
>
>> Not to Pierre, but ugly and broken? How so? Please expand with concrete
>> issues.
>>
>> ps: I'm the original integrator of cobertura into ofbiz.
>>
>> pps: I have a local branch that converted ofbiz to maven, and actually
>> produced a runnable output. Should I revive that?
>>
>> On 05/26/2016 07:54 AM, Pierre Smits wrote:
>>
>>> +1 as it never got off the ground properly. We can always revisit later
>>> when desire to do so rises again.
>>>
>>> I use Sonar, but that is another subject.
>>>
>>> Best regards,
>>>
>>> Pierre Smits
>>>
>>> ORRTIZ.COM <http://www.orrtiz.com>
>>> OFBiz based solutions & services
>>>
>>> OFBiz Extensions Marketplace
>>> http://oem.ofbizci.net/oci-2/
>>>
>>> On Thu, May 26, 2016 at 2:46 PM, Taher Alkhateeb <
>>> slidingfilame...@gmail.com
>>>
>>>> wrote:
>>>> Hello everyone,
>>>>
>>>> As part of the refactoring process, I suggest to completely remove
>>>> cobertura and sonar from the framework. My proposal is based on the
>>>> following:
>>>>
>>>> - The startup logic is more complex because of the existence of legacy
>>>> classes (Instrumenter, InstrumenterWorker, etc ...).
>>>> - No one (AFAIK) is actively using cobertura or sonar, and the targets
>>>> in
>>>> build.xml are actually broken
>>>> - The way cobertura is integrated with ofbiz is poor and ugly
>>>> - Before integrating cobertura, ofbiz first needs a better testing
>>>> framework that allows for TDD and red-green-refactor. Otherwise, this
>>>> whole
>>>> issue with test coverage is a moot point
>>>> - Too much complexity and legacy code in build.xml, common.xml, ivy.xml,
>>>> macros.xml and others. It's just really ugly
>>>>
>>>> All the code that I saw for cobertura is just ugly and broken. Now it's
>>>> perfectly fine to reintroduce cobertura cleanly in the future, but I
>>>> would
>>>> not use the existing code anyway, I would just wipe it all out and start
>>>> fresh.
>>>>
>>>> I'm not sure whether we need to vote on this? Appreciate your feedback.
>>>>
>>>> Taher Alkhateeb
>>>>
>>>>
>>
>
