Skip to content
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

test: Add tests for data storage #21

Merged
merged 28 commits into from
Nov 8, 2024
Merged

test: Add tests for data storage #21

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
33a1940
test: Add basic unit test structure and cmake for test
sitaowang1998 Nov 3, 2024
cc2ab9a
test: Add test source files into clang-tidy check
sitaowang1998 Nov 3, 2024
af9212b
test: Suppress clang-tidy check for catch2 generated code
sitaowang1998 Nov 4, 2024
2ae4779
Merge branch 'main' into storage_test
sitaowang1998 Nov 5, 2024
6be29b2
style: Add back quote for table and column name in sql
sitaowang1998 Nov 5, 2024
9e493bd
fix: Add parenthesis around key columns
sitaowang1998 Nov 5, 2024
469406a
test: Add basic test structure and fix table initialization sql
sitaowang1998 Nov 5, 2024
273a6e0
fix: Pass in char const* to sql::bytes constructor instead of char *.
sitaowang1998 Nov 5, 2024
521889b
fix: Add sql::ResultSet::next before accessing element
sitaowang1998 Nov 6, 2024
e1b8a63
fix: Check null value for get_data
sitaowang1998 Nov 6, 2024
d58b8b1
feat: Add remove_data in data storage and tests
sitaowang1998 Nov 6, 2024
044ce09
test: Add data with key test and fix the table creation order for for…
sitaowang1998 Nov 6, 2024
b0232a1
ci: Add test files into lint and format the test files
sitaowang1998 Nov 6, 2024
eedaab0
Merge branch 'main' into datastorage_test
sitaowang1998 Nov 6, 2024
52a1e17
fix: Fix typo in quote in sql
sitaowang1998 Nov 6, 2024
e9e5e40
test: Add test for data reference and add job in metadata storage
sitaowang1998 Nov 6, 2024
1f1445b
style: Fix lint check
sitaowang1998 Nov 6, 2024
67f108a
style: Improvde code style based on code rabbit suggestion
sitaowang1998 Nov 6, 2024
4322f85
style: Fix typo in comment
sitaowang1998 Nov 6, 2024
e15cf1f
build: Fix mariadb-conector-cpp include interface
sitaowang1998 Nov 7, 2024
47feb1e
build: Install libmariadb using package manager and build libmariadb …
sitaowang1998 Nov 7, 2024
dac1f75
build: Move apt install libmariadb-dev to install-lib from install-dev
sitaowang1998 Nov 7, 2024
7c3adbe
build: Use sudo to apt-get
sitaowang1998 Nov 7, 2024
8d40803
build: Use sudo to apt-get
sitaowang1998 Nov 7, 2024
0ae30cc
docs: Add doc for setup and running unit tests
sitaowang1998 Nov 8, 2024
1e3f1c5
docs: Fix typo
sitaowang1998 Nov 8, 2024
b939195
docs: Improve testing doc based on pr reviews
sitaowang1998 Nov 8, 2024
5929c68
docs: Improve testing doc based on pr reviews
sitaowang1998 Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ endif()

# Add abseil-cpp
set(ABSL_PROPAGATE_CXX_STD ON)
add_subdirectory(submodules/abseil-cpp EXCLUDE_FROM_ALL)
add_subdirectory(submodules/abseil-cpp)

# Add catch2
add_subdirectory(submodules/Catch2)

find_package(Threads REQUIRED)

Expand Down
15 changes: 13 additions & 2 deletions lint-tasks.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ tasks:
- "{{.G_SRC_SPIDER_DIR}}/**/*.cpp"
- "{{.G_SRC_SPIDER_DIR}}/**/*.h"
- "{{.G_SRC_SPIDER_DIR}}/**/*.hpp"
- "{{.G_TEST_DIR}}/**/*.cpp"
- "{{.G_TEST_DIR}}/**/*.h"
- "{{.G_TEST_DIR}}/**/*.hpp"
- "{{.TASKFILE}}"
- "tools/yscope-dev-utils/lint-configs/.clang-format"
deps: ["cpp-configs", "venv"]
Expand All @@ -67,6 +70,10 @@ tasks:
vars:
FLAGS: "-i"
SRC_DIR: "{{.G_SRC_SPIDER_DIR}}"
- task: "clang-format"
vars:
FLAGS: "-i"
SRC_DIR: "{{.G_TEST_DIR}}"

