[GitHub] [ozone] duongkame commented on pull request #4212: HDDS-7829. SCM to reject adding container to closed pipeline

["duongkame (via GitHub)" <gi...@apache.org>]

duongkame commented on PR #4212:
URL: https://github.com/apache/ozone/pull/4212#issuecomment-1421505742

   > Following up on the testing part of this patch, the unit test is good, but it does not test the changes to `SCMStateMachine#applyTransaction`. It looks like this could be done by modifying `TestPipelineClose#testPipelineCloseWithClosedContainer` to use 1 node Ratis for SCM, then calling `pipelineManager#addContainerToPipeline` on the closed pipeline and check that an exception is thrown.
   
   @errose28, thanks to your suggestion, I was able to create a proper integration test for the failure scenario.
   
   > I don't see a way to test that generic IOExceptions or SCMExceptions with fatal result codes terminate the SCM without failing the test due to JVM exit.
   
   I believe that can be resorted by `ExitUtils.disableSystemExit`, which prevents actual JVM exit for testing purposes.
   Yet, I scanned the code and could not find a good way to produce internal errors, maybe we can somehow generate `TimeoutException` and SCM always translates it to internal errors. 
   
   


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