You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Martin Ritchie <ri...@apache.org> on 2010/08/01 01:25:12 UTC
Re: svn commit: r979316 - /qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java
On 26 July 2010 15:39, <gr...@apache.org> wrote:
> Author: grkvlt
> Date: Mon Jul 26 14:39:33 2010
> New Revision: 979316
>
> URL: http://svn.apache.org/viewvc?rev=979316&view=rev
> Log:
> QPID-2744: Make JMSPropertiesTest deal with both types of error messages
>
> Modified:
> qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java
>
> Modified: qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java
> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java?rev=979316&r1=979315&r2=979316&view=diff
> ==============================================================================
> --- qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java (original)
> +++ qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java Mon Jul 26 14:39:33 2010
> @@ -100,7 +100,8 @@ public class JMSPropertiesTest extends Q
> catch (MessageFormatException mfe)
> {
> // Check the error message
> - assertTrue("Incorrect error message: " + mfe.getMessage(), mfe.getMessage().contains("Object is null"));
> + assertEquals("Incorrect error message",
> + isBroker010() ? "Object is null" : "Only Primitives objects allowed Object is:null", mfe.getMessage());
> }
>
> // send it
As the 0-10 code base was recently modified to throw "Object is null"
I would suggest we update it to throw the same exception text as the
0-8/9 code path.. this will simplify testing and simplify the user
experience.
Martin
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project: http://qpid.apache.org
> Use/Interact: mailto:commits-subscribe@qpid.apache.org
>
>
--
Martin Ritchie
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r979316 - /qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/unit/message/JMSPropertiesTest.java
Posted by Andrew Kennedy <an...@gmail.com>.
Agreed,
In fact, I've changed all of the places that check for this situation
to use the same message. The only problem is, I had to find somewhere
to put the exception message being used, since it needed to be
available for two different exceptions, one of which is a JMS
provided class. In the end, I chose a public static final field in
the AMQPInvalidClassException class, since it is accessible by all
the code paths. However I view this as a stop-gap solution, since if
we ever want to embark on any i18n of messages this isn't the best
option. Should there be some kind of error message resource file,
perhaps as part of the common module?
I used a similar mechanism for error messages in the access control
plugin with printf style placeholders, but since one of the things
that is being worked on currently is broker and client exceptions, we
should put some more thought into the message text handling. As
mentioned, the obvious ways are final Strings in a single interface
or class, or resources in property files - with either printf '%s'
style placeholders for String.format(...) or '{0}' MessageFormat
style placeholders. Since resource files are more suitable for i18n,
I'd prefer this way, although it involves a bit more work, but the
choice of printf versus MessageFormat is just a matter of preference.
Any ideas appreciated...
Andrew.
--
-- andrew d kennedy ? do not fold, bend, spindle, or mutilate ;
-- http://grkvlt.blogspot.com/ ? edinburgh : +44 7941 197 134 ;
On 1 Aug 2010, at 00:25, Martin Ritchie wrote:
> On 26 July 2010 15:39, <gr...@apache.org> wrote:
>> Author: grkvlt
>> Date: Mon Jul 26 14:39:33 2010
>> New Revision: 979316
>>
>> URL: http://svn.apache.org/viewvc?rev=979316&view=rev
>> Log:
>> QPID-2744: Make JMSPropertiesTest deal with both types of error
>> messages
>>
>> Modified:
>> qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/
>> test/unit/message/JMSPropertiesTest.java
>>
>> Modified: qpid/trunk/qpid/java/systests/src/main/java/org/apache/
>> qpid/test/unit/message/JMSPropertiesTest.java
>> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/systests/
>> src/main/java/org/apache/qpid/test/unit/message/
>> JMSPropertiesTest.java?rev=979316&r1=979315&r2=979316&view=diff
>> =====================================================================
>> =========
>> --- qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/
>> test/unit/message/JMSPropertiesTest.java (original)
>> +++ qpid/trunk/qpid/java/systests/src/main/java/org/apache/qpid/
>> test/unit/message/JMSPropertiesTest.java Mon Jul 26 14:39:33 2010
>> @@ -100,7 +100,8 @@ public class JMSPropertiesTest extends Q
>> catch (MessageFormatException mfe)
>> {
>> // Check the error message
>> - assertTrue("Incorrect error message: " +
>> mfe.getMessage(), mfe.getMessage().contains("Object is null"));
>> + assertEquals("Incorrect error message",
>> + isBroker010() ? "Object is null" : "Only
>> Primitives objects allowed Object is:null", mfe.getMessage());
>> }
>>
>> // send it
>
> As the 0-10 code base was recently modified to throw "Object is null"
> I would suggest we update it to throw the same exception text as the
> 0-8/9 code path.. this will simplify testing and simplify the user
> experience.
>
> Martin
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org