cpp-static-check:
# Alias task to `cpp-static-fix` since we don't currently support automatic fixes.
Expand All @@ -79,6 +86,9 @@ tasks:
- "{{.G_SRC_SPIDER_DIR}}/**/*.cpp"
- "{{.G_SRC_SPIDER_DIR}}/**/*.h"
- "{{.G_SRC_SPIDER_DIR}}/**/*.hpp"
- "{{.G_TEST_DIR}}/**/*.cpp"
- "{{.G_TEST_DIR}}/**/*.h"
- "{{.G_TEST_DIR}}/**/*.hpp"
- "{{.G_SPIDER_CMAKE_CACHE}}"
- "{{.G_SPIDER_COMPILE_COMMANDS_DB}}"
- "{{.TASKFILE}}"
Expand All @@ -90,6 +100,7 @@ tasks:
vars:
FLAGS: "--config-file=.clang-tidy -p {{.G_SPIDER_COMPILE_COMMANDS_DB}}"
SRC_DIR: "{{.G_SRC_SPIDER_DIR}}"
TEST_DIR: "{{.G_TEST_DIR}}"

yml:
aliases:
Expand Down Expand Up @@ -123,10 +134,10 @@ tasks:
clang-tidy:
internal: true
requires:
vars: ["FLAGS", "SRC_DIR"]
vars: ["FLAGS", "SRC_DIR", "TEST_DIR"]
cmd: |-
. "{{.G_LINT_VENV_DIR}}/bin/activate"
find "{{.SRC_DIR}}" \
find "{{.SRC_DIR}}" "{{.TEST_DIR}}" \
-type f \
\( -iname "*.cpp" -o -iname "*.h" -o -iname "*.hpp" \) \
-print0 | \
Expand Down
14 changes: 6 additions & 8 deletions src/spider/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
# set variable as CACHE INTERNAL to access it from other scope
set(SPIDER_CORE_SOURCES
set(SPIDER_CORE_SOURCES storage/MysqlStorage.cpp CACHE INTERNAL "spider core source files")

set(SPIDER_CORE_HEADERS
core/Error.hpp
core/Data.hpp
core/Task.hpp
core/TaskGraph.hpp
storage/MetadataStorage.hpp
storage/DataStorage.hpp
storage/MysqlStorage.cpp
storage/MysqlStorage.hpp
CACHE INTERNAL
"spider core source files"
"spider core header files"
)

if(SPIDER_USE_STATIC_LIBS)
add_library(spider_core STATIC)
else()
add_library(spider_core SHARED)
endif()
add_library(spider_core)
target_sources(spider_core PRIVATE ${SPIDER_CORE_SOURCES})
target_sources(spider_core PUBLIC ${SPIDER_CORE_HEADERS})
Comment on lines +16 to +18
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Library type should be explicitly defined

The removal of static/shared configuration in src/spider/CMakeLists.txt conflicts with the project-wide build configuration. The main CMakeLists.txt still manages the SPIDER_USE_STATIC_LIBS option which affects multiple dependencies (Boost, spdlog) and has platform-specific handling. The library type should be explicitly set based on this global setting.

  • In src/spider/CMakeLists.txt, restore the conditional library type:
if(SPIDER_USE_STATIC_LIBS)
    add_library(spider_core STATIC)
else()
    add_library(spider_core SHARED)
endif()
🔗 Analysis chain

Consider preserving build flexibility

The removal of the static/shared library condition might limit build configuration options. Was this intentional?

Let me check the content of these CMake files to better understand the library configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for other components that might depend on library type

echo "Checking for potential library type dependencies:"
rg -l "SPIDER_USE_STATIC_LIBS" .

echo "Checking for library references in test configurations:"
fd CMakeLists.txt -t f tests/

Length of output: 332

Script:

#!/bin/bash
# Check the main CMakeLists.txt for static library configuration
echo "=== Main CMakeLists.txt ==="
rg "SPIDER_USE_STATIC_LIBS" CMakeLists.txt -A 3 -B 3

echo -e "\n=== Test CMakeLists.txt ==="
cat tests/CMakeLists.txt

echo -e "\n=== Previous spider library configuration ==="
git log -p src/spider/CMakeLists.txt | grep -A 10 "add_library"

Length of output: 4035

target_link_libraries(
spider_core
PUBLIC
Expand Down
2 changes: 2 additions & 0 deletions src/spider/core/Data.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ class Data {
public:
explicit Data(std::string value) : m_value(std::move(value)) { init_id(); }

Data(boost::uuids::uuid id, std::string value) : m_id(id), m_value(std::move(value)) {}

Data(std::string key, std::string value) : m_key(std::move(key)), m_value(std::move(value)) {
init_id();
}
Expand Down
2 changes: 1 addition & 1 deletion src/spider/core/Error.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct StorageErr {
: type(type),
description(std::move(description)) {}

explicit operator bool() const { return StorageErrType::Success != type; }
[[nodiscard]] auto success() const -> bool { return StorageErrType::Success == type; }
};

} // namespace spider::core
Expand Down
11 changes: 1 addition & 10 deletions src/spider/core/TaskGraph.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <absl/container/flat_hash_map.h>
#include <absl/container/flat_hash_set.h>

#include <boost/uuid/random_generator.hpp>
#include <boost/uuid/uuid.hpp>
#include <optional>
#include <utility>
Expand All @@ -15,12 +14,7 @@
namespace spider::core {
class TaskGraph {
public:
TaskGraph() {
boost::uuids::random_generator gen;
m_id = gen();
}

explicit TaskGraph(boost::uuids::uuid id) : m_id(id) {}
TaskGraph() = default;

auto add_child_task(Task const& task, std::vector<boost::uuids::uuid> const& parents) -> bool {
boost::uuids::uuid task_id = task.get_id();
Expand Down Expand Up @@ -54,8 +48,6 @@ class TaskGraph {
m_dependencies.emplace_back(parent, child);
}

[[nodiscard]] auto get_id() const -> boost::uuids::uuid { return m_id; }

[[nodiscard]] auto get_task(boost::uuids::uuid id) const -> std::optional<Task> {
if (m_tasks.contains(id)) {
return m_tasks.at(id);
Expand Down Expand Up @@ -106,7 +98,6 @@ class TaskGraph {
}

private:
boost::uuids::uuid m_id;
absl::flat_hash_map<boost::uuids::uuid, Task> m_tasks;
std::vector<std::pair<boost::uuids::uuid, boost::uuids::uuid>> m_dependencies;
};
Expand Down
11 changes: 7 additions & 4 deletions src/spider/storage/DataStorage.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
namespace spider::core {
class DataStorage {
public:
DataStorage(DataStorage const&) = default;
DataStorage(DataStorage&&) = default;
auto operator=(DataStorage const&) -> DataStorage& = default;
auto operator=(DataStorage&&) -> DataStorage& = default;
DataStorage() = default;
DataStorage(DataStorage const&) = delete;
DataStorage(DataStorage&&) = delete;
auto operator=(DataStorage const&) -> DataStorage& = delete;
auto operator=(DataStorage&&) -> DataStorage& = delete;
virtual ~DataStorage() = default;

virtual auto connect(std::string const& url) -> StorageErr = 0;
Expand All @@ -22,6 +23,8 @@ class DataStorage {

virtual auto add_data(Data const& data) -> StorageErr = 0;
virtual auto get_data(boost::uuids::uuid id, Data* data) -> StorageErr = 0;
virtual auto get_data_by_key(std::string const& key, Data* data) -> StorageErr = 0;
virtual auto remove_data(boost::uuids::uuid id) -> StorageErr = 0;
virtual auto add_task_reference(boost::uuids::uuid id, boost::uuids::uuid task_id) -> StorageErr
= 0;
virtual auto
Expand Down
20 changes: 13 additions & 7 deletions src/spider/storage/MetadataStorage.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@
namespace spider::core {
class MetadataStorage {
public:
MetadataStorage(MetadataStorage const&) = default;
MetadataStorage(MetadataStorage&&) = default;
auto operator=(MetadataStorage const&) -> MetadataStorage& = default;
auto operator=(MetadataStorage&&) -> MetadataStorage& = default;
MetadataStorage() = default;
MetadataStorage(MetadataStorage const&) = delete;
MetadataStorage(MetadataStorage&&) = delete;
auto operator=(MetadataStorage const&) -> MetadataStorage& = delete;
auto operator=(MetadataStorage&&) -> MetadataStorage& = delete;
virtual ~MetadataStorage() = default;

virtual auto connect(std::string const& url) -> StorageErr = 0;
Expand All @@ -26,10 +27,15 @@ class MetadataStorage {
virtual auto add_driver(boost::uuids::uuid id, std::string const& addr, int port) -> StorageErr
= 0;

virtual auto add_task_graph(TaskGraph const& task_graph) -> StorageErr = 0;
virtual auto
add_job(boost::uuids::uuid job_id, boost::uuids::uuid client_id, TaskGraph const& task_graph
) -> StorageErr = 0;
virtual auto get_task_graph(boost::uuids::uuid id, TaskGraph* task_graph) -> StorageErr = 0;
virtual auto get_task_graphs(std::vector<boost::uuids::uuid>* task_graphs) -> StorageErr = 0;
virtual auto remove_task_graph(boost::uuids::uuid id) -> StorageErr = 0;
virtual auto get_jobs_by_client_id(
boost::uuids::uuid client_id,
std::vector<boost::uuids::uuid>* job_ids
) -> StorageErr = 0;
virtual auto remove_job(boost::uuids::uuid id) -> StorageErr = 0;
virtual auto add_child(boost::uuids::uuid parent_id, Task const& child) -> StorageErr = 0;
virtual auto get_task(boost::uuids::uuid id, Task* task) -> StorageErr = 0;
virtual auto get_ready_tasks(std::vector<Task>* tasks) -> StorageErr = 0;
Expand Down
Loading