errose28 commented on code in PR #6385:
URL: https://github.com/apache/ozone/pull/6385#discussion_r1594935417


##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCommitRequest.java:
##########
@@ -218,6 +224,75 @@ public void testValidateAndUpdateCache() throws Exception {
         omKeyInfo.getLatestVersionLocations().getLocationList());
   }
 
+  @Test
+  public void testAtomicRewrite() throws Exception {

Review Comment:
   Optional suggestion:
   
   The large multi-scenario test methods used in the three request testing 
classes here would be clearer if each scenario was split into its own test 
case, like `testRewriteWhenKeyDeleted`, `testRewriteWhenKeyUpdated`, 
`testRewritePreservesMetadata` etc. These smaller units make it easier to see 
what is actually being tested, and what scenarios may have been missed.
   
   That said there's technically nothing wrong with the way it is implemented 
here so you can disregard this if you think there would be code duplication in 
test setup or other factors that make this suboptimal.



-- 
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]

Reply via email to