[GitHub] [activemq-artemis] gemmellr commented on pull request #4426: ARTEMIS-4235 fix map msg conversion from OpenWire->core

["gemmellr (via GitHub)" <gi...@apache.org>]

gemmellr commented on PR #4426:
URL: https://github.com/apache/activemq-artemis/pull/4426#issuecomment-1498910719

   > Not all of the tests cover this specific issue. Most of them, in fact, cover other uses cases involving the same JMS code with different protocols (i.e. core and AMQP). However, I felt it was worth confirming those work as expected.
   > 
   
   Yep, wasnt arguing against testing other things, just noting that it didnt look like it tested the reported issue and main thrust of the change.
   
   > That said, both `testMapMessageConversionBetweenOpenWireAndAMQP` and `testMapMessageConversionBetweenOpenWireAndCore` fail without the fix.
   > 
   > I'll add a unit test for good measure.
   
   Interesting, I'm actually surprised to hear those failed in the reported manner given the description and earlier discussion on the JIRA. It sounded like it was the handling of an empty/null map message bodies at issue, and the code change looked to be along lines of 'treat null map message contents as being empty', yet as the test then wasnt using an empty map message at all it just didnt seem like it was exercising the reported issue / main change at all.
   
   So, does that actually mean that no Openwire map messages worked if being converted?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org