Skip to content

[refactor] Unify all unit tests into one binary file #8958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yiguolei merged 1 commit into from
Apr 12, 2022
Merged

[refactor] Unify all unit tests into one binary file #8958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
49 changes: 1 addition & 48 deletions be/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,12 +534,6 @@ set(DORIS_LINK_LIBS
Vec
${WL_END_GROUP}
)
if (${MAKE_TEST} STREQUAL "ON")
set(DORIS_LINK_LIBS
${DORIS_LINK_LIBS}
TestUtil
)
endif()


# COMMON_THIRDPARTY are thirdparty dependencies that can run on all platform
Expand Down Expand Up @@ -685,10 +679,8 @@ set(DORIS_LINK_LIBS ${DORIS_LINK_LIBS}
# Set libraries for test
set (TEST_LINK_LIBS ${DORIS_LINK_LIBS}
${WL_START_GROUP}
Test_util
gmock
gtest
gtest_main
${WL_END_GROUP}
)

Expand All @@ -699,7 +691,6 @@ if (${MAKE_TEST} STREQUAL "ON")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage -DGTEST_USE_OWN_TR1_TUPLE=0")
SET(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-arcs -ftest-coverage -lgcov")
add_definitions(-DBE_TEST)
add_subdirectory(${SRC_DIR}/testutil)
endif ()

add_subdirectory(${SRC_DIR}/agent)
Expand All @@ -723,46 +714,8 @@ endif()
add_subdirectory(${SRC_DIR}/util)
add_subdirectory(${SRC_DIR}/vec)

# Utility CMake function to make specifying tests and benchmarks less verbose
FUNCTION(ADD_BE_TEST TEST_NAME)
# use argn to add additional files
set(ADDITIONAL_FILES ${ARGN})
set(BUILD_OUTPUT_ROOT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/")
# This gets the directory where the test is from (e.g. 'exprs' or 'runtime')
get_filename_component(DIR_NAME ${CMAKE_CURRENT_SOURCE_DIR} NAME)
get_filename_component(TEST_DIR_NAME ${TEST_NAME} PATH)
get_filename_component(TEST_FILE_NAME ${TEST_NAME} NAME)

ADD_EXECUTABLE(${TEST_FILE_NAME} ${TEST_NAME}.cpp ${ADDITIONAL_FILES})
TARGET_LINK_LIBRARIES(${TEST_FILE_NAME} ${TEST_LINK_LIBS})
SET_TARGET_PROPERTIES(${TEST_FILE_NAME} PROPERTIES COMPILE_FLAGS "-fno-access-control" ENABLE_EXPORTS 1)
if (NOT "${TEST_DIR_NAME}" STREQUAL "")
SET_TARGET_PROPERTIES(${TEST_FILE_NAME} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}/${TEST_DIR_NAME}")
endif()
ADD_TEST(${TEST_FILE_NAME} "${BUILD_OUTPUT_ROOT_DIRECTORY}/${TEST_NAME}")
ENDFUNCTION()

if (${MAKE_TEST} STREQUAL "ON")
add_subdirectory(${TEST_DIR}/test_util)
add_subdirectory(${TEST_DIR}/agent)
add_subdirectory(${TEST_DIR}/common)
add_subdirectory(${TEST_DIR}/env)
add_subdirectory(${TEST_DIR}/exec)
add_subdirectory(${TEST_DIR}/exprs)
add_subdirectory(${TEST_DIR}/geo)
add_subdirectory(${TEST_DIR}/gutil)
add_subdirectory(${TEST_DIR}/http)
add_subdirectory(${TEST_DIR}/olap)
add_subdirectory(${TEST_DIR}/runtime)
add_subdirectory(${TEST_DIR}/udf)
add_subdirectory(${TEST_DIR}/util)
add_subdirectory(${TEST_DIR}/vec/core)
add_subdirectory(${TEST_DIR}/vec/exec)
add_subdirectory(${TEST_DIR}/vec/exprs)
add_subdirectory(${TEST_DIR}/vec/function)
add_subdirectory(${TEST_DIR}/vec/runtime)
add_subdirectory(${TEST_DIR}/vec/aggregate_functions)
add_subdirectory(${TEST_DIR}/tools)
add_subdirectory(${TEST_DIR})
endif ()

# Install be
Expand Down
2 changes: 1 addition & 1 deletion be/src/exprs/cast_functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
#include "runtime/datetime_value.h"
#include "runtime/string_value.h"
#include "string_functions.h"
#include "util/array_parser.hpp"
#include "util/array_parser.h"
#include "util/mysql_global.h"
#include "util/string_parser.hpp"

Expand Down
12 changes: 6 additions & 6 deletions be/src/geo/geo_functions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,8 @@ void GeoFunctions::st_contains_prepare(doris_udf::FunctionContext* ctx,
if (str->is_null) {
contains_ctx->is_null = true;
} else {
contains_ctx->shapes[i] = GeoShape::from_encoded(str->ptr, str->len);
contains_ctx->shapes[i] =
std::shared_ptr<GeoShape>(GeoShape::from_encoded(str->ptr, str->len));
Copy link
Copy Markdown
Contributor

@morningman morningman Apr 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using shared_ptr instead of unique_ptr?

Copy link
Copy Markdown
Member Author

@yangzhg yangzhg Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shapes may share at line 275

if (contains_ctx->shapes[i] == nullptr) {
contains_ctx->is_null = true;
}
Expand Down Expand Up @@ -267,22 +268,21 @@ doris_udf::BooleanVal GeoFunctions::st_contains(doris_udf::FunctionContext* ctx,
if (state != nullptr && state->is_null) {
return BooleanVal::null();
}
GeoShape* shapes[2] = {nullptr, nullptr};
std::vector<std::shared_ptr<GeoShape>> shapes = {nullptr, nullptr};
const StringVal* strs[2] = {&lhs, &rhs};
// use this to delete new
StContainsState local_state;
for (int i = 0; i < 2; ++i) {
if (state != nullptr && state->shapes[i] != nullptr) {
shapes[i] = state->shapes[i];
} else {
shapes[i] = local_state.shapes[i] = GeoShape::from_encoded(strs[i]->ptr, strs[i]->len);
shapes[i] =
std::shared_ptr<GeoShape>(GeoShape::from_encoded(strs[i]->ptr, strs[i]->len));
if (shapes[i] == nullptr) {
return BooleanVal::null();
}
}
}

return shapes[0]->contains(shapes[1]);
return shapes[0]->contains(shapes[1].get());
Comment thread
yiguolei marked this conversation as resolved.
}

} // namespace doris
10 changes: 3 additions & 7 deletions be/src/geo/geo_functions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,11 @@ struct StConstructState {
std::string encoded_buf;
};


struct StContainsState {
StContainsState() : is_null(false), shapes{nullptr, nullptr} {}
~StContainsState() {
delete shapes[0];
delete shapes[1];
}
StContainsState() : is_null(false), shapes {nullptr, nullptr} {}
~StContainsState() {}
bool is_null;
GeoShape* shapes[2];
std::vector<std::shared_ptr<GeoShape>> shapes;
};

} // namespace doris
17 changes: 8 additions & 9 deletions be/src/olap/page_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,21 @@ StoragePageCache::StoragePageCache(size_t capacity, int32_t index_cache_percenta
MemTrackerLevel::OVERVIEW)) {
SCOPED_SWITCH_THREAD_LOCAL_MEM_TRACKER(_mem_tracker);
if (index_cache_percentage == 0) {
_data_page_cache =
std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
_data_page_cache = std::unique_ptr<Cache>(new_lru_cache("DataPageCache", capacity));
} else if (index_cache_percentage == 100) {
_index_page_cache =
std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
_index_page_cache = std::unique_ptr<Cache>(new_lru_cache("IndexPageCache", capacity));
} else if (index_cache_percentage > 0 && index_cache_percentage < 100) {
_data_page_cache = std::unique_ptr<Cache>(new_lru_cache(
"DataPageCache", capacity * (100 - index_cache_percentage) / 100));
_index_page_cache = std::unique_ptr<Cache>(new_lru_cache(
"IndexPageCache", capacity * index_cache_percentage / 100));
_data_page_cache = std::unique_ptr<Cache>(
new_lru_cache("DataPageCache", capacity * (100 - index_cache_percentage) / 100));
_index_page_cache = std::unique_ptr<Cache>(
new_lru_cache("IndexPageCache", capacity * index_cache_percentage / 100));
} else {
CHECK(false) << "invalid index page cache percentage";
}
}

bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle, segment_v2::PageTypePB page_type) {
bool StoragePageCache::lookup(const CacheKey& key, PageCacheHandle* handle,
segment_v2::PageTypePB page_type) {
auto cache = _get_page_cache(page_type);
auto lru_handle = cache->lookup(key.encode());
if (lru_handle == nullptr) {
Expand Down
11 changes: 5 additions & 6 deletions be/src/olap/page_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@
#include <string>
#include <utility>

#include "gutil/macros.h" // for DISALLOW_COPY_AND_ASSIGN
#include "olap/lru_cache.h"
#include "gen_cpp/segment_v2.pb.h" // for cache allocation
#include "gutil/macros.h" // for DISALLOW_COPY_AND_ASSIGN
#include "olap/lru_cache.h"
#include "runtime/mem_tracker.h"


namespace doris {

class PageCacheHandle;
Expand Down Expand Up @@ -100,10 +99,10 @@ class StoragePageCache {
std::shared_ptr<MemTracker> _mem_tracker = nullptr;

Cache* _get_page_cache(segment_v2::PageTypePB page_type) {
switch (page_type)
{
case segment_v2::DATA_PAGE:
switch (page_type) {
case segment_v2::DATA_PAGE: {
return _data_page_cache.get();
}
case segment_v2::INDEX_PAGE:
return _index_page_cache.get();
default:
Expand Down
26 changes: 13 additions & 13 deletions be/src/olap/storage_engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ StorageEngine::StorageEngine(const EngineOptions& options)
_default_rowset_type(ALPHA_ROWSET),
_heartbeat_flags(nullptr),
_stream_load_recorder(nullptr) {
if (_s_instance == nullptr) {
_s_instance = this;
}
_s_instance = this;
REGISTER_HOOK_METRIC(unused_rowsets_count, [this]() {
std::lock_guard<std::mutex> lock(_gc_mutex);
return _unused_rowsets.size();
Expand Down Expand Up @@ -580,11 +578,11 @@ void StorageEngine::stop() {
THREAD_JOIN(_tablet_checkpoint_tasks_producer_thread);
#undef THREAD_JOIN

#define THREADS_JOIN(threads) \
for (const auto& thread : threads) {\
if (thread) { \
thread->join(); \
} \
#define THREADS_JOIN(threads) \
for (const auto& thread : threads) { \
if (thread) { \
thread->join(); \
} \
}

THREADS_JOIN(_path_gc_threads);
Expand Down Expand Up @@ -745,8 +743,8 @@ void StorageEngine::_clean_unused_rowset_metas() {
// tablet may be dropped
// TODO(cmy): this is better to be a VLOG, because drop table is a very common case.
// leave it as INFO log for observation. Maybe change it in future.
LOG(INFO) << "failed to find tablet " << rowset_meta->tablet_id() << " for rowset: " << rowset_meta->rowset_id()
<< ", tablet may be dropped";
LOG(INFO) << "failed to find tablet " << rowset_meta->tablet_id()
<< " for rowset: " << rowset_meta->rowset_id() << ", tablet may be dropped";
invalid_rowset_metas.push_back(rowset_meta);
return true;
}
Expand All @@ -759,7 +757,8 @@ void StorageEngine::_clean_unused_rowset_metas() {
// which will creates a new tablet with the same id but a different uid.
// And in the historical version, when we deleted the replica, we did not delete the corresponding rowset meta,
// thus causing the original rowset meta to remain(with same tablet id but different uid).
LOG(WARNING) << "rowset's tablet uid " << rowset_meta->tablet_uid() << " does not equal to tablet uid: " << tablet->tablet_uid();
LOG(WARNING) << "rowset's tablet uid " << rowset_meta->tablet_uid()
<< " does not equal to tablet uid: " << tablet->tablet_uid();
invalid_rowset_metas.push_back(rowset_meta);
return true;
}
Expand All @@ -779,9 +778,10 @@ void StorageEngine::_clean_unused_rowset_metas() {
RowsetMetaManager::traverse_rowset_metas(data_dir->get_meta(), clean_rowset_func);
for (auto& rowset_meta : invalid_rowset_metas) {
RowsetMetaManager::remove(data_dir->get_meta(), rowset_meta->tablet_uid(),
rowset_meta->rowset_id());
rowset_meta->rowset_id());
}
LOG(INFO) << "remove " << invalid_rowset_metas.size() << " invalid rowset meta from dir: " << data_dir->path();
LOG(INFO) << "remove " << invalid_rowset_metas.size()
<< " invalid rowset meta from dir: " << data_dir->path();
invalid_rowset_metas.clear();
}
}
Expand Down
33 changes: 19 additions & 14 deletions be/src/runtime/user_function_cache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

#include "runtime/user_function_cache.h"

#include <atomic>
#include <boost/algorithm/string/classification.hpp> // boost::is_any_of
#include <boost/algorithm/string/predicate.hpp> // boost::algorithm::ends_with
#include <atomic>
#include <regex>
#include <vector>

Expand Down Expand Up @@ -56,11 +56,11 @@ struct UserFunctionCacheEntry {
std::string lib_file;

// make it atomic variable instead of holding a lock
std::atomic<bool> is_loaded{false};
std::atomic<bool> is_loaded {false};

// Set to true when this library is not needed.
// e.g. deleting some unused library to re
std::atomic<bool> should_delete_library{false};
std::atomic<bool> should_delete_library {false};

// lock to make sure only one can load this cache
std::mutex load_lock;
Expand All @@ -80,7 +80,7 @@ struct UserFunctionCacheEntry {
LibType type;

private:
std::atomic<int> _refs{0};
std::atomic<int> _refs {0};
};

UserFunctionCacheEntry::~UserFunctionCacheEntry() {
Expand Down Expand Up @@ -116,7 +116,10 @@ UserFunctionCache* UserFunctionCache::instance() {
}

Status UserFunctionCache::init(const std::string& lib_dir) {
DCHECK(_lib_dir.empty());
#ifndef BE_TEST
// _lib_dir may reuesd bettween unit tests
DCHECK(_lib_dir.empty()) << _lib_dir;
#endif
_lib_dir = lib_dir;
// 1. dynamic open current process
RETURN_IF_ERROR(dynamic_open(nullptr, &_current_process_handle));
Expand Down Expand Up @@ -241,16 +244,16 @@ Status UserFunctionCache::get_function_ptr(int64_t fid, const std::string& orig_

Status UserFunctionCache::_get_cache_entry(int64_t fid, const std::string& url,
const std::string& checksum,
UserFunctionCacheEntry** output_entry,
LibType type) {
UserFunctionCacheEntry** output_entry, LibType type) {
UserFunctionCacheEntry* entry = nullptr;
{
std::lock_guard<std::mutex> l(_cache_lock);
auto it = _entry_map.find(fid);
if (it != _entry_map.end()) {
entry = it->second;
} else {
entry = new UserFunctionCacheEntry(fid, checksum, _make_lib_file(fid, checksum, type), type);
entry = new UserFunctionCacheEntry(fid, checksum, _make_lib_file(fid, checksum, type),
type);

entry->ref();
_entry_map.emplace(fid, entry);
Expand Down Expand Up @@ -375,11 +378,13 @@ Status UserFunctionCache::_add_to_classpath(UserFunctionCacheEntry* entry) {
JNIEnv* env;
RETURN_IF_ERROR(JniUtil::GetJNIEnv(&env));
jclass class_class_loader = env->FindClass("java/lang/ClassLoader");
jmethodID method_get_system_class_loader =
env->GetStaticMethodID(class_class_loader, "getSystemClassLoader", "()Ljava/lang/ClassLoader;");
jobject class_loader = env->CallStaticObjectMethod(class_class_loader, method_get_system_class_loader);
jmethodID method_get_system_class_loader = env->GetStaticMethodID(
class_class_loader, "getSystemClassLoader", "()Ljava/lang/ClassLoader;");
jobject class_loader =
env->CallStaticObjectMethod(class_class_loader, method_get_system_class_loader);
jclass class_url_class_loader = env->FindClass("java/net/URLClassLoader");
jmethodID method_add_url = env->GetMethodID(class_url_class_loader, "addURL", "(Ljava/net/URL;)V");
jmethodID method_add_url =
env->GetMethodID(class_url_class_loader, "addURL", "(Ljava/net/URL;)V");
jclass class_url = env->FindClass("java/net/URL");
jmethodID url_ctor = env->GetMethodID(class_url, "<init>", "(Ljava/lang/String;)V");
jobject urlInstance = env->NewObject(class_url, url_ctor, env->NewStringUTF(path.c_str()));
Expand Down Expand Up @@ -412,8 +417,8 @@ void UserFunctionCache::release_entry(UserFunctionCacheEntry* entry) {
}
}

Status UserFunctionCache::get_jarpath(int64_t fid, const std::string& url, const std::string& checksum,
std::string* libpath) {
Status UserFunctionCache::get_jarpath(int64_t fid, const std::string& url,
const std::string& checksum, std::string* libpath) {
UserFunctionCacheEntry* entry = nullptr;
RETURN_IF_ERROR(_get_cache_entry(fid, url, checksum, &entry, LibType::JAR));
*libpath = entry->lib_file;
Expand Down
28 changes: 0 additions & 28 deletions be/src/testutil/CMakeLists.txt
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions be/src/util/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ set(UTIL_FILES
arrow/row_batch.cpp
arrow/row_block.cpp
arrow/utils.cpp
array_parser.cpp
bfd_parser.cpp
bitmap.cpp
block_compression.cpp
Expand Down
Loading