[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r548771173
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -251,13 +249,13 @@ private void byteArrayTest(final String transformation,
final byte[] key, final
final int n = enc.doFinal(input, 0, input.length, temp, 0);
final byte[] cipherText = new byte[n];
System.arraycopy(temp, 0, cipherText, 0, n);
- Assert.assertArrayEquals("byte array encryption error.",
output, cipherText);
+ assertArrayEquals(output, cipherText, "byte array encryption
error.");
temp = new byte[cipherText.length + blockSize];
final int m = dec.doFinal(cipherText, 0, cipherText.length,
temp, 0);
final byte[] plainText = new byte[m];
System.arraycopy(temp, 0, plainText, 0, m);
- Assert.assertArrayEquals("byte array decryption error.", input,
plainText);
+ assertArrayEquals(input, plainText,"byte array decryption
error.");
Review comment:
Fix formatting.
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
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r548771093
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -123,7 +127,7 @@ public void cryptoTest() throws Exception {
for (final String tran : transformations) {
/** uses the small data set in {@link TestData} */
cipherTests = TestData.getTestData(tran);
- assertNotNull(tran, cipherTests);
+ assertNotNull(cipherTests,tran);
Review comment:
Fix formatting.
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
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r542031287
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -251,13 +249,13 @@ private void byteArrayTest(final String transformation,
final byte[] key, final
final int n = enc.doFinal(input, 0, input.length, temp, 0);
final byte[] cipherText = new byte[n];
System.arraycopy(temp, 0, cipherText, 0, n);
- Assert.assertArrayEquals("byte array encryption error.",
output, cipherText);
+ assertArrayEquals( output, cipherText, "byte array encryption
error.");
temp = new byte[cipherText.length + blockSize];
final int m = dec.doFinal(cipherText, 0, cipherText.length,
temp, 0);
final byte[] plainText = new byte[m];
System.arraycopy(temp, 0, plainText, 0, m);
- Assert.assertArrayEquals("byte array decryption error.", input,
plainText);
+ assertArrayEquals( input, plainText,"byte array decryption
error.");
Review comment:
Fix formatting.
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -251,13 +249,13 @@ private void byteArrayTest(final String transformation,
final byte[] key, final
final int n = enc.doFinal(input, 0, input.length, temp, 0);
final byte[] cipherText = new byte[n];
System.arraycopy(temp, 0, cipherText, 0, n);
- Assert.assertArrayEquals("byte array encryption error.",
output, cipherText);
+ assertArrayEquals( output, cipherText, "byte array encryption
error.");
Review comment:
Fix formatting.
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
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] - Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r541744144
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -251,13 +247,13 @@ private void byteArrayTest(final String transformation,
final byte[] key, final
final int n = enc.doFinal(input, 0, input.length, temp, 0);
final byte[] cipherText = new byte[n];
System.arraycopy(temp, 0, cipherText, 0, n);
- Assert.assertArrayEquals("byte array encryption error.",
output, cipherText);
+ assertArrayEquals( output, cipherText);
Review comment:
You lost the assertion message and the formatting should match the rest
of the code.
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -220,9 +218,7 @@ private void byteBufferTest(final String transformation,
final byte[] key, final
final byte[] b = new byte[output.remaining()];
output.get(b);
final byte[] c = new
byte[encResult.remaining()];
- encResult.get(c);
- Assert.fail("AES failed encryption - expected "
+ new String(DatatypeConverter.printHexBinary(b))
- + " got " + new
String(DatatypeConverter.printHexBinary(c)));
+ assertThrows(IllegalArgumentException.class, ()
-> encResult.get(c));
Review comment:
Please preserve the assertion message, it will be handy when the test
fails.
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
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] - Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r541320735
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -58,11 +62,11 @@
0x08 };
private CryptoCipher enc, dec;
- @Before
+ @BeforeEach
public void setup() {
init();
- assertNotNull("cipherClass", cipherClass);
- assertNotNull("transformations", transformations);
+ assertNotNull( cipherClass, "cipherClass");
Review comment:
Fix formatting for this kind of change throughout.
##
File path:
src/test/java/org/apache/commons/crypto/cipher/AbstractCipherTest.java
##
@@ -123,7 +127,7 @@ public void cryptoTest() throws Exception {
for (final String tran : transformations) {
/** uses the small data set in {@link TestData} */
cipherTests = TestData.getTestData(tran);
- assertNotNull(tran, cipherTests);
+ assertNotNull( cipherTests);
Review comment:
Why are we loosing the failure message here?
##
File path: src/test/java/org/apache/commons/crypto/cipher/OpenSslCipherTest.java
##
@@ -18,174 +18,163 @@
package org.apache.commons.crypto.cipher;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
import java.nio.ByteBuffer;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.Properties;
+import java.util.concurrent.TimeUnit;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.IvParameterSpec;
import javax.crypto.spec.SecretKeySpec;
import javax.crypto.spec.GCMParameterSpec;
-import org.junit.Assert;
-import org.junit.Assume;
-import org.junit.Test;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
public class OpenSslCipherTest extends AbstractCipherTest {
@Override
public void init() {
-Assume.assumeTrue(OpenSsl.getLoadingFailureReason() == null);
+assumeTrue(OpenSsl.getLoadingFailureReason() == null);
transformations = new String[] {
"AES/CBC/NoPadding",
"AES/CBC/PKCS5Padding",
"AES/CTR/NoPadding"};
cipherClass = OPENSSL_CIPHER_CLASSNAME;
}
-@Test(expected = NoSuchPaddingException.class, timeout = 12)
-public void testInvalidPadding() throws Exception {
-Assume.assumeTrue(OpenSsl.getLoadingFailureReason() == null);
-OpenSsl.getInstance("AES/CTR/NoPadding2");
+@Test
+@Timeout(value = 12, unit = TimeUnit.MILLISECONDS)
+public void testInvalidPadding() {
+assumeTrue(OpenSsl.getLoadingFailureReason() == null);
+assertThrows(NoSuchPaddingException.class,
+() -> OpenSsl.getInstance("AES/CTR/NoPadding2"));
}
-@Test(expected = NoSuchAlgorithmException.class, timeout = 12)
-public void testInvalidMode() throws Exception {
-Assume.assumeTrue(OpenSsl.getLoadingFailureReason() == null);
-OpenSsl.getInstance("AES/CTR2/NoPadding");
+@Test
+@Timeout(value = 12, unit = TimeUnit.MILLISECONDS)
+public void testInvalidMode() {
+assumeTrue(OpenSsl.getLoadingFailureReason() == null);
+assertThrows(NoSuchAlgorithmException.class,
+() -> OpenSsl.getInstance("AES/CTR2/NoPadding"));
}
-@Test(timeout = 12)
+@Test
+@Timeout(value = 12, unit = TimeUnit.MILLISECONDS)
public void testUpdateArguments() throws Exception {
-Assume.assumeTrue(OpenSsl.getLoadingFailureReason() == null);
+assumeTrue(OpenSsl.getLoadingFailureReason() == null);
final OpenSsl cipher = OpenSsl
.getInstance("AES/CTR/NoPadding");
-Assert.assertNotNull(cipher);
+assertNotNull(cipher);
cipher.init(OpenSsl.ENCRYPT_MODE, KEY, new IvParameterSpec(IV));
// Require direct buffers
ByteBuffer input = ByteBuffer.allocate(1024);
ByteBuffer output = ByteBuffer.allocate(1024);
-try {
-cipher.update(input, output);
-Assert.fail("Should have failed to accept non-direct buffers.");
-} catch (final IllegalArgumentException e) {
-Assert.assertTrue(e.getMessage().contains(
-"Direct buffers are required"));
-}
+final ByteBuffer finalInput = input;
+final ByteBuffer finalOutput = output;
+assertThrows(IllegalArgumentException.class, () ->
cipher.update(finalInput, finalOutput));
// Output
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] - Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r537046441
##
File path:
src/test/java/org/apache/commons/crypto/stream/PositionedCryptoInputStreamTest.java
##
@@ -247,7 +250,7 @@ private void testSeekFailed(final String cipherClass, final
int position, final
getCipher(cipherClass), bufferSize);
try {
in.seek(position);
-Assert.fail("Excepted exception for cannot seek to negative
offset.");
+fail("Excepted exception for cannot seek to negative offset.");
} catch (final IllegalArgumentException iae) {
Review comment:
Hi @arturobernalg
Thank you for your PR.
If we go to JUnit 5, then let's replace this `try-catch` pattern with
`assertThrows()`
WDYT?
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
[GitHub] [commons-crypto] garydgregory commented on a change in pull request #114: [CRYPTO-151] - Migrate to Junit 5
garydgregory commented on a change in pull request #114:
URL: https://github.com/apache/commons-crypto/pull/114#discussion_r537046441
##
File path:
src/test/java/org/apache/commons/crypto/stream/PositionedCryptoInputStreamTest.java
##
@@ -247,7 +250,7 @@ private void testSeekFailed(final String cipherClass, final
int position, final
getCipher(cipherClass), bufferSize);
try {
in.seek(position);
-Assert.fail("Excepted exception for cannot seek to negative
offset.");
+fail("Excepted exception for cannot seek to negative offset.");
} catch (final IllegalArgumentException iae) {
Review comment:
Hi @arturobernalg
Thank you for your PR.
If we go to JUnit 5, then let's remove this pattern with `assertThrows()`
WDYT?
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
