[GitHub] [hadoop-ozone] llemec commented on pull request #1425: HDDS-2981 Add unit tests for Proto [de]serialization

[GitBox <gi...@apache.org>]

llemec commented on pull request #1425:
URL: https://github.com/apache/hadoop-ozone/pull/1425#issuecomment-694161427


   Hello @fapifta,
   
   Thank you for reviewing the diff. Addressing your points:
   
   The testgetFromProtobufOneMetadataOneAcl() indeed only checks the presence of metadata and acl because looking at OmPrefixInfo#getFromProtobuf(), those parts of the message are included by calling the KeyValueUtil#getFromProtobuf() and OzoneAclUtil#fromProtobuf() methods respectively. To test the contents of the Acls and Metadata would mean in fact to test these methods implicitly. I propose we do those explicitly and in a separate jira.
   By the same token with testGetProtobuf(), I propose we test the getProtobuf methods of KeyValueUtil and OzoneAclUtil separately and explicitly. 
   In the explicit tests I agree, we should check the corner cases for both metadata and acls
   Following this train of thought it is visible that Protobuf objects will be necessary for testing the different protos in Om. That is the proposed role of TestInstanceHelper - true, at the moment it only contains the helper methods for OmPrefixInfo testing.
   
   Please let me know your thoughts on this.
   
   Thank you,
   
   ps: empty lines removal: sure.
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org