You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adam Heath <do...@brainfood.com> on 2010/02/15 18:22:39 UTC

Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

doogie@apache.org wrote:
> Author: doogie
> Date: Sun Feb 14 22:41:19 2010
> New Revision: 910110
> 
> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
> Log:
> Implement a creation framework; this allows creaters to provide
> optimized versions of converters.
> 
> Added:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
> Modified:
>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java

This is the commit that broke the build.

What happened here, is I had tested an earlier version of this change,
and everything worked(localized tests in base, and full system tests).
 I then changed the commit, and got excited about committing
everything, so didn't go thru a full test cycle.

Ideally, going forward, to help solve this, we need to have full
coverage of base; using cobertura, you can see what lines are being
run.  I am currently getting the conversion system to something close
to 100%.  If it had more coverage, this problem would have been
discovered with the simple tests.

Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by Adrian Crum <ad...@hlmksw.com>.
Adam Heath wrote:
> doogie@apache.org wrote:
>> Author: doogie
>> Date: Sun Feb 14 22:41:19 2010
>> New Revision: 910110
>>
>> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
>> Log:
>> Implement a creation framework; this allows creaters to provide
>> optimized versions of converters.
>>
>> Added:
>>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
>> Modified:
>>     ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java
> 
> This is the commit that broke the build.
> 
> What happened here, is I had tested an earlier version of this change,
> and everything worked(localized tests in base, and full system tests).
>  I then changed the commit, and got excited about committing
> everything, so didn't go thru a full test cycle.
> 
> Ideally, going forward, to help solve this, we need to have full
> coverage of base; using cobertura, you can see what lines are being
> run.  I am currently getting the conversion system to something close
> to 100%.  If it had more coverage, this problem would have been
> discovered with the simple tests.


I thought it was the misspelling of creators that broke the build. Thank 
you for the clarification.

Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by Adam Heath <do...@brainfood.com>.
>> What happened here, is I had tested an earlier version of this change,
>> and everything worked(localized tests in base, and full system tests).
>> I then changed the commit, and got excited about committing
>> everything, so didn't go thru a full test cycle.
>>
>> Ideally, going forward, to help solve this, we need to have full
>> coverage of base; using cobertura, you can see what lines are being
>> run.  I am currently getting the conversion system to something close
>> to 100%.  If it had more coverage, this problem would have been
>> discovered with the simple tests.
> 
> I think I'm missing something here... how does this relate to a failure to run tests?

I guess you didn't read the first paragraph I wrote.

It doesn't.  But in this particular case, if the original code *was*
tested, both the line, *and* all branch points, then the test cases
would have failed when I changed the code.  This is only applicable
for this particular bug that I introduced.

==
if (foo || bar || baz > 1) {
==

cobertura is smart enough to see that the line is covered, yes, but
that the bar and baz parts are not.

> Another small and insignificant detail to consider is that: 100% coverage does not mean 100% bug-free, unless perhaps you are a marketing professional.

Never said that 100% coverage means you've handled all cases.  But if
you do *not* have 100% coverage, you *know*, beyond a shadow of a
doubt, that there are things not being tested.

Re: svn commit: r910110 - in /ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion: ConverterCreater.java Converters.java

Posted by David E Jones <de...@me.com>.
On Feb 15, 2010, at 10:22 AM, Adam Heath wrote:

> doogie@apache.org wrote:
>> Author: doogie
>> Date: Sun Feb 14 22:41:19 2010
>> New Revision: 910110
>> 
>> URL: http://svn.apache.org/viewvc?rev=910110&view=rev
>> Log:
>> Implement a creation framework; this allows creaters to provide
>> optimized versions of converters.
>> 
>> Added:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/ConverterCreater.java
>> Modified:
>>    ofbiz/trunk/framework/base/src/org/ofbiz/base/conversion/Converters.java
> 
> This is the commit that broke the build.
> 
> What happened here, is I had tested an earlier version of this change,
> and everything worked(localized tests in base, and full system tests).
> I then changed the commit, and got excited about committing
> everything, so didn't go thru a full test cycle.
> 
> Ideally, going forward, to help solve this, we need to have full
> coverage of base; using cobertura, you can see what lines are being
> run.  I am currently getting the conversion system to something close
> to 100%.  If it had more coverage, this problem would have been
> discovered with the simple tests.

I think I'm missing something here... how does this relate to a failure to run tests?

Another small and insignificant detail to consider is that: 100% coverage does not mean 100% bug-free, unless perhaps you are a marketing professional.

-David