lxy-9602 commented on code in PR #338:
URL: https://github.com/apache/paimon-cpp/pull/338#discussion_r4005488382
##########
src/paimon/core/utils/file_utils_test.cpp:
##########
@@ -27,6 +27,30 @@
namespace paimon::test {
+namespace {
+
+/// Counts what a listing asks of the file system, so the round trips it saves
stay saved. On a
+/// remote store an `Exists()` in front of a `ListDir()` is a trip of its own,
and the listing
+/// already answers the same question: a directory that is not there comes
back empty.
+class ListingCountingFileSystem : public LocalFileSystem {
+ public:
+ Result<bool> Exists(const std::string& path) const override {
+ ++exists_count;
+ return LocalFileSystem::Exists(path);
+ }
+
+ Status ListDir(const std::string& directory,
+ std::vector<BasicFileStatus>* file_status_list) const
override {
+ ++list_dir_count;
+ return LocalFileSystem::ListDir(directory, file_status_list);
+ }
+
+ mutable int exists_count = 0;
+ mutable int list_dir_count = 0;
+};
Review Comment:
I think these tests are somewhat redundant and too tightly coupled to the
current implementation. Even small changes to the I/O path could break it. I
would not recommend adding this kind of test just for the sake of increasing
test coverage.
--
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]
