You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by Babak Vahdat <ba...@swissonline.ch> on 2012/02/14 18:38:36 UTC
About false-positive Test inside Camel source!
Hi
Today there was a question on the user forum [1] where I applied a patch
regarding it, however my concern by this post is not the issue user suffered
from but the unit-test [2] which does behave false-positive!
If at the line 147 you would change:
assertEquals(TEXT, result);
With:
assertEquals(null, result);
The test would *still* pass!
I looked into it a bit and IMHO the problem is because of
RedeliveryErrorHandler.isDone() [3] which somehow has the effect of:
- x - = + <== minus multiplied with minus == plus
That's if answer value assigned through:
boolean answer = isCancelledOrInterrupted(exchange);
Is *already* true we should return !answer because as the method name says
we ask for if isDone(). I changed the logic of that method on my workspace
to something making better sense to me and the result was an endless
retrying of routing the failed inflight exchange again and again.
In all cases I assume we've got an issue here, What do you think?
[1]
http://camel.465427.n5.nabble.com/Zipping-the-content-of-several-files-into-one-file-td5481638.html
[2]
https://svn.apache.org/repos/asf/camel/trunk/camel-core/src/test/java/org/apache/camel/impl/ZipDataFormatTest.java
[3]
https://svn.apache.org/repos/asf/camel/trunk/camel-core/src/main/java/org/apache/camel/processor/RedeliveryErrorHandler.java
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5483171.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: About false-positive Test inside Camel source!
Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi Christian,
Both
junit.framework.AssertionFailedError
org.junit.ComparisonFailure
extend java.lang.AssertionError
Now inside Processors by [1] we do some assertEquals() which by it's failure
would throw org.junit.ComparisonFailure *however* we used to do:
onException(junit.framework.AssertionFailedError)...
which was *not* triggered *at all* as JUnit did throw
org.junit.ComparisonFailure so that
errors.expectedMessageCount(0);
did pass as junit.framework.AssertionFailedError has *no* direct type
relationship with org.junit.ComparisonFailure. On the other hand we make use
of the jms consumer by this test so that the thrown AssertionErrors was
happening inside the AsyncMessageListenerInvoker threads of spring DMLC
(DefaultMessageListenerContainer). If for example we had a usage of the
direct consumer then we had realized the failed asserts as we had consumed
inside the *main* thread.
Just to see the failed assertion by this test get the previous version of it
on your workspace and set a breakpoint on the line *116* and you should see
how the assertions did fail *but* the test *passes*.
In general it's a sharp corner of using JUnit not being able to report
failed asserts not happening inside main thread. There're many places on the
web [2] talking about this.
[1]
https://svn.apache.org/repos/asf/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsHeaderFilteringTest.java
[2]
http://andrejserafim.wordpress.com/2010/10/07/testing-threads-with-junit/
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5535127.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: About false-positive Test inside Camel source!
Posted by Christian Müller <ch...@gmail.com>.
Ok, thanks for the information.
Best,
Christian
On Mon, Mar 5, 2012 at 5:03 PM, Babak Vahdat <ba...@swissonline.ch>wrote:
> Hi Christian,
>
> I did check all the Camel tests regarding this issue:
>
> onException(some.subtype.of.java.lang.AssertionError)...
>
> And I couldn't find any one more other than [1] which I did already fix
> [2].
> As an example [3] does it the correct way, that's:
>
> onException(AssertionError.class)
>
> [1]
>
> http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsHeaderFilteringTest.java?revision=1151890&view=markup
> [2]
>
> http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsHeaderFilteringTest.java?revision=1295573&view=markup
> [3]
>
> https://svn.apache.org/repos/asf/camel/trunk/tests/camel-itest/src/test/java/org/apache/camel/itest/jms/JMSTransactionIsTransactedRedeliveredTest.java
>
> Babak
>
>
> --
> View this message in context:
> http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5537986.html
> Sent from the Camel Development mailing list archive at Nabble.com.
>
Re: About false-positive Test inside Camel source!
Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi Christian,
I did check all the Camel tests regarding this issue:
onException(some.subtype.of.java.lang.AssertionError)...
And I couldn't find any one more other than [1] which I did already fix [2].
As an example [3] does it the correct way, that's:
onException(AssertionError.class)
[1]
http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsHeaderFilteringTest.java?revision=1151890&view=markup
[2]
http://svn.apache.org/viewvc/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsHeaderFilteringTest.java?revision=1295573&view=markup
[3]
https://svn.apache.org/repos/asf/camel/trunk/tests/camel-itest/src/test/java/org/apache/camel/itest/jms/JMSTransactionIsTransactedRedeliveredTest.java
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5537986.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: About false-positive Test inside Camel source!
Posted by Christian Müller <ch...@gmail.com>.
I "fixed" all countDownLatch.await(timeout, timeUnit) calls and check the
return value as assertTrue(countDownLatch.await(timeout, timeUnit)). All
tests still pass at this time.
I didn't got the point that the onException() definition was not triggered,
when we throw an AssertionFailureError or ComparisonFailure inside a
Processor. In the unit test I improved a few day ago the problem was, that
we didn't checked whether the exchange was done successfully or not. Is
this what you mean?
Best,
Christian
On Thu, Mar 1, 2012 at 3:14 PM, Babak Vahdat <ba...@swissonline.ch>wrote:
> Hi
>
> It seems there're still other false-positive unit tests as well one of
> which
> I fixed
> right before [1] as I saw the JMS Header discussion @ user forum.
>
> IMHO in *almost* all cases we should assertTrue() on ALL
> CountDownLatch.await() calls
> to return true (the timeout should not elapse). And also using the
> onException() clause
> NEVER and EVER rely on:
>
> junit.framework.AssertionFailedError, org.junit.ComparisonFailure or the
> likes, BUT only
> on:
>
> java.lang.AssertionError
>
> In the case of the test mentioned we had:
>
> onException(AssertionFailedError.class)
>
> However the thrown exception through JUnit was org.junit.ComparisonFailure
> while we did
> assertEquals() inside the processor so that the onException() clause did
> *not* trigger at all!
>
> I hope I'm *definitely* wrong but somehow I've got the feeling that
> there're
> much more such
> unit-tests out there in Camel source itself!
>
> As the test behaved false positive we didn't realize it by CAMEL-1871. BTW,
> maybe it's
> just a matter of my personal taste but I HATE false-positive tests :-(((
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=1295573
>
> Babak
>
> --
> View this message in context:
> http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5527960.html
> Sent from the Camel Development mailing list archive at Nabble.com.
>
Re: About false-positive Test inside Camel source!
Posted by Christian Müller <ch...@gmail.com>.
I have a look on the latch.await(...) calls and changed them. Running a
build at the moment to see if we really have some wrong-positiv tests...
Best,
Christian
On Thu, Mar 1, 2012 at 3:14 PM, Babak Vahdat <ba...@swissonline.ch>wrote:
> Hi
>
> It seems there're still other false-positive unit tests as well one of
> which
> I fixed
> right before [1] as I saw the JMS Header discussion @ user forum.
>
> IMHO in *almost* all cases we should assertTrue() on ALL
> CountDownLatch.await() calls
> to return true (the timeout should not elapse). And also using the
> onException() clause
> NEVER and EVER rely on:
>
> junit.framework.AssertionFailedError, org.junit.ComparisonFailure or the
> likes, BUT only
> on:
>
> java.lang.AssertionError
>
> In the case of the test mentioned we had:
>
> onException(AssertionFailedError.class)
>
> However the thrown exception through JUnit was org.junit.ComparisonFailure
> while we did
> assertEquals() inside the processor so that the onException() clause did
> *not* trigger at all!
>
> I hope I'm *definitely* wrong but somehow I've got the feeling that
> there're
> much more such
> unit-tests out there in Camel source itself!
>
> As the test behaved false positive we didn't realize it by CAMEL-1871. BTW,
> maybe it's
> just a matter of my personal taste but I HATE false-positive tests :-(((
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=1295573
>
> Babak
>
> --
> View this message in context:
> http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5527960.html
> Sent from the Camel Development mailing list archive at Nabble.com.
>
Re: About false-positive Test inside Camel source!
Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi
It seems there're still other false-positive unit tests as well one of which
I fixed
right before [1] as I saw the JMS Header discussion @ user forum.
IMHO in *almost* all cases we should assertTrue() on ALL
CountDownLatch.await() calls
to return true (the timeout should not elapse). And also using the
onException() clause
NEVER and EVER rely on:
junit.framework.AssertionFailedError, org.junit.ComparisonFailure or the
likes, BUT only
on:
java.lang.AssertionError
In the case of the test mentioned we had:
onException(AssertionFailedError.class)
However the thrown exception through JUnit was org.junit.ComparisonFailure
while we did
assertEquals() inside the processor so that the onException() clause did
*not* trigger at all!
I hope I'm *definitely* wrong but somehow I've got the feeling that there're
much more such
unit-tests out there in Camel source itself!
As the test behaved false positive we didn't realize it by CAMEL-1871. BTW,
maybe it's
just a matter of my personal taste but I HATE false-positive tests :-(((
[1] http://svn.apache.org/viewvc?view=revision&revision=1295573
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5527960.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: About false-positive Test inside Camel source!
Posted by Babak Vahdat <ba...@swissonline.ch>.
Thanks Christian!
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5510214.html
Sent from the Camel Development mailing list archive at Nabble.com.
Re: About false-positive Test inside Camel source!
Posted by Christian Müller <ch...@gmail.com>.
I got it.
template.sendBodyAndProperty(endpoint, body, propertyKey, propertyValue)
calls ExchangeHelper.extractResultBody(result, pattern) after processing
the exchange to check whether the exchange failed or not what
template.send(endpoint, processor) didn't call.
Best,
Christian
On Sun, Feb 19, 2012 at 2:06 PM, Christian Müller <
christian.mueller@gmail.com> wrote:
> Hello Babak!
>
> I improved this unit test with r1290995. Now the test fails, if you expect
> "null" instead of "TEXT".
>
> But I'm wondering why template.sendBodyAndProperty(endpoint, body,
> propertyKey, propertyValue) has a different behavior than
> template.send(endpoint, processor)...
>
> Best,
> Christian
>
>
> On Thu, Feb 16, 2012 at 11:55 AM, Babak Vahdat <
> babak.vahdat@swissonline.ch> wrote:
>
>> Hi
>>
>> The last couple of days the user forum has been really active so maybe why
>> no body saw this thread...
>>
>> I think we've got an issue here!
>>
>> Babak
>>
>> --
>> View this message in context:
>> http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5489169.html
>> Sent from the Camel Development mailing list archive at Nabble.com.
>>
>
>
Re: About false-positive Test inside Camel source!
Posted by Christian Müller <ch...@gmail.com>.
Hello Babak!
I improved this unit test with r1290995. Now the test fails, if you expect
"null" instead of "TEXT".
But I'm wondering why template.sendBodyAndProperty(endpoint, body,
propertyKey, propertyValue) has a different behavior than
template.send(endpoint, processor)...
Best,
Christian
On Thu, Feb 16, 2012 at 11:55 AM, Babak Vahdat
<ba...@swissonline.ch>wrote:
> Hi
>
> The last couple of days the user forum has been really active so maybe why
> no body saw this thread...
>
> I think we've got an issue here!
>
> Babak
>
> --
> View this message in context:
> http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5489169.html
> Sent from the Camel Development mailing list archive at Nabble.com.
>
Re: About false-positive Test inside Camel source!
Posted by Babak Vahdat <ba...@swissonline.ch>.
Hi
The last couple of days the user forum has been really active so maybe why
no body saw this thread...
I think we've got an issue here!
Babak
--
View this message in context: http://camel.465427.n5.nabble.com/About-false-positive-Test-inside-Camel-source-tp5483171p5489169.html
Sent from the Camel Development mailing list archive at Nabble.com.