On 5/04/2010, at 11:38 PM, Adrian Crum wrote:
> --- On Mon, 4/5/10, Adam Heath <doo...@brainfood.com> wrote:
>> Bob Morley wrote:
>>>
>>> Scott Gray-2 wrote:
>>>> On 5/04/2010, at 10:54 PM, Adam Heath wrote:
>>>>
>>>>> Bob Morley wrote:
>>>>>> Thanks for that url; very interesting
>> indeed. From what I could tell
>>>>>> the
>>>>>> set of unit tests that execute are
>> littered with failures (or they
>>>>>> intentionally cause a lot stack traces due
>> to exception).
>>>>> I've been trying to reduce the number of
>> duplicated errors logged.
>>>>> OfBiz is famous of catching an error, logging
>> it, then rethrowing it.
>>>>> Repeat ad-infinitum.
>>>> OfBiz is famous of catching an error, logging it,
>> then rethrowing it.
>>>> Repeat ad-infinitum.
>>>>
>>>
>>> Ahh yes; I spotted one simple problem --
>>>
>>> [java] Unable to load test suite
>> class :
>>> org.ofbiz.order.test.CustRequestTest
>>> [java] Exception:
>> java.lang.ClassNotFoundException
>>> [java] Message:
>> org.ofbiz.order.test.CustRequestTest
>>>
>>> Looks like if we have a bad testdef configured, we do
>> not bubble this up and
>>> it gets lost. This should probably be failing
>> the build ...
>>>
>>> Most of the noise seems to come as you guys indicated,
>> from negative test
>>> cases coupled with the fact that we are logging
>> "info". Here is a pattern
>>> from the bottom of a tester --
>>>
>> assertNotNull("Foreign key referential
>> integrity is not observed for
>>> create (INSERT)", caught);
>>>
>> Debug.logInfo(caught.toString(), module);
>>>
>>> The second is explicit rollback and the end of a test
>> method which under the
>>> covers (if info is turned on) it will create an
>> exception and logError with
>>> it --
>>>
>> TransactionUtil.rollback(transBegin, null,
>> null);
>>>
>>>
>>> I will fix the first problem when I get into the
>> office tomorrow (as it is
>>> not really pressing). The second thing though,
>> wanna consider changing the
>>> log4j threshold on the run-tests target to warn?
>> This would cut down on the
>>> noise (but not eliminate).
>>
>> I just fixed the first.
>>
>> The others should have their logging fixed, don't change
>> the log level
>> for run-tests.
>>
>> If low-level code logs an error, and rethrows, then just
>> stop logging
>> the error.
>
> Another pattern that bugs me:
>
> try {
> ...
> } catch (Exception e) {
> Debug.logError(e, "Something went wrong: " + e.getMessage(), module);
> }
>
> The e.getMessage isn't necessary - the message will be printed in the stack
> trace.
>
> -Adrian
That's nothing, check out the 80-odd occurrences of this:
try {
...
} catch (Exception e) {
ServiceUtil.returnError(e.getMessage);
}
Not really logging related but wow, 80 occurrences?
smime.p7s
Description: S/MIME cryptographic signature
