API: Add tests for CharSequenceUtil - #17890
Conversation
CharSequenceUtil.unequalPaths had no unit coverage even though it is used on the position delete write path in FileScopedPositionDeleteWriter. Add tests covering the same-reference and equal-content fast paths, differing lengths, differences at the first and last characters, and empty inputs.
|
Hi @huaxingao @szehon-ho |
| // paths are compared from the end, so a trailing difference is found immediately | ||
| assertThat(CharSequenceUtil.unequalPaths("data/file-1.parquet", "data/file-2.parquet")) | ||
| .isTrue(); |
There was a problem hiding this comment.
a trailing difference
I can understand the intention, but the actual difference (1 vs 2) exists in the middle of the path. The unequalPaths method doesn't care about the separator. Also, the test name looks misleading.
I suggest adding another case where the last character is distinctly different. This way, we can cover three cases: the first, middle, and last character.
There was a problem hiding this comment.
Thanks @ebyhr — you're right that it's a middle-of-string difference. I've renamed that case to ...WhenAMiddleCharacterDiffers, fixed the comments, and added a real last-character case (data/part-00001 vs data/part-00002), so first/middle/last are all covered now. Pushed as a follow-up.
The "data/file-1.parquet" vs "data/file-2.parquet" pair differs in the middle of the string, not at the end, so rename that case accordingly and add a dedicated case where only the final character differs. The three positive cases now cover a difference at the first, a middle, and the last character.
2 participants
CharSequenceUtil.unequalPaths had no unit coverage even though it is used on the position delete write path in FileScopedPositionDeleteWriter. Add tests covering the same-reference and equal-content fast paths, differing lengths, differences at the first and last characters, and empty inputs.