EdColeman commented on code in PR #3177:
URL: https://github.com/apache/accumulo/pull/3177#discussion_r1092551024
##########
core/src/test/java/org/apache/accumulo/core/conf/PropertyTest.java:
##########
@@ -88,13 +88,28 @@ public void testPorts() {
for (Property prop : Property.values()) {
if (prop.getType().equals(PropertyType.PORT)) {
int port = Integer.parseInt(prop.getDefaultValue());
+ assertTrue(Property.isValidProperty(prop.getKey(),
Integer.toString(port)));
assertFalse(usedPorts.contains(port), "Port already in use: " + port);
usedPorts.add(port);
assertTrue(port > 1023 && port < 65536, "Port out of range of valid
ports: " + port);
}
}
}
+ @Test
+ public void testBooleans() {
+ for (Property prop : Property.values()) {
Review Comment:
Is this loop necessary? The test would be valid with any single boolean
property. Also, the test in ShellIT that you added is more comprehensive,
testing each type - would it be better to test those conditions here? Not sure
if it needs to be in both places, but unit testing would be faster than needing
to run an IT.
--
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]
