kpvdr commented on pull request #263: URL: https://github.com/apache/qpid-proton/pull/263#issuecomment-656768640
@gemmellr: The question of testing against the expected encoding on a byte-for-byte basis should affect ALL codec tests, not just the array tests. Do we check this for other AMQP type encodings? That being said, there is nothing wrong with introducing this for array tests. I agree with your analysis that it would pick up additional test and encoding path issues that just comparing the inspect() values would not do. Agreed on adding additional list test cases as you have described, this is fortunately trivial. With the proposed fix for the array empty list issue, I think all lists, even when all lists are empty, will result in the long (4-byte) encoding for each empty list. I think checking each and every array element for emptiness so that the empty list encoding could be used would probably add noticeable overhead (not to mention some additional complexity) to the code. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
