kieker-monitoring / kieker

Kieker's main repository
Apache License 2.0
70 stars 41 forks source link

[KIEKER-806] JMSReader - Missing tests #1116

Open rju opened 3 weeks ago

rju commented 3 weeks ago

JIRA Issue: KIEKER-806 JMSReader - Missing tests Original Reporter: Nils Christian Ehmke


I just recognized that the run method of the JMSReader seems to return false in any case. At the top of the method:

boolean retVal = false;

But the only write-access is

retVal = false;

And the only return call is

return retVal;

One would assume that the reader would return true if everything worked correctly.

Checklist:

rju commented 2 weeks ago

author André van Hoorn -- Tue, 5 Feb 2013 09:17:46 +0100

Looks strange, indeed. I expect we didn't see it so far, because with JMS, the analysis is terminated by CTRL+C and then a bunch of messages occurs.

According the JMXReader, I changed the initial assignment to true:

   public boolean read() {
        boolean retVal = true;

(see changeset:b3330ab4e378ae6cac403f20ebf734581971745a/kieker-git)

However, I didn't execute a test --- we only have manual ones for the JMS reader, so far ( KIEKER-57 Done ).

Close this ticket, after tested

Calling read() on Reader '...' (...)  returned false.
rju commented 2 weeks ago

author André van Hoorn -- Tue, 5 Feb 2013 09:33:55 +0100

I messed up s.th. Will fix it.

rju commented 2 weeks ago

author André van Hoorn -- Tue, 5 Feb 2013 09:38:46 +0100

Replying to [avh|comment:2]:
> I messed up s.th. Will fix it.

Fixed in changeset:45310fb59d871cd7a04a9f33b9bd9a1886b9ed76/kieker-git

rju commented 2 weeks ago

author André van Hoorn -- Tue, 5 Feb 2013 09:39:16 +0100

Remaining:

> However, I didn't execute a test --- we only have manual ones for the JMS reader, so far ( KIEKER-57 Done ).
>
> Close this ticket, after tested
>
> * Expected outcome: The following output must not occur:
>
> {{

{ > Calling read() on Reader '...' (...) returned false. > }

}}
> * (Maybe previously set to false in order to make sure that it did occur before)

rju commented 2 weeks ago

author nils-christian -- Mon, 4 Nov 2013 10:42:45 +0100

Currently we have the BasicJMSWriterReaderTest as a JUnit test. Do we need another test for the "read has to return true"-thing?

rju commented 2 weeks ago

author nils-christian -- Tue, 19 Nov 2013 12:15:09 +0100

Added missing test in changeset 1848a898d278db11e786db902e8ae222811cac64.