[GitHub] [ozone] adoroszlai commented on a diff in pull request #3415: HDDS-6738. Migrate tests with rules in hdds-server-framework to JUnit5

[GitBox <gi...@apache.org>]

adoroszlai commented on code in PR #3415:
URL: https://github.com/apache/ozone/pull/3415#discussion_r875545104


##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java:
##########
@@ -218,131 +219,100 @@ public void forEachAndIterator() throws Exception {
           localCount++;
         }
 
-        Assert.assertEquals(iterCount, localCount);
+        Assertions.assertEquals(iterCount, localCount);
         iter.seekToFirst();
         iter.forEachRemaining(TestRDBTableStore::consume);
-        Assert.assertEquals(iterCount, count);
+        Assertions.assertEquals(iterCount, count);
 
       }
     }
   }
 
   @Test
   public void testIsExist() throws Exception {
-    DBOptions rocksDBOptions = new DBOptions();
-    rocksDBOptions.setCreateIfMissing(true);
-    rocksDBOptions.setCreateMissingColumnFamilies(true);
-
-    String tableName = StringUtils.bytes2String(RocksDB.DEFAULT_COLUMN_FAMILY);
-
-    Set<TableConfig> configSet = new HashSet<>();
-    TableConfig newConfig = new TableConfig(tableName,
-        new ColumnFamilyOptions());
-    configSet.add(newConfig);
 
-    File rdbLocation = folder.newFolder();
-    RDBStore dbStore = new RDBStore(rdbLocation, rocksDBOptions, configSet);

Review Comment:
   I'm not sure this (and other similar blocks below) should be removed at all, but certainly not part of a junit upgrade patch.



##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java:
##########
@@ -239,101 +226,84 @@ public void listTables() throws Exception {
     int count = families.size();
     // Assert that we have all the tables in the list and no more.
     for (String name : families) {
-      Assert.assertTrue(hashTable.containsKey(name));
+      Assertions.assertTrue(hashTable.containsKey(name));
       count--;
     }
-    Assert.assertEquals(0, count);
+    Assertions.assertEquals(0, count);
   }
 
   @Test
   public void testRocksDBCheckpoint() throws Exception {
-    try (RDBStore newStore =
-             new RDBStore(folder.newFolder(), options, configSet)) {

Review Comment:
   Can we keep these?  I'm not sure why a new store is created, but let's clean them up in separate Jira.



##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java:
##########
@@ -374,9 +344,12 @@ public void testIteratorRemoveFromDB() throws Exception {
       TableIterator<byte[], ? extends Table.KeyValue<byte[], byte[]>> iterator =
           testTable.iterator();
       iterator.removeFromDB();
-      Assert.assertNull(testTable.get("1".getBytes(StandardCharsets.UTF_8)));
-      Assert.assertNotNull(testTable.get("2".getBytes(StandardCharsets.UTF_8)));
-      Assert.assertNotNull(testTable.get("3".getBytes(StandardCharsets.UTF_8)));
+      Assertions.assertNull(
+          testTable.get("1".getBytes(StandardCharsets.UTF_8)));
+      Assertions.assertNotNull(
+          testTable.get("2".getBytes(StandardCharsets.UTF_8)));
+      Assertions.assertNotNull(
+          testTable.get("3".getBytes(StandardCharsets.UTF_8)));

Review Comment:
   Instead of line wrapping, can you please extract constants for `"1".getBytes(StandardCharsets.UTF_8)`, etc.?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org