keith-turner commented on a change in pull request #2177:
URL: https://github.com/apache/accumulo/pull/2177#discussion_r657264184



##########
File path: 
server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java
##########
@@ -98,4 +103,48 @@ public void testSortOrder() {
     }
 
   }
+
+  private void testToFromKey(int tabletId, long seq) throws IOException {
+    var logFileKey = nk(LogEvents.DEFINE_TABLET, tabletId, seq);

Review comment:
       Would it make sense to loop through all of the LogEvent types here, 
running the test for each one?

##########
File path: 
server/tserver/src/test/java/org/apache/accumulo/tserver/log/LogFileKeyTest.java
##########
@@ -98,4 +103,48 @@ public void testSortOrder() {
     }
 
   }
+
+  private void testToFromKey(int tabletId, long seq) throws IOException {
+    var logFileKey = nk(LogEvents.DEFINE_TABLET, tabletId, seq);
+    logFileKey.tablet = new KeyExtent(TableId.of("5"), new Text("b"), null);
+    Key k = logFileKey.toKey();
+
+    var converted = LogFileKey.fromKey(k);
+    assertEquals("Failed to convert tabletId = " + tabletId + " seq = " + seq, 
logFileKey,
+        converted);
+  }
+
+  @Test
+  public void convertToKeyLargeTabletId() throws IOException {
+    // test continuous range of numbers to make sure all cases of signed bytes 
are covered
+    for (int i = -1_000; i < 1_000_000; i++) {
+      testToFromKey(i, 1);
+    }
+  }
+
+  @Test
+  public void convertToKeyLargeSeq() throws IOException {
+    // use numbers large enough to cover all bytes used
+    testToFromKey(1, 8); // 1 byte
+    testToFromKey(1, 1_000L); // 2 bytes

Review comment:
       could use hex instead of decimal, would make it easier to see what bytes 
are covered.




-- 
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:
[email protected]


Reply via email to