superdiaodiao commented on PR #17464: URL: https://github.com/apache/iceberg/pull/17464#issuecomment-5588244514
Done in 16d9b53 — and it works better than I expected, so `checkCurrentMetadataLocation` is back to `private` in `HiveViewOperations`. No internals exposed anywhere now. The count is 2, not 1, and the two call sites are exactly the ones that matter: ``` -> BaseViewOperations.current(BaseViewOperations.java:79) -> HiveViewOperations.checkCurrentMetadataLocation(HiveViewOperations.java:279) ``` The second is the one under test — reaching it at all proves the status check resolved the null metadata rather than aborting. It also still distinguishes the fix, which was my main worry about an indirect assertion. Dropping the guard gives **3** calls, because the NPE propagates and the check is retried: ``` Wanted 2 times ... But was 3 times: -> BaseViewOperations.current(...) -> HiveViewOperations.checkCurrentMetadataLocation(...) -> HiveViewOperations.checkCurrentMetadataLocation(...) ``` So `verify(spyOps, times(2)).refresh()` fails on `main` and passes with the fix, same as the direct call did, without touching visibility. ``` TestBaseMetastoreTableOperations tests=2 failures=0 TestHiveCommits tests=16 failures=0 TestHiveViewCommits tests=13 failures=0 ./gradlew spotlessCheck passed ``` Thanks for pushing on this one — the test is smaller and the production code is untouched, which is clearly the better shape. -- 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]
