adoroszlai commented on code in PR #5901:
URL: https://github.com/apache/ozone/pull/5901#discussion_r1439215584
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java:
##########
@@ -633,16 +634,16 @@ public void testCreateSnapshotIdempotent() throws
Exception {
// Create first checkpoint for the snapshot checkpoint
OmSnapshotManager.createOmSnapshotCheckpoint(om.getMetadataManager(),
first);
- assertFalse(logCapturer.getOutput().contains(
- "for snapshot " + first.getName() + " already exists."));
+ assertThat(logCapturer.getOutput()).contains(
+ "for snapshot " + first.getName() + " already exists.");
Review Comment:
This one was `assertFalse`:
```suggestion
assertThat(logCapturer.getOutput()).doesNotContain(
"for snapshot " + first.getName() + " already exists.");
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java:
##########
@@ -806,7 +807,7 @@ private void checkNotAValidPath(String keyName) {
omKeyCreateRequest.preExecute(ozoneManager);
fail("checkNotAValidPath failed for path" + keyName);
} catch (IOException ex) {
- assertTrue(ex instanceof OMException);
+ assertInstanceOf(OMException.class, ex);
OMException omException = (OMException) ex;
Review Comment:
nit: cast can be removed
```suggestion
OMException omException = assertInstanceOf(OMException.class, ex);
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java:
##########
@@ -282,7 +283,7 @@ public void testFetchSecretForRevokedUser() throws
IOException {
ozoneManager, 2);
// Verify that the revoke operation was successful.
- assertTrue(omRevokeResponse instanceof S3RevokeSecretResponse);
+ assertInstanceOf(S3RevokeSecretResponse.class, omRevokeResponse);
S3RevokeSecretResponse s3RevokeSecretResponse =
(S3RevokeSecretResponse) omRevokeResponse;
Review Comment:
nit: cast can be removed
```suggestion
S3RevokeSecretResponse s3RevokeSecretResponse =
assertInstanceOf(S3RevokeSecretResponse.class, omRevokeResponse);
```
(Also in other test cases in this class.)
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java:
##########
@@ -1174,7 +1175,7 @@ public void testListSnapshotDiffJobs(String jobStatus,
// there should be a response.
// Otherwise, response list should be empty.
if (containsJob) {
- assertTrue(jobList.contains(diffJob));
+ assertThat(jobList).contains(diffJob);
} else {
assertTrue(jobList.isEmpty());
Review Comment:
nit: this can be replaced, too. This way test failure shows list contents
if it is found not empty.
```suggestion
assertThat(jobList).isEmpty();
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerUnit.java:
##########
@@ -515,9 +515,9 @@ public void testLookupFileWithDnFailure() throws
IOException {
.getBlockID().getLocalID());
assertEquals(pipelineTwo.getId(),
newBlockLocation.getPipeline().getId());
- assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnFour));
- assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnFive));
- assertTrue(newBlockLocation.getPipeline().getNodes().contains(dnSix));
+ assertThat(newBlockLocation.getPipeline().getNodes()).contains(dnFour);
+ assertThat(newBlockLocation.getPipeline().getNodes()).contains(dnFive);
+ assertThat(newBlockLocation.getPipeline().getNodes()).contains(dnSix);
Review Comment:
nit: we can check multiple items in a single call
```suggestion
assertThat(newBlockLocation.getPipeline().getNodes())
.contains(dnFour, dnFive, dnSix);
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/ratis/TestOzoneManagerRatisServer.java:
##########
@@ -197,10 +197,9 @@ public void testIsReadOnlyCapturesAllCmdTypeEnums() throws
Exception {
.setClientId(clientId)
.build();
OmUtils.isReadOnly(request);
- assertFalse(
- logCapturer.getOutput().contains("CmdType " + cmdtype + " is not " +
- "categorized as readOnly or not."),
- cmdtype + " is not categorized in OmUtils#isReadyOnly");
+ assertThat(logCapturer.getOutput())
+ .withFailMessage(cmdtype + " is not categorized in
OmUtils#isReadyOnly")
+ .doesNotContain("CmdType " + cmdtype + " is not " + "categorized as
readOnly or not.");
Review Comment:
nit:
```suggestion
.doesNotContain("CmdType " + cmdtype + " is not categorized as
readOnly or not.");
```
--
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]
