nastra commented on PR #14378: URL: https://github.com/apache/iceberg/pull/14378#issuecomment-3425020577
> Hey @nastra , Thanks for taking a look! In my opinion the unit tests for `TestFileIOTracker` are required but not entirely enough. There is untested code in RESTSessionCatalog where we have no detection of a regression if someone messes up the usage of the tracker on the catalog side. I was working on this areas of the code for the client-side table cache for the freshness-aware table loading and I had a false impression that everything is fine with my ongoing changes because the tests passed, however, I did it wrong with the tracker and was surprised there are no test coverage that detects this. Also, with my mentioned freshness loading change I'd like to add further tests relevant for IO tracking together with the table cache and this PR is the first step for that. Hope this makes sense! I'm currently undecided on this, so let's see what others think -- 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]
