Re: svn commit: r478253 - in /incubator/qpid/trunk/qpid/java/client/src: main/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/test/unit/basic/ test/java/org/apache/qpid/test/unit/client/message

[Martin Ritchie <ri...@apache.org>]

I was seeing some strangeness when running from my IDE the the
connection would simply sleep because the broker had not been created.
I saw that again just now with the TransactedTest (who's setup method
had the wrong capitalisation, which is why it wasn't working with
junit3) but after I added the create broker in the setup before the
connection creation all was well. And oddly all is still well after I
have taken it out.

I think it is important that on the wiki page you document how to
create/destroy a broker between test cases. I believe we have (but
can't find them right) tests that leave the broker with stale data. We
don't want to have to require our tests to ensure that they leave the
broker 100% clean for the next test. Do we?

On 22/11/06, Steve Vinoski <vi...@iona.com> wrote:
> Yes, I agree, and will do so.
>
> --steve
>
> On Nov 22, 2006, at 1:05 PM, Carl Trieloff wrote:
>
> >
> > Steve,
> >
> > reading this, I think we should wiki it for anyone new to the project.
> >
> > Carl.
> >
> > Steve Vinoski wrote:
> >> I notice that the test just added below, and maybe other news ones
> >> too, is explicitly creating a VMBroker in its setUp(). I forgot to
> >> point out that in the refactored junit3 tests I recently
> >> committed, I added a VMBrokerSetup class to do this. It's an
> >> instance of junit TestSetup, which allows for once-per-test-class
> >> setup and teardown, as opposed to the class setup and teardown
> >> that occur for every test (i.e., TestSetup is like the junit4 @
> >> {Before,After}Class annotations).
> >>
> >> To use it, just wrap the normal return from suite() with it, like
> >> this:
> >>
> >>   return new VMBrokerSetup(new junit.framework.TestSuite
> >> (MapMessageTest.class));
> >>
> >> The MapMessageTest class below already does this, which means that
> >> its setUp() and tearDown() aren't really needed.
> >>
> >> Martin, I can fix this test and any other new ones if you like,
> >> given that I should have mentioned this earlier.
> >>
> >> --steve
> >>
> >> On Nov 22, 2006, at 12:46 PM, ritchiem@apache.org wrote:
> >>
> >>> client/src/test/java/org/apache/qpid/test/unit/basic/
> >>> MapMessageTest.java
> >>
> >
>
>


-- 
Martin Ritchie

Re: svn commit: r478253 - in /incubator/qpid/trunk/qpid/java/client/src: main/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/test/unit/basic/ test/java/org/apache/qpid/test/unit/client/message

[Martin Ritchie <ri...@apache.org>]

On 22/11/06, Steve Vinoski <vi...@iona.com> wrote:
> On Nov 22, 2006, at 6:02 PM, Martin Ritchie wrote:
>
> > I was seeing some strangeness when running from my IDE the the
> > connection would simply sleep because the broker had not been created.
> > I saw that again just now with the TransactedTest (who's setup method
> > had the wrong capitalisation, which is why it wasn't working with
> > junit3) but after I added the create broker in the setup before the
> > connection creation all was well. And oddly all is still well after I
> > have taken it out.
>
> Ah yes, there were a bunch of tests whose setUp() methods had a
> lowercase 'u' in them. I fixed a bunch, but obviously missed this one.
>
> > I think it is important that on the wiki page you document how to
> > create/destroy a broker between test cases. I believe we have (but
> > can't find them right) tests that leave the broker with stale data. We
> > don't want to have to require our tests to ensure that they leave the
> > broker 100% clean for the next test. Do we?
>
> Well, there's setUp() and tearDown() for creating a new broker for
> every testcase, and VMBrokerSetup for setting up a broker once for
> all of a class's testcases. I agree we don't want to be trying to go
> any more coarse-grained than that, and trying to share brokers across
> unit tests, due to the cleanup issues.
>
> BTW, the client basic ReceiveTest looks like it could use the
> VMBrokerSetup, but it's a got a main() in it that makes it look like
> somebody might be using it for other purposes as well, so I left it
> alone. If that's not the case, let me know, and I'll fix it too.

I think a lot of the tests were originally written as applications and
then had junit functionality added later. You'll not a lot of the
"tests" don't use asserts they just throw exceptions if something went
wrong.

It would be good to fix up as many of the test classes as we sensibly
can to be in the unit test directory.. with the goal of eventually
sorting all the test classes to the unit directory or the system test.
Then we can restore the more normal test package hierarchy.

Must be time for me to go... realised my comment on QPID-123 was total
rubbish... that was the python tests not the java ones!

> --steve
>
> >
> > On 22/11/06, Steve Vinoski <vi...@iona.com> wrote:
> >> Yes, I agree, and will do so.
> >>
> >> --steve
> >>
> >> On Nov 22, 2006, at 1:05 PM, Carl Trieloff wrote:
> >>
> >> >
> >> > Steve,
> >> >
> >> > reading this, I think we should wiki it for anyone new to the
> >> project.
> >> >
> >> > Carl.
> >> >
> >> > Steve Vinoski wrote:
> >> >> I notice that the test just added below, and maybe other news ones
> >> >> too, is explicitly creating a VMBroker in its setUp(). I forgot to
> >> >> point out that in the refactored junit3 tests I recently
> >> >> committed, I added a VMBrokerSetup class to do this. It's an
> >> >> instance of junit TestSetup, which allows for once-per-test-class
> >> >> setup and teardown, as opposed to the class setup and teardown
> >> >> that occur for every test (i.e., TestSetup is like the junit4 @
> >> >> {Before,After}Class annotations).
> >> >>
> >> >> To use it, just wrap the normal return from suite() with it, like
> >> >> this:
> >> >>
> >> >>   return new VMBrokerSetup(new junit.framework.TestSuite
> >> >> (MapMessageTest.class));
> >> >>
> >> >> The MapMessageTest class below already does this, which means that
> >> >> its setUp() and tearDown() aren't really needed.
> >> >>
> >> >> Martin, I can fix this test and any other new ones if you like,
> >> >> given that I should have mentioned this earlier.
> >> >>
> >> >> --steve
> >> >>
> >> >> On Nov 22, 2006, at 12:46 PM, ritchiem@apache.org wrote:
> >> >>
> >> >>> client/src/test/java/org/apache/qpid/test/unit/basic/
> >> >>> MapMessageTest.java
> >> >>
> >> >
> >>
> >>
> >
> >
> > --
> > Martin Ritchie
>
>


-- 
Martin Ritchie

Re: svn commit: r478253 - in /incubator/qpid/trunk/qpid/java/client/src: main/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/test/unit/basic/ test/java/org/apache/qpid/test/unit/client/message

[Steve Vinoski <vi...@iona.com>]

On Nov 22, 2006, at 6:02 PM, Martin Ritchie wrote:

> I was seeing some strangeness when running from my IDE the the
> connection would simply sleep because the broker had not been created.
> I saw that again just now with the TransactedTest (who's setup method
> had the wrong capitalisation, which is why it wasn't working with
> junit3) but after I added the create broker in the setup before the
> connection creation all was well. And oddly all is still well after I
> have taken it out.

Ah yes, there were a bunch of tests whose setUp() methods had a  
lowercase 'u' in them. I fixed a bunch, but obviously missed this one.

> I think it is important that on the wiki page you document how to
> create/destroy a broker between test cases. I believe we have (but
> can't find them right) tests that leave the broker with stale data. We
> don't want to have to require our tests to ensure that they leave the
> broker 100% clean for the next test. Do we?

Well, there's setUp() and tearDown() for creating a new broker for  
every testcase, and VMBrokerSetup for setting up a broker once for  
all of a class's testcases. I agree we don't want to be trying to go  
any more coarse-grained than that, and trying to share brokers across  
unit tests, due to the cleanup issues.

BTW, the client basic ReceiveTest looks like it could use the  
VMBrokerSetup, but it's a got a main() in it that makes it look like  
somebody might be using it for other purposes as well, so I left it  
alone. If that's not the case, let me know, and I'll fix it too.

--steve

>
> On 22/11/06, Steve Vinoski <vi...@iona.com> wrote:
>> Yes, I agree, and will do so.
>>
>> --steve
>>
>> On Nov 22, 2006, at 1:05 PM, Carl Trieloff wrote:
>>
>> >
>> > Steve,
>> >
>> > reading this, I think we should wiki it for anyone new to the  
>> project.
>> >
>> > Carl.
>> >
>> > Steve Vinoski wrote:
>> >> I notice that the test just added below, and maybe other news ones
>> >> too, is explicitly creating a VMBroker in its setUp(). I forgot to
>> >> point out that in the refactored junit3 tests I recently
>> >> committed, I added a VMBrokerSetup class to do this. It's an
>> >> instance of junit TestSetup, which allows for once-per-test-class
>> >> setup and teardown, as opposed to the class setup and teardown
>> >> that occur for every test (i.e., TestSetup is like the junit4 @
>> >> {Before,After}Class annotations).
>> >>
>> >> To use it, just wrap the normal return from suite() with it, like
>> >> this:
>> >>
>> >>   return new VMBrokerSetup(new junit.framework.TestSuite
>> >> (MapMessageTest.class));
>> >>
>> >> The MapMessageTest class below already does this, which means that
>> >> its setUp() and tearDown() aren't really needed.
>> >>
>> >> Martin, I can fix this test and any other new ones if you like,
>> >> given that I should have mentioned this earlier.
>> >>
>> >> --steve
>> >>
>> >> On Nov 22, 2006, at 12:46 PM, ritchiem@apache.org wrote:
>> >>
>> >>> client/src/test/java/org/apache/qpid/test/unit/basic/
>> >>> MapMessageTest.java
>> >>
>> >
>>
>>
>
>
> -- 
> Martin Ritchie

Re: svn commit: r478253 - in /incubator/qpid/trunk/qpid/java/client/src: main/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/client/message/ test/java/org/apache/qpid/test/unit/basic/ test/java/org/apache/qpid/test/unit/client/message

[Martin Ritchie <ri...@apache.org>]

On 22/11/06, Martin Ritchie <ri...@apache.org> wrote:
> I was seeing some strangeness when running from my IDE the the
> connection would simply sleep because the broker had not been created.
> I saw that again just now with the TransactedTest (who's setup method
> had the wrong capitalisation, which is why it wasn't working with
> junit3) but after I added the create broker in the setup before the
> connection creation all was well. And oddly all is still well after I
> have taken it out.
>
> I think it is important that on the wiki page you document how to
> create/destroy a broker between test cases. I believe we have (but
> can't find them right) tests that leave the broker with stale data. We
> don't want to have to require our tests to ensure that they leave the
> broker 100% clean for the next test. Do we?

Ah just saw QPID-123 was this issue. Gordon resolved the issue with a
different approach but as things develop using private queues may not
be enough.

> On 22/11/06, Steve Vinoski <vi...@iona.com> wrote:
> > Yes, I agree, and will do so.
> >
> > --steve
> >
> > On Nov 22, 2006, at 1:05 PM, Carl Trieloff wrote:
> >
> > >
> > > Steve,
> > >
> > > reading this, I think we should wiki it for anyone new to the project.
> > >
> > > Carl.
> > >
> > > Steve Vinoski wrote:
> > >> I notice that the test just added below, and maybe other news ones
> > >> too, is explicitly creating a VMBroker in its setUp(). I forgot to
> > >> point out that in the refactored junit3 tests I recently
> > >> committed, I added a VMBrokerSetup class to do this. It's an
> > >> instance of junit TestSetup, which allows for once-per-test-class
> > >> setup and teardown, as opposed to the class setup and teardown
> > >> that occur for every test (i.e., TestSetup is like the junit4 @
> > >> {Before,After}Class annotations).
> > >>
> > >> To use it, just wrap the normal return from suite() with it, like
> > >> this:
> > >>
> > >>   return new VMBrokerSetup(new junit.framework.TestSuite
> > >> (MapMessageTest.class));
> > >>
> > >> The MapMessageTest class below already does this, which means that
> > >> its setUp() and tearDown() aren't really needed.
> > >>
> > >> Martin, I can fix this test and any other new ones if you like,
> > >> given that I should have mentioned this earlier.
> > >>
> > >> --steve
> > >>
> > >> On Nov 22, 2006, at 12:46 PM, ritchiem@apache.org wrote:
> > >>
> > >>> client/src/test/java/org/apache/qpid/test/unit/basic/
> > >>> MapMessageTest.java
> > >>
> > >
> >
> >
>
>
> --
> Martin Ritchie
>


-- 
Martin Ritchie