ctubbsii commented on code in PR #5398:
URL: https://github.com/apache/accumulo/pull/5398#discussion_r2045790261
##########
core/src/test/java/org/apache/accumulo/core/rpc/AccumuloProtocolTest.java:
##########
@@ -103,6 +105,59 @@ public void testIncompatibleVersion() throws TException {
}
}
+ /**
+ * Test that compatible accumulo version (same major.minor) passes validation
+ */
+ @Test
+ public void testCompatibleVersions() throws TException {
+ try (TMemoryBuffer transport = new TMemoryBuffer(100)) {
+ TCompactProtocol protocol = new TCompactProtocol(transport);
+ protocol.writeI32(VALID_MAGIC_NUMBER);
+ protocol.writeByte(VALID_PROTOCOL_VERSION);
+
+ // Write current version but with different patch version
+ String serverMajorMinor = Constants.VERSION.substring(0,
Constants.VERSION.lastIndexOf('.'));
+ String clientVersion = serverMajorMinor + ".999";
+
+ protocol.writeString(clientVersion);
+ protocol.writeBool(false);
+
+ AccumuloProtocolFactory.AccumuloProtocol serverProtocol =
+ (AccumuloProtocolFactory.AccumuloProtocol)
AccumuloProtocolFactory.serverFactory()
+ .getProtocol(transport);
+
+ assertDoesNotThrow(serverProtocol::validateHeader,
+ "Expected compatible version to pass validation");
Review Comment:
I think assertDoesNotThrow can be useful when we want to ignore a return
value that normally would be thrown and expected to be used by some static
analysis tool (like spotbugs). But in most code, we can just skip it
altogether, as in:
```suggestion
serverProtocol.validateHeader();
```
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
