rdblue commented on code in PR #3231:
URL: https://github.com/apache/iceberg/pull/3231#discussion_r1292845058


##########
core/src/test/java/org/apache/iceberg/encryption/TestCiphers.java:
##########
@@ -51,6 +53,21 @@ private void testEncryptDecrypt(byte[] aad) {
       Ciphers.AesGcmDecryptor decryptor = new Ciphers.AesGcmDecryptor(key);
       byte[] decryptedText = decryptor.decrypt(ciphertext, aad);
       assertThat(decryptedText).as("Key length " + 
keyLength).isEqualTo(plaintext);
+
+      // Test bad aad

Review Comment:
   This needs to be a separate test case / method.
   
   When you add additional cases to an existing test method, the tests are 
harder to work with because you don't get accurate descriptions of what failed. 
Plus, if there are issues with the case above, this test code won't be run so 
the person trying to figure out why tests are failing gets less information. 
Multiple failing cases are helpful for determining what needs to be fixed and 
for finding a good fix the first time that doesn't cause other failures.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to