I agree we should be checking all code against both brokers... but I think it will soon become unreasonable to expect that before every checkin we test all combinations.
At the moment we *should* be checking the following scenarios on a Java client checkin: Java InVM broker Java External broker AMQP 0-8/9/9-1 protocols (these should really be three separate test runs) Java External broker AMQP 0-10 protocol C++ External broker AMQP 0-10 protocol In addition some tests may perform differently when there is a persistent store attached, or when the broker is run on alternative operating systems, etc. Add in future work to add 1-0 protocol support to both brokers, and soon you are starting to look at 24 hours of test runs before any checkin - which I don't think is reasonable. Moreover the C++ broker is not portable to all operating systems, which is a severe obstacle if a Java programmer wishes to work on an operating system other than Linux (and at a stretch Windows). My own view is that on checkin we should need only to be running *unit* tests (which are in rather short supply to be fair). We should have a CI environment where *system* / *integration* tests are being run constantly with all possible profiles being tested. IMHO unit tests for the client should not be requiring a broker of any kind - broker functionality would be mocked up as part of the testing suite.... however there'd need to be a fair amount of work to get to that point. The highest priority for me would be looking to see if we can get a shared CI environment running on Apache which reported failures to the list. -- Rob On 26 July 2010 16:28, Rajith Attapattu <rajit...@gmail.com> wrote: > Andrew, > > Have you tested this change with the C++ test profiles ? > Anytime the 0-10 code path is changed, please make sure to test with > the C++ test profiles. > > For example a recent checkin cause the JMSProperty test to fail (I > believe in all test profiles). > While it's understandable that humans make mistakes, it's important we > ensure that the builds don't break. > > Rajith > > On Mon, Jul 26, 2010 at 9:56 AM, <grk...@apache.org> wrote: >> Author: grkvlt >> Date: Mon Jul 26 13:56:52 2010 >> New Revision: 979283 >> >> URL: http://svn.apache.org/viewvc?rev=979283&view=rev >> Log: >> QPID-2586: Give Client 0-10 close semantics not detach >> >> Added a sessionRequestTimeout handler that sets expiry and responds with a >> sessionTimeout, and makes sessionTimeout set expiry appropriately also. On >> attach uses the expiry provided, rather than forcing a value of 0. >> >> Modified: >> >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java >> >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java >> >> Modified: >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java >> URL: >> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java?rev=979283&r1=979282&r2=979283&view=diff >> ============================================================================== >> --- >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java >> (original) >> +++ >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java >> Mon Jul 26 13:56:52 2010 >> @@ -237,9 +237,7 @@ public class Session extends SessionInvo >> { >> initReceiver(); >> sessionAttach(name.getBytes()); >> - // XXX: when the broker and client support full session >> - // recovery we should use expiry as the requested timeout >> - sessionRequestTimeout(0); >> + sessionRequestTimeout(expiry); >> } >> >> void resume() >> >> Modified: >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java >> URL: >> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java?rev=979283&r1=979282&r2=979283&view=diff >> ============================================================================== >> --- >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java >> (original) >> +++ >> qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java >> Mon Jul 26 13:56:52 2010 >> @@ -57,6 +57,12 @@ public class SessionDelegate >> log.warn("UNHANDLED: [%s] %s", ssn, method); >> } >> >> + �...@override public void sessionRequestTimeout(Session ssn, >> SessionRequestTimeout t) >> + { >> + ssn.setExpiry(t.getTimeout()); >> + ssn.sessionTimeout(t.getTimeout()); >> + } >> + >> @Override public void sessionAttached(Session ssn, SessionAttached atc) >> { >> ssn.setState(Session.State.OPEN); >> @@ -64,9 +70,7 @@ public class SessionDelegate >> >> @Override public void sessionTimeout(Session ssn, SessionTimeout t) >> { >> - // XXX: we ignore this right now, we should uncomment this >> - // when full session resume is supported: >> - // ssn.setExpiry(t.getTimeout()); >> + ssn.setExpiry(t.getTimeout()); >> } >> >> @Override public void sessionCompleted(Session ssn, SessionCompleted cmp) >> >> >> >> --------------------------------------------------------------------- >> Apache Qpid - AMQP Messaging Implementation >> Project: http://qpid.apache.org >> Use/Interact: mailto:commits-subscr...@qpid.apache.org >> >> > > > > -- > Regards, > > Rajith Attapattu > Red Hat > http://rajith.2rlabs.com/ > > --------------------------------------------------------------------- > Apache Qpid - AMQP Messaging Implementation > Project: http://qpid.apache.org > Use/Interact: mailto:dev-subscr...@qpid.apache.org > > --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
