anuragp010 commented on code in PR #11066:
URL: https://github.com/apache/ozone/pull/11066#discussion_r3832220347


##########
hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestListInfoSubcommand.java:
##########
@@ -459,6 +466,54 @@ private void validateOrderingFromTextOutput(String output, 
String orderDirection
     }
   }
 
+  @ParameterizedTest
+  @EnumSource(value = StorageTier.class, names = {"EMPTY"}, mode = 
EnumSource.Mode.EXCLUDE)

Review Comment:
   @rich7420 , thanks for taking a look !
   
   I initially left `StorageTier.EMPTY` out of 
`testRelatedPipelineShowsStorageTier` because it shouldn't be reachable on the 
client side. Specifically because:
   1. Pipeline creation prevents it (`RatisPipelineProvider` and 
`SimplePipelineProvider` use `StorageTierUtil.validateNotEmpty`).
   2. Serialization would fail (`listPipelines` calling 
`pipeline.getProtobufMessage()` in 
`StorageContainerLocationProtocolServerSideTranslatorPB` triggers 
`StorageTier.toProto()`, which throws an `IllegalStateException` for `EMPTY`).
   
   Given that it's an unreachable state, do you think it's still worth adding 
handling and a test case for `EMPTY`?



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