github-actions[bot] commented on code in PR #17246:
URL: https://github.com/apache/doris/pull/17246#discussion_r1121518652
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@ static const std::string kTestDir =
"./ut_dir/tablet_cooldown_test";
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
~FileWriterMock() = default;
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
Review Comment:
warning: only virtual member functions can be marked 'override'
[clang-diagnostic-error]
```suggestion
Status open() {
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
+ return Status::OK();
+ }
+
+ Status write(const uint8_t *buf, size_t buf_len, size_t *written_len)
override {
+ return Status::OK();
+ }
+
+ Status close() override {
+ return Status::OK();
+ }
+ }
Review Comment:
warning: expected ';' after class [clang-diagnostic-error]
```suggestion
};
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
+ return Status::OK();
+ }
+
+ Status write(const uint8_t *buf, size_t buf_len, size_t *written_len)
override {
+ return Status::OK();
+ }
+
+ Status close() override {
+ return Status::OK();
+ }
+ }
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type)
{}
+ ~RemoteFileSystemMock() override {}
+
+ Status create_file(const Path& path, io::FileWriterPtr* writer)
override {
+ writer->reset(new FileWriterMock());
Review Comment:
warning: allocating an object of abstract class type
'doris::TabletCooldownTest::FileWriterMock' [clang-diagnostic-error]
```cpp
writer->reset(new FileWriterMock());
^
```
**be/src/io/fs/file_writer.h:41:** unimplemented pure virtual method 'abort'
in 'FileWriterMock'
```cpp
virtual Status abort() = 0;
^
```
**be/src/io/fs/file_writer.h:43:** unimplemented pure virtual method
'append' in 'FileWriterMock'
```cpp
virtual Status append(const Slice& data) = 0;
^
```
**be/src/io/fs/file_writer.h:45:** unimplemented pure virtual method
'appendv' in 'FileWriterMock'
```cpp
virtual Status appendv(const Slice* data, size_t data_cnt) = 0;
^
```
**be/src/io/fs/file_writer.h:47:** unimplemented pure virtual method
'write_at' in 'FileWriterMock'
```cpp
virtual Status write_at(size_t offset, const Slice& data) = 0;
^
```
**be/src/io/fs/file_writer.h:51:** unimplemented pure virtual method
'finalize' in 'FileWriterMock'
```cpp
virtual Status finalize() = 0;
^
```
**be/src/io/fs/file_writer.h:53:** unimplemented pure virtual method
'bytes_appended' in 'FileWriterMock'
```cpp
virtual size_t bytes_appended() const = 0;
^
```
**be/src/io/fs/file_writer.h:55:** unimplemented pure virtual method 'fs' in
'FileWriterMock'
```cpp
virtual std::shared_ptr<FileSystem> fs() const = 0;
^
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
Review Comment:
warning: annotate this function with 'override' or (rarely) 'final'
[modernize-use-override]
```suggestion
~FileWriterMock() override {}
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
+ return Status::OK();
+ }
+
+ Status write(const uint8_t *buf, size_t buf_len, size_t *written_len)
override {
Review Comment:
warning: only virtual member functions can be marked 'override'
[clang-diagnostic-error]
```suggestion
Status write(const uint8_t *buf, size_t buf_len, size_t
*written_len) {
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
+ return Status::OK();
+ }
+
+ Status write(const uint8_t *buf, size_t buf_len, size_t *written_len)
override {
+ return Status::OK();
+ }
+
+ Status close() override {
+ return Status::OK();
+ }
+ }
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type)
{}
+ ~RemoteFileSystemMock() override {}
+
+ Status create_file(const Path& path, io::FileWriterPtr* writer)
override {
+ writer->reset(new FileWriterMock());
+ return Status::OK();
+ }
+
+ Status open_file(const Path& path, io::FileReaderSPtr* reader,
IOContext* io_ctx) override {
+ return Status::OK();
+ }
+
+ Status delete_file(const Path& path) override {
+ return Status::OK();
+ }
+
+ Status create_directory(const Path& path) override {
+ return Status::OK();
+ }
+
+ Status delete_directory(const Path& path) override {
+ return Status::OK();
+ }
+
+ Status link_file(const Path& src, const Path& dest) override {
+ return Status::OK();
+ }
+
+ Status exists(const Path& path, bool* res) const override {
+ return Status::OK();
+ }
+
+ Status file_size(const Path& path, size_t* file_size) const override {
+ return Status::OK();
+ }
+
+ Status list(const Path& path, std::vector<Path>* files) override {
+ return Status::OK();
+ }
+
+ Status upload(const Path& local_path, const Path& dest_path) override {
+ return Status::OK();
+ }
+
+ Status batch_upload(const std::vector<Path>& local_paths,
+ const std::vector<Path>& dest_paths) override {
+ return Status::OK();
+ }
+
+ Status batch_delete(const std::vector<Path>& paths) override {
+ return Status::OK();
+ }
+
+ Status connect() override {
+ return Status::OK();
+ }
+ };
+
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});
+ io::FileSystemSPtr s3_fs(new RemoteFileSystemMock("test_path",
std::to_string(kResourceId),
Review Comment:
warning: calling a private constructor of class
'doris::TabletCooldownTest::RemoteFileSystemMock' [clang-diagnostic-error]
```cpp
io::FileSystemSPtr s3_fs(new RemoteFileSystemMock("test_path",
std::to_string(kResourceId),
^
```
**be/test/olap/tablet_cooldown_test.cpp:69:** implicitly declared private
here
```cpp
RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
^
```
##########
be/test/olap/tablet_cooldown_test.cpp:
##########
@@ -41,21 +42,96 @@
static constexpr int64_t kResourceId = 10000;
static constexpr int64_t kStoragePolicyId = 10002;
+using io::Path;
+
// remove DISABLED_ when need run this test
-#define TabletCooldownTest DISABLED_TabletCooldownTest
+// #define TabletCooldownTest DISABLED_TabletCooldownTest
class TabletCooldownTest : public testing::Test {
+class FileWriterMock : public io::FileWriter {
+ public:
+ FileWriterMock(Path path) : FileWriter(std::move(path)) {}
+
+ ~FileWriterMock() {}
+
+ Status open() override {
+ return Status::OK();
+ }
+
+ Status write(const uint8_t *buf, size_t buf_len, size_t *written_len)
override {
+ return Status::OK();
+ }
+
+ Status close() override {
+ return Status::OK();
+ }
+ }
+
+ class RemoteFileSystemMock : public io::RemoteFileSystem {
+ RemoteFileSystemMock(Path root_path, std::string&& id,
io::FileSystemType type)
+ : RemoteFileSystem(std::move(root_path), std::move(id), type)
{}
+ ~RemoteFileSystemMock() override {}
Review Comment:
warning: use '= default' to define a trivial destructor
[modernize-use-equals-default]
```suggestion
~RemoteFileSystemMock() 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
