On Tue, Mar 15, 2011 at 12:08 PM, Robbie Gemmell <[email protected]>wrote:
> I feel that this change should not be included in the 0.10 release. > > I think we are too close to release (RC1 is due tomorrow yes?) and that > there isnt significant enough testing in this area to provide enough > confidence when making what is actually an appreciable change in client > behaviour at this stage. > > I actually disagree with your assessment. I certainly consider this a **very low risk** change, given that addressing is the *default*. The only difference here is that the create option is not selected and the impact of that is an exception will be thrown if the queue is not present. There is absolutely no other **extra** risk involved here. The risk you perceive here is the impact of the addressing implementation due to lack of tests which is a valid concern. However that risk is there whether or not this commit is included or not. This commit does not include anymore risk in the client than what is already there due to addressing being the default. Does that make sense ? Do you agree with that ? > So far as I can tell from the quick look I've taken so far > AddressBasedDestinationTest is the only test which exercises the Addressing > code to any extent, and does so often by prefixing the address strings with > 'ADDR:' because the test profiles are all running set to use the old > BindingURL syntax. As a result it would seem we ultimately arent testing > the > precise execution paths for many operations users will undertake at all. > > This is one of the biggest obstacles I had when testing the new address based implementation. Switching the default will create many test failures that is impossible to handle on my own. Also switching the default is not a good solution either as we still support the older binding URL mechanism as well. The addressing scheme actually exercised code paths in a certain way that it highlighted several race conditions, a few dead locks and many other errors that were there in the code base for a long time. Therefore it would have been nice had I been able to catch these issues using our test suites. Ex issues in durable subscribers, queue receivers etc.. I also believe this highlights a weakness in a our test suite - all though this is separate discussion on it's own. We should probably try to use the JMS interfaces as much as possible when writing our integration tests and have enough provisions to abstract out as many details as possible. This allows us to easily configure a single test to run under different configurations. Rajith Robbie > > On 14 March 2011 16:48, Rajith Attapattu <[email protected]> wrote: > > > The attached patch in QPID-3143 contains a simple one line fix and a > > modified test case to test the change. > > The same is committed to trunk at rev 1081460. > > The fix is low risk IMHO. > > > > However the fix is important as it makes the session.createQueue method > > spec > > complaint. > > It also allows the default behaviour to be easily overridden if the user > > chooses do so. > > > > Regards, > > > > Rajith > > >
