You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Taher Alkhateeb <sl...@gmail.com> on 2016/05/26 12:46:54 UTC
proposal to remove cobertura and sonar from ofbiz
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
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Jacopo Cappellato <ja...@hotwaxsystems.com>.
+1 to remove that code and if needed other "optional" features, proceed
with the cleanup/refactoring and then re-evaluate the introduction of them
on a case-by-case after the cleanup is done.
Not because I think that the code is ugly but only to simplify the
refactoring effort.
Jacopo
On Thu, May 26, 2016 at 2:46 PM, Taher Alkhateeb <slidingfilaments@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
>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Jacques Le Roux <ja...@les7arts.com>.
+1 to get ahead :)
Jacques
Le 27/05/2016 � 08:03, Taher Alkhateeb a �crit :
> Hi Jacques,
>
> I am interested first and foremost in cleaning the code base which is hard
> work and i don't want to be sidetracked by other tasks.
>
> The links below confirm my concerns that the code is problematic, and after
> your feedback as well as Jacopo's I am more inclined to remove the code
> than work _around_ it. I am just not sure what to do next or whether to
> request a vote or just go ahead and try to isolate the code away from the
> startup component.
>
> FYI I have a ready patch that can remove the whole thing, so my hesitation
> is only in terms ok getting the thumbs up. Suggestions?
>
> Taher Alkhateeb
> On May 27, 2016 8:33 AM, "Jacques Le Roux" <ja...@les7arts.com>
> wrote:
>
>> Hi Taher,
>>
>> While at it, FYI: I'd love to see this flight
>> https://issues.apache.org/jira/browse/INFRA-3590
>>
>> See also https://issues.apache.org/jira/browse/OFBIZ-4757
>>
>> Jacques
>>
>>
>> Le 26/05/2016 � 19:24, Taher Alkhateeb a �crit :
>>
>>> 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 <do...@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 <
>>>>>> slidingfilaments@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
>>>>>>>
>>>>>>>
>>>>>>>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Jacopo Cappellato <ja...@hotwaxsystems.com>.
Hi Taher,
I have added a comment in Jira, please have a look.
Thanks,
Jacopo
On Mon, May 30, 2016 at 5:31 PM, Taher Alkhateeb <slidingfilaments@gmail.com
> wrote:
> Hey Everyone,
>
>
>
> My patch has been sitting in OFBIZ-6783 for a few days and I don’t want it
> to get outdated as it touches important files including build.xml
>
>
>
> This patch shaves off 701 lines of code from OFBiz putting it on a good
> healthy diet. I’ve tested it and all is good from my side, I hesitate to
> commit before I get one or two feedbacks. Thank you in advance for your
> help.
>
>
>
> Taher Alkhateeb
>
>
>
> From: Taher Alkhateeb [mailto:slidingfilaments@gmail.com]
> Sent: 27 May 2016 11:49
> To: OFBIZ Development Mailing List
> Subject: Re: proposal to remove cobertura and sonar from ofbiz
>
>
>
> Hello Everyone,
>
> Thank you all for your votes and support. I will submit a patch in
> https://issues.apache.org/jira/browse/OFBIZ-6783 and refer to this thread
> accordingly.
>
> I really appreciate your support. Also thank you Adam for the tips, I did
> a little bit of research and it seems that yes cobertura is the best
> coverage tool to use with ofbiz and I am definitely interested in
> reintroducing it in the future.
>
> I won't commit anything to give time for everyone to test and/or
> contribute more to this thread.
>
> Cheers
>
> Taher Alkhateeb
>
>
>
> On Fri, May 27, 2016 at 10:00 AM, Sharan-F <sharan.foga@gmail.com <mailto:
> sharan.foga@gmail.com> > wrote:
>
> +1 to go ahead too.
>
> If we have had these issues hanging around for years without being resolved
> then I think a clean start sounds the best.
>
> Thanks
> Sharan
>
>
>
> --
> View this message in context:
> http://ofbiz.135035.n4.nabble.com/proposal-to-remove-cobertura-and-sonar-from-ofbiz-tp4681662p4681716.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>
>
>
>
RE: proposal to remove cobertura and sonar from ofbiz
Posted by Taher Alkhateeb <sl...@gmail.com>.
Hey Everyone,
My patch has been sitting in OFBIZ-6783 for a few days and I don’t want it to get outdated as it touches important files including build.xml
This patch shaves off 701 lines of code from OFBiz putting it on a good healthy diet. I’ve tested it and all is good from my side, I hesitate to commit before I get one or two feedbacks. Thank you in advance for your help.
Taher Alkhateeb
From: Taher Alkhateeb [mailto:slidingfilaments@gmail.com]
Sent: 27 May 2016 11:49
To: OFBIZ Development Mailing List
Subject: Re: proposal to remove cobertura and sonar from ofbiz
Hello Everyone,
Thank you all for your votes and support. I will submit a patch in https://issues.apache.org/jira/browse/OFBIZ-6783 and refer to this thread accordingly.
I really appreciate your support. Also thank you Adam for the tips, I did a little bit of research and it seems that yes cobertura is the best coverage tool to use with ofbiz and I am definitely interested in reintroducing it in the future.
I won't commit anything to give time for everyone to test and/or contribute more to this thread.
Cheers
Taher Alkhateeb
On Fri, May 27, 2016 at 10:00 AM, Sharan-F <sharan.foga@gmail.com <ma...@gmail.com> > wrote:
+1 to go ahead too.
If we have had these issues hanging around for years without being resolved
then I think a clean start sounds the best.
Thanks
Sharan
--
View this message in context: http://ofbiz.135035.n4.nabble.com/proposal-to-remove-cobertura-and-sonar-from-ofbiz-tp4681662p4681716.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Taher Alkhateeb <sl...@gmail.com>.
Hello Everyone,
Thank you all for your votes and support. I will submit a patch in
https://issues.apache.org/jira/browse/OFBIZ-6783 and refer to this thread
accordingly.
I really appreciate your support. Also thank you Adam for the tips, I did a
little bit of research and it seems that yes cobertura is the best coverage
tool to use with ofbiz and I am definitely interested in reintroducing it
in the future.
I won't commit anything to give time for everyone to test and/or contribute
more to this thread.
Cheers
Taher Alkhateeb
On Fri, May 27, 2016 at 10:00 AM, Sharan-F <sh...@gmail.com> wrote:
> +1 to go ahead too.
>
> If we have had these issues hanging around for years without being resolved
> then I think a clean start sounds the best.
>
> Thanks
> Sharan
>
>
>
> --
> View this message in context:
> http://ofbiz.135035.n4.nabble.com/proposal-to-remove-cobertura-and-sonar-from-ofbiz-tp4681662p4681716.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.
>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Sharan-F <sh...@gmail.com>.
+1 to go ahead too.
If we have had these issues hanging around for years without being resolved
then I think a clean start sounds the best.
Thanks
Sharan
--
View this message in context: http://ofbiz.135035.n4.nabble.com/proposal-to-remove-cobertura-and-sonar-from-ofbiz-tp4681662p4681716.html
Sent from the OFBiz - Dev mailing list archive at Nabble.com.
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Taher Alkhateeb <sl...@gmail.com>.
Hi Jacques,
I am interested first and foremost in cleaning the code base which is hard
work and i don't want to be sidetracked by other tasks.
The links below confirm my concerns that the code is problematic, and after
your feedback as well as Jacopo's I am more inclined to remove the code
than work _around_ it. I am just not sure what to do next or whether to
request a vote or just go ahead and try to isolate the code away from the
startup component.
FYI I have a ready patch that can remove the whole thing, so my hesitation
is only in terms ok getting the thumbs up. Suggestions?
Taher Alkhateeb
On May 27, 2016 8:33 AM, "Jacques Le Roux" <ja...@les7arts.com>
wrote:
> Hi Taher,
>
> While at it, FYI: I'd love to see this flight
> https://issues.apache.org/jira/browse/INFRA-3590
>
> See also https://issues.apache.org/jira/browse/OFBIZ-4757
>
> Jacques
>
>
> Le 26/05/2016 à 19:24, Taher Alkhateeb a écrit :
>
>> 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 <do...@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 <
>>>>> slidingfilaments@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
>>>>>>
>>>>>>
>>>>>>
>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Taher,
While at it, FYI: I'd love to see this flight https://issues.apache.org/jira/browse/INFRA-3590
See also https://issues.apache.org/jira/browse/OFBIZ-4757
Jacques
Le 26/05/2016 � 19:24, Taher Alkhateeb a �crit :
> 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 <do...@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 <
>>>> slidingfilaments@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
>>>>>
>>>>>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Taher Alkhateeb <sl...@gmail.com>.
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 <do...@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 <
>>> slidingfilaments@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
>>>>
>>>>
>>
>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Adam Heath <do...@brainfood.com>.
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
>> <slidingfilaments@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
>>>
>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Adam Heath <do...@brainfood.com>.
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 <slidingfilaments@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
>>
Re: proposal to remove cobertura and sonar from ofbiz
Posted by Pierre Smits <pi...@gmail.com>.
+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 <slidingfilaments@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
>