greyp9 commented on a change in pull request #5870:
URL: https://github.com/apache/nifi/pull/5870#discussion_r828322872
##########
File path:
nifi-commons/nifi-site-to-site-client/src/test/java/org/apache/nifi/remote/client/socket/TestSiteToSiteClient.java
##########
@@ -16,89 +16,18 @@
*/
package org.apache.nifi.remote.client.socket;
-import com.esotericsoftware.kryo.Kryo;
-import com.esotericsoftware.kryo.io.Input;
-import com.esotericsoftware.kryo.io.Output;
-import org.apache.nifi.components.state.StateManager;
import org.apache.nifi.remote.client.SiteToSiteClient;
import org.apache.nifi.remote.client.SiteToSiteClientConfig;
import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
import java.util.LinkedHashSet;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
public class TestSiteToSiteClient {
- @Test
- public void testSerialization() {
- final SiteToSiteClientConfig clientConfig = new
SiteToSiteClient.Builder()
- .url("http://localhost:8080/nifi";)
- .portName("input")
- .buildConfig();
-
- final Kryo kryo = new Kryo();
-
- final ByteArrayOutputStream out = new ByteArrayOutputStream();
- final Output output = new Output(out);
-
- try {
- kryo.writeObject(output, clientConfig);
- } finally {
- output.close();
- }
-
- final ByteArrayInputStream in = new
ByteArrayInputStream(out.toByteArray());
- final Input input = new Input(in);
-
- try {
- SiteToSiteClientConfig clientConfig2 = kryo.readObject(input,
SiteToSiteClient.StandardSiteToSiteClientConfig.class);
- assertEquals(clientConfig.getUrls(), clientConfig2.getUrls());
- } finally {
- input.close();
- }
- }
-
- @Test
- public void testSerializationWithStateManager() {
- final StateManager stateManager = Mockito.mock(StateManager.class);
- final SiteToSiteClientConfig clientConfig = new
SiteToSiteClient.Builder()
- .url("http://localhost:8080/nifi";)
- .portName("input")
- .stateManager(stateManager)
- .buildConfig();
-
- final Kryo kryo = new Kryo();
-
- final ByteArrayOutputStream out = new ByteArrayOutputStream();
- final Output output = new Output(out);
-
- try {
- kryo.writeObject(output, clientConfig);
- } finally {
- output.close();
- }
-
- final ByteArrayInputStream in = new
ByteArrayInputStream(out.toByteArray());
- final Input input = new Input(in);
-
- try {
- SiteToSiteClientConfig clientConfig2 = kryo.readObject(input,
SiteToSiteClient.StandardSiteToSiteClientConfig.class);
- assertEquals(clientConfig.getUrls(), clientConfig2.getUrls());
- // Serialization works, but the state manager is not serialized.
- assertNotNull(clientConfig.getStateManager());
- assertNull(clientConfig2.getStateManager());
- } finally {
- input.close();
- }
- }
-
+ @SuppressWarnings("deprecation")
Review comment:
There seems to be a good deal of alternate test coverage for the three
nifi classes under test here. But another option would be to replace the
problematic `com.esotericsoftware` usage. (just making notes...)
##########
File path:
nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/test/java/org/apache/hive/streaming/TestNiFiRecordSerDe.java
##########
@@ -43,6 +45,7 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
+@DisabledOnJre(value = JRE.JAVA_17, disabledReason = "Hive3 StringInternUtils
illegal reflective access")
Review comment:
Definitely a benefit of moving to junit 5!
##########
File path:
nifi-registry/nifi-registry-extensions/nifi-registry-ranger/nifi-registry-ranger-plugin/src/test/java/org/apache/nifi/registry/ranger/TestRangerAuthorizer.java
##########
@@ -220,26 +204,7 @@ public void testKerberosEnabledWithoutKeytabOrPrincipal() {
when(registryProperties.getKerberosServiceKeytabLocation()).thenReturn("");
when(registryProperties.getKerberosServicePrincipal()).thenReturn("");
- try {
- setup(registryProperties, mock(UserGroupProvider.class),
configurationContext);
- Assert.fail("Should have thrown exception");
- } catch (SecurityProviderCreationException e) {
- // want to make sure this exception is from our authorizer code
- verifyOnlyAuthorizeCreationExceptions(e);
- }
- }
-
- private void
verifyOnlyAuthorizeCreationExceptions(SecurityProviderCreationException e) {
- boolean foundOtherException = false;
- Throwable cause = e.getCause();
- while (cause != null) {
- if (!(cause instanceof SecurityProviderCreationException)) {
- foundOtherException = true;
- break;
- }
- cause = cause.getCause();
- }
- assertFalse(foundOtherException);
+ assertThrows(SecurityProviderCreationException.class, () ->
setup(registryProperties, mock(UserGroupProvider.class), configurationContext));
Review comment:
Are we losing anything significant here?
##########
File path:
nifi-nar-bundles/nifi-enrich-bundle/nifi-enrich-processors/src/test/java/org/apache/nifi/processors/TestISPEnrichIP.java
##########
@@ -252,8 +240,6 @@ public void
whenInetAddressThrowsUnknownHostFlowFileShouldBeSentToNotFound() thr
final Map<String, String> attributes = new HashMap<>();
attributes.put("ip", "somenonexistentdomain.comm");
-
when(InetAddress.getByName("somenonexistentdomain.comm")).thenThrow(UnknownHostException.class);
Review comment:
Why don't we need the `Mockito.when` anymore?
##########
File path:
nifi-nar-bundles/nifi-ranger-bundle/nifi-ranger-plugin/src/test/java/org/apache/nifi/ranger/authorization/TestRangerNiFiAuthorizer.java
##########
@@ -200,19 +181,7 @@ public void testKerberosEnabled() {
authorizer = new MockRangerNiFiAuthorizer(rangerBasePlugin);
authorizer.setNiFiProperties(nifiProperties);
- AuthorizerCreationException e =
assertThrows(AuthorizerCreationException.class,
- () -> authorizer.onConfigured(configurationContext));
- // getting a LoginException here means we attempted to login which is
what we want
- boolean foundLoginException = false;
- Throwable cause = e.getCause();
- while (cause != null) {
- if (cause instanceof LoginException) {
- foundLoginException = true;
- break;
- }
- cause = cause.getCause();
- }
- assertTrue(foundLoginException);
+ assertThrows(AuthorizerCreationException.class, () ->
authorizer.onConfigured(configurationContext));
Review comment:
Not sure how important it is, but we're losing some precision here.
##########
File path:
nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/util/TlsHelperTest.java
##########
@@ -108,37 +95,16 @@
private KeyPairGenerator keyPairGenerator;
- private KeyStore keyStore;
-
- private String password = "changeit";
-
- @Mock
- KeyStoreSpi keyStoreSpi;
+ private final String password = "changeit";
- @Mock
- Provider keyStoreProvider;
-
- @Mock
+ @Mock(lenient = true)
OutputStreamFactory outputStreamFactory;
- @Rule
- public TemporaryFolder tempFolder = new TemporaryFolder();
-
private ByteArrayOutputStream tmpFileOutputStream;
private File file;
- private static void setUnlimitedCrypto(boolean value) {
Review comment:
Does this removal imply that the unlimited strength crypto is expected,
or are we saying that it shouldn't matter?
##########
File path:
nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/util/PasswordUtilTest.java
##########
@@ -17,47 +17,28 @@
package org.apache.nifi.toolkit.tls.util;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
-import java.math.BigInteger;
-import java.nio.ByteBuffer;
import java.security.SecureRandom;
-import java.util.Arrays;
-import java.util.Base64;
import java.util.function.Supplier;
-import static org.junit.Assert.assertEquals;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.mock;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class PasswordUtilTest {
@Test
public void testGeneratePassword() {
- SecureRandom secureRandom = mock(SecureRandom.class);
+ SecureRandom secureRandom = new SecureRandom();
PasswordUtil passwordUtil = new PasswordUtil(secureRandom);
- int value = 8675309;
Review comment:
!
##########
File path:
nifi-nar-bundles/nifi-ignite-bundle/nifi-ignite-processors/src/test/java/org/apache/nifi/processors/ignite/cache/TestGetIgniteCache.java
##########
@@ -142,9 +142,8 @@ protected Ignite getIgnite() {
List<MockFlowFile> getFailureFlowFiles =
getRunner.getFlowFilesForRelationship(GetIgniteCache.REL_FAILURE);
assertEquals(1, getFailureFlowFiles.size());
- final MockFlowFile getOut =
getRunner.getFlowFilesForRelationship(GetIgniteCache.REL_FAILURE).get(0);
-
getOut.assertAttributeEquals(GetIgniteCache.IGNITE_GET_FAILED_REASON_ATTRIBUTE_KEY,
Review comment:
```
Assertions.assertTrue(getOut.getAttribute(GetIgniteCache.IGNITE_GET_FAILED_REASON_ATTRIBUTE_KEY).contains(
GetIgniteCache.IGNITE_GET_FAILED_MESSAGE_PREFIX +
"java.lang.NullPointerException"));
```
Looks like the new exception is purely additive, so we could constrain the
value safely.
--
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]
