simbadzina commented on PR #5145: URL: https://github.com/apache/hadoop/pull/5145#issuecomment-1320761423
> Thanks @simbadzina for your contribution. > > @goiri - Do you think we can add a specific UT for this change? Also, I can see the current test has been updated to check it. > > I am +1 overall. Thanks for the reviews @ashutoshcipher and @goiri . In this review I didn't update tests. I spent some time trying to add a test but I couldn't find an elegant way to do use. I had two ideas 1) Adding a fault injector to StateStoreFileBaseImpl . And then use the injector to throw an en exception during the write of the file. 2) Mocking StateStoreFileSystemImpl and throwing an exception when the serializeString(..) method is called. (1) felt like overkill. I couldn't get (2) to work because serializeString() is a protected method in a parent class. If you have any thoughts on a test we can add I'd greatly appreciate pointers. -- 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]
