adoroszlai commented on code in PR #6291:
URL: https://github.com/apache/ozone/pull/6291#discussion_r1505528889
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBTableStore.java:
##########
@@ -96,11 +96,7 @@ public static void initConstants() {
private static boolean consume(Table.KeyValue keyValue) {
count++;
- try {
- assertNotNull(keyValue.getKey());
- } catch (IOException ex) {
- fail("Unexpected Exception " + ex);
- }
+ assertDoesNotThrow(() -> assertNotNull(keyValue.getKey()));
Review Comment:
Since both `assertNotNull` and `getKey()` can throw, I think it's better to
reverse order of assertions:
```suggestion
assertNotNull(assertDoesNotThrow(keyValue::getKey));
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java:
##########
@@ -534,16 +533,14 @@ private void assertTableRowCount(Table<String, ?> table,
int count)
private boolean assertTableRowCount(int expectedCount,
Table<String, ?> table) {
- long count = 0L;
- try {
- count = cluster.getOzoneManager().getMetadataManager()
- .countRowsInTable(table);
+ AtomicLong count = new AtomicLong(0L);
+ assertDoesNotThrow(() -> {
+
count.set(cluster.getOzoneManager().getMetadataManager().countRowsInTable(table));
LOG.info("{} actual row count={}, expectedCount={}", table.getName(),
- count, expectedCount);
- } catch (IOException ex) {
- fail("testDoubleBuffer failed with: " + ex);
- }
- return count == expectedCount;
+ count.get(), expectedCount);
Review Comment:
nit: please move `LOG.info` after the assertion, as it cannot throw.
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestTypedRDBTableStore.java:
##########
@@ -217,11 +217,7 @@ public void batchDelete() throws Exception {
private static boolean consume(Table.KeyValue keyValue) {
count++;
- try {
- assertNotNull(keyValue.getKey());
- } catch (IOException ex) {
- fail(ex.toString());
- }
+ assertDoesNotThrow(() -> assertNotNull(keyValue.getKey()));
Review Comment:
nit: Reverse the order of assertions, similar to the previous comment.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDirectoryCleaningService.java:
##########
@@ -258,15 +256,12 @@ private void assertTableRowCount(Table<String, ?> table,
int count)
private boolean assertTableRowCount(int expectedCount,
Table<String, ?> table) {
- long count = 0L;
- try {
- count = cluster.getOzoneManager().getMetadataManager()
- .countRowsInTable(table);
+ AtomicLong count = new AtomicLong(0L);
+ assertDoesNotThrow(() -> {
+
count.set(cluster.getOzoneManager().getMetadataManager().countRowsInTable(table));
LOG.info("{} actual row count={}, expectedCount={}", table.getName(),
- count, expectedCount);
- } catch (IOException ex) {
- fail("testDoubleBuffer failed with: " + ex);
- }
- return count == expectedCount;
+ count.get(), expectedCount);
Review Comment:
nit: please move `LOG.info` after the assertion, as it cannot throw.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java:
##########
@@ -626,15 +627,13 @@ private static void assertTableRowCount(Table<String, ?>
table,
private static boolean assertTableRowCount(long expectedCount,
Table<String, ?> table,
OMMetadataManager metadataManager) {
- long count = 0L;
- try {
- count = metadataManager.countRowsInTable(table);
+ AtomicLong count = new AtomicLong(0L);
+ assertDoesNotThrow(() -> {
+ count.set(metadataManager.countRowsInTable(table));
LOG.info("{} actual row count={}, expectedCount={}", table.getName(),
- count, expectedCount);
- } catch (IOException ex) {
- fail("testDoubleBuffer failed with: " + ex);
- }
- return count == expectedCount;
+ count.get(), expectedCount);
Review Comment:
nit: please move `LOG.info` after the assertion, as it cannot throw.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedDDSWithFSO.java:
##########
@@ -227,16 +225,13 @@ private void assertTableRowCount(Table<String, ?> table,
int count)
private boolean assertTableRowCount(int expectedCount,
Table<String, ?> table) {
- long count = 0L;
- try {
- count = cluster.getOzoneManager().getMetadataManager()
- .countRowsInTable(table);
+ AtomicLong count = new AtomicLong(0L);
+ assertDoesNotThrow(() -> {
+
count.set(cluster.getOzoneManager().getMetadataManager().countRowsInTable(table));
LOG.info("{} actual row count={}, expectedCount={}", table.getName(),
- count, expectedCount);
- } catch (IOException ex) {
- fail("testDoubleBuffer failed with: " + ex);
- }
- return count == expectedCount;
+ count.get(), expectedCount);
Review Comment:
nit: please move `LOG.info` after the assertion, as it cannot throw.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestSnapshotDeletingService.java:
##########
@@ -553,15 +554,12 @@ private void assertTableRowCount(Table<String, ?> table,
int count)
private boolean assertTableRowCount(int expectedCount,
Table<String, ?> table) {
- long count = 0L;
- try {
- count = cluster.getOzoneManager().getMetadataManager()
- .countRowsInTable(table);
+ AtomicLong count = new AtomicLong(0L);
+ assertDoesNotThrow(() -> {
+
count.set(cluster.getOzoneManager().getMetadataManager().countRowsInTable(table));
LOG.info("{} actual row count={}, expectedCount={}", table.getName(),
- count, expectedCount);
- } catch (IOException ex) {
- fail("testDoubleBuffer failed with: " + ex);
- }
- return count == expectedCount;
+ count.get(), expectedCount);
Review Comment:
nit: please move `LOG.info` after the assertion, as it cannot throw.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
