You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "skytin1004 (via GitHub)" <gi...@apache.org> on 2023/06/11 13:56:12 UTC

[GitHub] [iceberg] skytin1004 commented on pull request #7803: Core: Switch tests to Junit5 in metrics,puffin,rest pakages

skytin1004 commented on PR #7803:
URL: https://github.com/apache/iceberg/pull/7803#issuecomment-1586176014

   Thank you for the detailed review, @nastra.
   Based on your review, I went back through all the code in this PR and deleted the `.as(..) `and added a  `.hasMessage(..)` method in `assertThatThrownBy` . 
   
   In addition, I deleted the unnecessary as(..) in the part you suggested ("Namespaces should be equal" and "Properties should be equal") and 
      `Assert.assertFalse(
           "The merged properties map should omit keys with null values", merged.containsValue(null))` I changed this to code that uses AssertJ using the method you suggested.


-- 
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: issues-unsubscribe@iceberg.apache.org

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


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