Rob, I agree about the complexity involved in running the test profiles and I agree with you about the set of test profiles you mentioned in the email. While the above criteria could be relaxed for certain changes, for other non trivial changes that has a history of issues the above should be mandatory. I think we could trust each committer to make a call on what those changes are and certainly others could help with reviews etc.
An earlier attempt at solving the above problem created issues before and I am concerned the same is being committed again. I am currently running the C++ test profile on this, but I would have certainly appreciated if it was run before the commit. Rajith On Mon, Jul 26, 2010 at 10:49 AM, Robert Godfrey <rob.j.godf...@gmail.com> wrote: > 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 > > -- 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
