alien11689 commented on code in PR #107:
URL: https://github.com/apache/aries-rsa/pull/107#discussion_r3228844848
##########
provider/tcp/src/test/java/org/apache/aries/rsa/provider/tcp/TcpProviderTest.java:
##########
@@ -90,14 +90,14 @@ public void createServerAndProxy() throws IOException {
Map<String, Object> props = new HashMap<>();
EndpointHelper.addObjectClass(props, exportedInterfaces);
int port = getFreePort();
- props.put("aries.rsa.hostname", "localhost");
- props.put("aries.rsa.port", port);
- props.put("aries.rsa.numThreads", NUM_THREADS);
+ props.put("aries.tcp.hostname", "localhost");
Review Comment:
I think it would be really good to reuse constants - if we afraid that we
can by mistake change value for the const in main code then let's create the
separate exact consts in the test code
##########
provider/tcp/src/main/java/org/apache/aries/rsa/provider/tcp/Config.java:
##########
@@ -65,7 +69,11 @@ int getInt(String key, int defaultValue) {
String getString(String key, String defaultValue) {
Object value = props.get(key);
- return value != null ? value.toString() : defaultValue;
+ if (value == null)
Review Comment:
please add `{}` in the body - it's a good practice that should be always used
##########
rsa/src/test/java/org/apache/aries/rsa/core/RemoteServiceAdminCoreTest.java:
##########
@@ -25,11 +25,7 @@
import static org.hamcrest.Matchers.array;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
Review Comment:
honestly I prefer the exact imports instead of wildcards - then as the
reviewer I always know from where the class / method comes
--
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]
