github-actions[bot] commented on code in PR #17246:
URL: https://github.com/apache/doris/pull/17246#discussion_r1124103374
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -40,22 +44,141 @@
static const std::string kTestDir = "./ut_dir/tablet_cooldown_test";
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+static constexpr int64_t kTabletId = 10005;
+static constexpr int64_t kReplicaId = 10009;
+static constexpr int64_t kSchemaHash = 270068377;
+
+using io::Path;
+
+static io::FileSystemSPtr s_fs;
+
+static std::string get_remote_path(const Path& path) {
+ return fmt::format("{}/remote/{}", config::storage_root_path,
path.string());
+}
-// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+public:
+ class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : io::FileWriter(std::move(path)) {
+ io::global_local_filesystem()->create_file(get_remote_path(_path),
&_local_file_writer);
+ }
+
+ ~FileWriterMock() = default;
+
+ Status close() override {
+ return _local_file_writer->close();
+ }
+
+ Status abort() override {
+ return _local_file_writer->abort();
+ }
+
+ Status append(const Slice& data) override {
+ return _local_file_writer->append(data);
+ }
+
+ Status appendv(const Slice* data, size_t data_cnt) override {
+ return _local_file_writer->appendv(data, data_cnt);
+ }
+
+ Status write_at(size_t offset, const Slice& data) override {
+ return _local_file_writer->write_at(offset, data);
+ }
+
+ Status finalize() override {
+ return _local_file_writer->finalize();
+ }
+
+ size_t bytes_appended() const override { return
_local_file_writer->bytes_appended(); }
+
+ io::FileSystemSPtr fs() const override { return s_fs; }
+ private:
+ std::unique_ptr<io::FileWriter> _local_file_writer;
+ };
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type) {
+ _local_fs =
io::LocalFileSystem::create(get_remote_path(_root_path));
+ }
+ ~RemoteFileSystemMock() = default;
+
+ Status create_file(const Path& path, io::FileWriterPtr* writer)
override {
+ Path fs_path = path;
+ *writer = std::make_unique<FileWriterMock>(fs_path);
+ return Status::OK();
+ }
+
+ Status open_file(const Path& path, io::FileReaderSPtr* reader,
IOContext* io_ctx) override {
+ return _local_fs->open_file(get_remote_path(path), reader, io_ctx);
+ }
+
+ Status delete_file(const Path& path) override {
+ return _local_fs->delete_file(get_remote_path(path));
+ }
+
+ Status create_directory(const Path& path) override {
+ return _local_fs->create_directory(get_remote_path(path));
+ }
+
+ Status delete_directory(const Path& path) override {
+ return _local_fs->delete_directory(get_remote_path(path));
+ }
+
+ Status link_file(const Path& src, const Path& dest) override {
+ return _local_fs->link_file(get_remote_path(src),
get_remote_path(dest));
+ }
+
+ Status exists(const Path& path, bool* res) const override {
+ return _local_fs->exists(get_remote_path(path), res);
+ }
+
+ Status file_size(const Path& path, size_t* file_size) const override {
+ return _local_fs->file_size(get_remote_path(path), file_size);
+ }
+
+ Status list(const Path& path, std::vector<Path>* files) override {
+ std::vector<Path> local_paths;
+ RETURN_IF_ERROR(_local_fs->list(get_remote_path(path),
&local_paths));
+ for (Path path : local_paths) {
+
files->emplace_back(path.string().substr(config::storage_root_path.size() + 1));
+ }
+ return Status::OK();
+ }
+
+ Status upload(const Path& local_path, const Path& dest_path) override {
+ return _local_fs->link_file(local_path.string(),
get_remote_path(dest_path));
+ }
+
+ Status batch_upload(const std::vector<Path>& local_paths,
+ const std::vector<Path>& dest_paths) override {
+ for (int i = 0; i < local_paths.size(); ++i) {
+ RETURN_IF_ERROR(upload(local_paths[i], dest_paths[i]));
+ }
+ return Status::OK();
+ }
+
+ Status batch_delete(const std::vector<Path>& paths) override {
+ for (int i = 0; i < paths.size(); ++i) {
+ RETURN_IF_ERROR(delete_file(paths[i]));
+ }
+ return Status::OK();
+ }
+
+ Status connect() override {
+ return Status::OK();
+ }
+ private:
+ std::shared_ptr<io::LocalFileSystem> _local_fs;
+ };
+
public:
static void SetUpTestSuite() {
- S3Conf s3_conf;
- s3_conf.ak = config::test_s3_ak;
- s3_conf.sk = config::test_s3_sk;
- s3_conf.endpoint = config::test_s3_endpoint;
- s3_conf.region = config::test_s3_region;
- s3_conf.bucket = config::test_s3_bucket;
- s3_conf.prefix = config::test_s3_prefix + "/tablet_cooldown_test";
- auto s3_fs = io::S3FileSystem::create(std::move(s3_conf),
std::to_string(kResourceId));
- ASSERT_TRUE(s3_fs->connect().ok());
- put_storage_resource(kResourceId, {s3_fs, 1});
+ s_fs.reset(new RemoteFileSystemMock("test_path",
std::to_string(kResourceId),
Review Comment:
warning: calling a private constructor of class
'doris::TabletCooldownTest::RemoteFileSystemMock' [clang-diagnostic-error]
```cpp
s_fs.reset(new RemoteFileSystemMock("test_path",
std::to_string(kResourceId),
^
```
**be/test/olap/tablet_cooldown_test.cpp:100:** implicitly declared private
here
```cpp
RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
^
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -40,22 +44,141 @@
static const std::string kTestDir = "./ut_dir/tablet_cooldown_test";
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+static constexpr int64_t kTabletId = 10005;
+static constexpr int64_t kReplicaId = 10009;
+static constexpr int64_t kSchemaHash = 270068377;
+
+using io::Path;
+
+static io::FileSystemSPtr s_fs;
+
+static std::string get_remote_path(const Path& path) {
+ return fmt::format("{}/remote/{}", config::storage_root_path,
path.string());
+}
-// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+public:
+ class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : io::FileWriter(std::move(path)) {
+ io::global_local_filesystem()->create_file(get_remote_path(_path),
&_local_file_writer);
+ }
+
+ ~FileWriterMock() = default;
+
+ Status close() override {
+ return _local_file_writer->close();
+ }
+
+ Status abort() override {
+ return _local_file_writer->abort();
+ }
+
+ Status append(const Slice& data) override {
+ return _local_file_writer->append(data);
+ }
+
+ Status appendv(const Slice* data, size_t data_cnt) override {
+ return _local_file_writer->appendv(data, data_cnt);
+ }
+
+ Status write_at(size_t offset, const Slice& data) override {
+ return _local_file_writer->write_at(offset, data);
+ }
+
+ Status finalize() override {
+ return _local_file_writer->finalize();
+ }
+
+ size_t bytes_appended() const override { return
_local_file_writer->bytes_appended(); }
+
+ io::FileSystemSPtr fs() const override { return s_fs; }
+ private:
+ std::unique_ptr<io::FileWriter> _local_file_writer;
+ };
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type) {
+ _local_fs =
io::LocalFileSystem::create(get_remote_path(_root_path));
+ }
+ ~RemoteFileSystemMock() = default;
+
+ Status create_file(const Path& path, io::FileWriterPtr* writer)
override {
+ Path fs_path = path;
+ *writer = std::make_unique<FileWriterMock>(fs_path);
+ return Status::OK();
+ }
+
+ Status open_file(const Path& path, io::FileReaderSPtr* reader,
IOContext* io_ctx) override {
+ return _local_fs->open_file(get_remote_path(path), reader, io_ctx);
+ }
+
+ Status delete_file(const Path& path) override {
+ return _local_fs->delete_file(get_remote_path(path));
+ }
+
+ Status create_directory(const Path& path) override {
+ return _local_fs->create_directory(get_remote_path(path));
+ }
+
+ Status delete_directory(const Path& path) override {
+ return _local_fs->delete_directory(get_remote_path(path));
+ }
+
+ Status link_file(const Path& src, const Path& dest) override {
+ return _local_fs->link_file(get_remote_path(src),
get_remote_path(dest));
+ }
+
+ Status exists(const Path& path, bool* res) const override {
+ return _local_fs->exists(get_remote_path(path), res);
+ }
+
+ Status file_size(const Path& path, size_t* file_size) const override {
+ return _local_fs->file_size(get_remote_path(path), file_size);
+ }
+
+ Status list(const Path& path, std::vector<Path>* files) override {
+ std::vector<Path> local_paths;
+ RETURN_IF_ERROR(_local_fs->list(get_remote_path(path),
&local_paths));
+ for (Path path : local_paths) {
+
files->emplace_back(path.string().substr(config::storage_root_path.size() + 1));
+ }
+ return Status::OK();
+ }
+
+ Status upload(const Path& local_path, const Path& dest_path) override {
+ return _local_fs->link_file(local_path.string(),
get_remote_path(dest_path));
+ }
+
+ Status batch_upload(const std::vector<Path>& local_paths,
+ const std::vector<Path>& dest_paths) override {
+ for (int i = 0; i < local_paths.size(); ++i) {
+ RETURN_IF_ERROR(upload(local_paths[i], dest_paths[i]));
+ }
+ return Status::OK();
+ }
+
+ Status batch_delete(const std::vector<Path>& paths) override {
+ for (int i = 0; i < paths.size(); ++i) {
+ RETURN_IF_ERROR(delete_file(paths[i]));
+ }
+ return Status::OK();
+ }
+
+ Status connect() override {
+ return Status::OK();
+ }
+ private:
+ std::shared_ptr<io::LocalFileSystem> _local_fs;
+ };
+
public:
Review Comment:
warning: redundant access specifier has the same accessibility as the
previous access specifier [readability-redundant-access-specifiers]
```suggestion
```
**be/test/olap/tablet_cooldown_test.cpp:59:** previously declared here
```cpp
public:
^
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -40,22 +44,141 @@
static const std::string kTestDir = "./ut_dir/tablet_cooldown_test";
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+static constexpr int64_t kTabletId = 10005;
+static constexpr int64_t kReplicaId = 10009;
+static constexpr int64_t kSchemaHash = 270068377;
+
+using io::Path;
+
+static io::FileSystemSPtr s_fs;
+
+static std::string get_remote_path(const Path& path) {
+ return fmt::format("{}/remote/{}", config::storage_root_path,
path.string());
+}
-// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+public:
+ class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : io::FileWriter(std::move(path)) {
+ io::global_local_filesystem()->create_file(get_remote_path(_path),
&_local_file_writer);
+ }
+
+ ~FileWriterMock() = default;
+
+ Status close() override {
+ return _local_file_writer->close();
+ }
+
+ Status abort() override {
+ return _local_file_writer->abort();
+ }
+
+ Status append(const Slice& data) override {
+ return _local_file_writer->append(data);
+ }
+
+ Status appendv(const Slice* data, size_t data_cnt) override {
+ return _local_file_writer->appendv(data, data_cnt);
+ }
+
+ Status write_at(size_t offset, const Slice& data) override {
+ return _local_file_writer->write_at(offset, data);
+ }
+
+ Status finalize() override {
+ return _local_file_writer->finalize();
+ }
+
+ size_t bytes_appended() const override { return
_local_file_writer->bytes_appended(); }
+
+ io::FileSystemSPtr fs() const override { return s_fs; }
+ private:
+ std::unique_ptr<io::FileWriter> _local_file_writer;
+ };
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type) {
+ _local_fs =
io::LocalFileSystem::create(get_remote_path(_root_path));
+ }
+ ~RemoteFileSystemMock() = default;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final'
[modernize-use-override]
```suggestion
~RemoteFileSystemMock() override = default;
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -40,22 +44,141 @@ static StorageEngine* k_engine = nullptr;
static const std::string kTestDir = "./ut_dir/tablet_cooldown_test";
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+static constexpr int64_t kTabletId = 10005;
+static constexpr int64_t kReplicaId = 10009;
+static constexpr int64_t kSchemaHash = 270068377;
+
+using io::Path;
+
+static io::FileSystemSPtr s_fs;
+
+static std::string get_remote_path(const Path& path) {
+ return fmt::format("{}/remote/{}", config::storage_root_path,
path.string());
+}
-// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+public:
+ class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : io::FileWriter(std::move(path)) {
+ io::global_local_filesystem()->create_file(get_remote_path(_path),
&_local_file_writer);
+ }
+
+ ~FileWriterMock() = default;
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final'
[modernize-use-override]
```suggestion
~FileWriterMock() override = default;
```
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
