From f8bdb597c326b7528eaf76e5df5c296a8b3b3497 Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Mon, 27 Jun 2022 16:36:25 +0200 Subject: [PATCH 1/4] Add ADM_strerror() --- src/api/CMakeLists.txt | 2 +- src/api/admire.cpp | 25 +++++++++------------ src/api/admire.h | 18 ++++++++++++--- src/api/c_wrapper.cpp | 8 +++---- src/api/detail/impl.cpp | 3 +-- src/api/errors.c | 49 +++++++++++++++++++++++++++++++++++++++++ tests/CMakeLists.txt | 0 tests/test.cpp | 3 +++ 8 files changed, 83 insertions(+), 25 deletions(-) create mode 100644 src/api/errors.c create mode 100644 tests/CMakeLists.txt create mode 100644 tests/test.cpp diff --git a/src/api/CMakeLists.txt b/src/api/CMakeLists.txt index 77cb0bd8..e6a5eddb 100644 --- a/src/api/CMakeLists.txt +++ b/src/api/CMakeLists.txt @@ -26,7 +26,7 @@ add_library(adm_iosched SHARED) target_sources(adm_iosched PUBLIC admire.h admire.hpp - PRIVATE admire.cpp c_wrapper.cpp detail/impl.hpp detail/impl.cpp) + PRIVATE admire.cpp c_wrapper.cpp detail/impl.hpp detail/impl.cpp errors.c) target_include_directories(adm_iosched PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/src/api/admire.cpp b/src/api/admire.cpp index b3586426..79a83e10 100644 --- a/src/api/admire.cpp +++ b/src/api/admire.cpp @@ -60,16 +60,15 @@ register_job(const server& srv, ADM_job_requirements_t reqs) { const auto rv = detail::register_job(srv, reqs); if(!rv) { - /* TODO ADM_strerror(rv.error()) */ - throw std::runtime_error("ADM_register_job() error"); + throw std::runtime_error(fmt::format("ADM_register_job() error: {}", + ADM_strerror(rv.error()))); } return rv.value(); } ADM_return_t -update_job(const server& srv, ADM_job_t job, - ADM_job_requirements_t reqs) { +update_job(const server& srv, ADM_job_t job, ADM_job_requirements_t reqs) { (void) srv; (void) job; (void) reqs; @@ -152,8 +151,7 @@ register_adhoc_storage(const server& srv, ADM_job_t job, } ADM_return_t -update_adhoc_storage(const server& srv, ADM_job_t job, - ADM_adhoc_context_t ctx, +update_adhoc_storage(const server& srv, ADM_job_t job, ADM_adhoc_context_t ctx, ADM_adhoc_storage_handle_t adhoc_handle) { (void) srv; (void) job; @@ -301,8 +299,8 @@ set_dataset_information(const server& srv, ADM_job_t job, } ADM_return_t -set_io_resources(const server& srv, ADM_job_t job, - ADM_storage_handle_t tier, ADM_storage_resources_t resources) { +set_io_resources(const server& srv, ADM_job_t job, ADM_storage_handle_t tier, + ADM_storage_resources_t resources) { (void) srv; (void) job; (void) tier; @@ -447,8 +445,7 @@ get_pending_transfers(const server& srv, ADM_job_t job, } ADM_return_t -set_qos_constraints(const server& srv, ADM_job_t job, - ADM_limit_t limit) { +set_qos_constraints(const server& srv, ADM_job_t job, ADM_limit_t limit) { (void) srv; (void) job; (void) limit; @@ -477,9 +474,8 @@ set_qos_constraints(const server& srv, ADM_job_t job, } ADM_return_t -get_qos_constraints(const server& srv, ADM_job_t job, - ADM_qos_scope_t scope, ADM_qos_entity_t entity, - ADM_limit_t** limits) { +get_qos_constraints(const server& srv, ADM_job_t job, ADM_qos_scope_t scope, + ADM_qos_entity_t entity, ADM_limit_t** limits) { (void) srv; (void) job; (void) scope; @@ -641,8 +637,7 @@ link_transfer_to_data_operation(const server& srv, ADM_job_t job, } ADM_return_t -get_statistics(const server& srv, ADM_job_t job, - ADM_job_stats_t** stats) { +get_statistics(const server& srv, ADM_job_t job, ADM_job_stats_t** stats) { (void) srv; (void) job; (void) stats; diff --git a/src/api/admire.h b/src/api/admire.h index 33d19e6a..a103d4a0 100644 --- a/src/api/admire.h +++ b/src/api/admire.h @@ -46,9 +46,11 @@ extern "C" { /* Error return codes */ typedef enum { ADM_SUCCESS = 0, - ADM_EINVAL, - // generic error code - ADM_OTHER_ERROR + ADM_ESNAFU, + ADM_EBADARGS, + ADM_ENOMEM, + ADM_EOTHER, + ADM_ERR_MAX = 512 } ADM_return_t; /* A server */ @@ -579,6 +581,16 @@ ADM_link_transfer_to_data_operation(ADM_server_t server, ADM_job_t job, ADM_return_t ADM_get_statistics(ADM_server_t server, ADM_job_t job, ADM_job_stats_t** stats); +/** + * Return a string describing the error number + * + * @param[in] errnum The error number for which a description should be + * returned. + * @return A pointer to a string describing `errnum`. + */ +const char* +ADM_strerror(ADM_return_t errnum); + #ifdef __cplusplus } // extern "C" #endif diff --git a/src/api/c_wrapper.cpp b/src/api/c_wrapper.cpp index 2496c43b..a8816f58 100644 --- a/src/api/c_wrapper.cpp +++ b/src/api/c_wrapper.cpp @@ -77,7 +77,7 @@ ADM_server_destroy(ADM_server_t server) { if(!server) { LOGGER_ERROR("Invalid ADM_server_t") - return ADM_EINVAL; + return ADM_EBADARGS; } free(server); @@ -106,7 +106,7 @@ ADM_dataset_destroy(ADM_dataset_handle_t dataset) { if(!dataset) { LOGGER_ERROR("Invalid ADM_dataset_t") - return ADM_EINVAL; + return ADM_EBADARGS; } free(dataset); @@ -142,7 +142,7 @@ ADM_job_requirements_destroy(ADM_job_requirements_t reqs) { if(!reqs) { LOGGER_ERROR("Invalid ADM_job_requirements_t") - return ADM_EINVAL; + return ADM_EBADARGS; } free(reqs); @@ -188,7 +188,7 @@ ADM_register_job(ADM_server_t server, ADM_job_requirements_t reqs, const auto jh = ADM_job_create(rv->m_id); if(!jh) { - return ADM_OTHER_ERROR; + return ADM_EOTHER; } *job = jh; diff --git a/src/api/detail/impl.cpp b/src/api/detail/impl.cpp index 962444de..b14f1fe0 100644 --- a/src/api/detail/impl.cpp +++ b/src/api/detail/impl.cpp @@ -47,8 +47,7 @@ register_job(const admire::server& srv, ADM_job_requirements_t reqs) { if(out.ret < 0) { LOGGER_ERROR("ADM_register_job() = {}", out.ret); - /* TODO ADM_strerror(out.ret) */ - throw std::runtime_error("ADM_register_job() error"); + return tl::make_unexpected(static_cast(out.ret)); } LOGGER_INFO("ADM_register_job() = {}", ADM_SUCCESS); diff --git a/src/api/errors.c b/src/api/errors.c new file mode 100644 index 00000000..87985cae --- /dev/null +++ b/src/api/errors.c @@ -0,0 +1,49 @@ +/****************************************************************************** + * Copyright 2021-2022, Barcelona Supercomputing Center (BSC), Spain + * + * This software was partially supported by the EuroHPC-funded project ADMIRE + * (Project ID: 956748, https://www.admire-eurohpc.eu). + * + * This file is part of scord. + * + * scord is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * scord is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with scord. If not, see . + * + * SPDX-License-Identifier: GPL-3.0-or-later + *****************************************************************************/ + +#include + +const char* const adm_errlist[ADM_ERR_MAX + 1] = { + [ADM_SUCCESS] = "Success", + [ADM_ESNAFU] = "Internal error", + [ADM_EBADARGS] = "Bad arguments", + [ADM_ENOMEM] = "Cannot allocate memory", + + [ADM_EOTHER] = "Undetermined error", + + /* fallback */ + [ADM_ERR_MAX] = "Unknown error", + +}; + +const char* +ADM_strerror(ADM_return_t errnum) { + + if(errnum > ADM_ERR_MAX) { + errnum = ADM_ERR_MAX; + } + + const char* s = adm_errlist[errnum]; + return s ? s : adm_errlist[ADM_EOTHER]; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/test.cpp b/tests/test.cpp new file mode 100644 index 00000000..d4d864bb --- /dev/null +++ b/tests/test.cpp @@ -0,0 +1,3 @@ +// +// Created by amiranda on 28/06/22. +// -- GitLab From b9ff8fdaf2762db88c9daca4743a1048a3fed9e1 Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Tue, 28 Jun 2022 15:19:26 +0200 Subject: [PATCH 2/4] Add unit tests for ADM_strerror() This commit brings the Catch2 dependency into the project --- CMakeLists.txt | 33 ++++++++++++++++++-- tests/CMakeLists.txt | 31 +++++++++++++++++++ tests/test.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 131 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 21909d93..c3db076a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,6 +123,8 @@ message(STATUS "[${PROJECT_NAME}] server bind port: ${SCORD_BIND_PORT}") option(SCORD_BUILD_EXAMPLES "Build examples (disabled by default)" OFF) +option(SCORD_BUILD_TESTS "Build tests (disabled by default)" OFF) + set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # ############################################################################## @@ -225,6 +227,27 @@ FetchContent_Declare( FetchContent_MakeAvailable(expected) +if (SCORD_BUILD_TESTS) + + enable_testing() + + ### catch2: required for unit testing + message(STATUS "[${PROJECT_NAME}] Downloading and building Catch2") + FetchContent_Declare( + Catch2 + GIT_REPOSITORY https://github.com/catchorg/Catch2.git + GIT_TAG 605a34765aa5d5ecbf476b4598a862ada971b0cc # v3.0.1 + GIT_SHALLOW ON + GIT_PROGRESS ON + ) + + FetchContent_MakeAvailable(Catch2) + + # Ensure that CMake can find Catch2 extra CMake modules in case + # they are needed + list(APPEND CMAKE_MODULE_PATH "${catch2_SOURCE_DIR}/extras") +endif () + ### Mark any CMake variables imported from {fmt} and spdlog as advanced, so ### that they don't appear in cmake-gui or ccmake. Similarly for FETCHCONTENT ### variables. @@ -236,11 +259,11 @@ mark_variables_as_advanced(REGEX "^(FETCHCONTENT|fmt|FMT|spdlog|SPDLOG)_.*$") # set compile flags add_compile_options("-Wall" "-Wextra" "$<$:-O3>") -if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") +if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang") add_compile_options("-stdlib=libc++") -else() +else () # nothing special for gcc at the moment -endif() +endif () add_subdirectory(etc) add_subdirectory(src) @@ -248,3 +271,7 @@ add_subdirectory(src) if (SCORD_BUILD_EXAMPLES) add_subdirectory(examples) endif () + +if(SCORD_BUILD_TESTS) + add_subdirectory(tests) +endif() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e69de29b..679503c6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -0,0 +1,31 @@ +################################################################################ +# Copyright 2021, Barcelona Supercomputing Center (BSC), Spain # +# # +# This software was partially supported by the EuroHPC-funded project ADMIRE # +# (Project ID: 956748, https://www.admire-eurohpc.eu). # +# # +# This file is part of scord. # +# # +# scord is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# scord is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with scord. If not, see . # +# # +# SPDX-License-Identifier: GPL-3.0-or-later # +################################################################################ + +add_executable(tests) + +target_sources(tests PRIVATE test.cpp) +target_link_libraries(tests PRIVATE Catch2::Catch2WithMain adm_iosched) + +include(Catch) +catch_discover_tests(tests) diff --git a/tests/test.cpp b/tests/test.cpp index d4d864bb..f7830dc8 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -1,3 +1,70 @@ -// -// Created by amiranda on 28/06/22. -// +/****************************************************************************** + * Copyright 2021-2022, Barcelona Supercomputing Center (BSC), Spain + * + * This software was partially supported by the EuroHPC-funded project ADMIRE + * (Project ID: 956748, https://www.admire-eurohpc.eu). + * + * This file is part of scord. + * + * scord is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * scord is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with scord. If not, see . + * + * SPDX-License-Identifier: GPL-3.0-or-later + *****************************************************************************/ + +#include +#include +#include + +SCENARIO("Error messages can be printed", "[lib][ADM_strerror]") { + + GIVEN("An error number") { + WHEN("The error number is ADM_SUCCESS") { + REQUIRE(std::string{ADM_strerror(ADM_SUCCESS)} == "Success"); + } + + WHEN("The error number is ADM_ESNAFU") { + REQUIRE(std::string{ADM_strerror(ADM_ESNAFU)} == "Internal error"); + } + + WHEN("The error number is ADM_EBADARGS") { + REQUIRE(std::string{ADM_strerror(ADM_EBADARGS)} == "Bad arguments"); + } + + WHEN("The error number is ADM_ENOMEM") { + REQUIRE(std::string{ADM_strerror(ADM_ENOMEM)} == + "Cannot allocate memory"); + } + + WHEN("The error number is ADM_EOTHER") { + REQUIRE(std::string{ADM_strerror(ADM_EOTHER)} == + "Undetermined error"); + } + + WHEN("The error number is larger than ADM_EOTHER and " + "lower than ADM_ERR_MAX") { + + for(int i = ADM_EOTHER; i < ADM_ERR_MAX; ++i) { + const auto e = static_cast(i); + REQUIRE(std::string{ADM_strerror(e)} == "Undetermined error"); + } + } + + WHEN("The error number is larger than ADM_ERR_MAX") { + for(int i = ADM_ERR_MAX; i < ADM_ERR_MAX * 2; ++i) { + const auto e = static_cast(i); + REQUIRE(std::string{ADM_strerror(e)} == "Unknown error"); + } + } + } +} -- GitLab From 2ca716576a2d145d8c34992ae172783637a93eb0 Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Wed, 29 Jun 2022 11:55:14 +0200 Subject: [PATCH 3/4] Update README.md --- README.md | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0a5714ec..34351904 100644 --- a/README.md +++ b/README.md @@ -27,8 +27,9 @@ available in the system: The following libraries are also required by `scord`, but will be downloaded and compiled by the project as part of the standard build process. -- [{fmt}]() version 8.0.1 or later. -- [spdlog]() version 1.9.2 or later. +- [{fmt}](https://fmt.dev/latest/index.html) version 8.0.1 or later. +- [spdlog](https://github.com/gabime/spdlog) version 1.9.2 or later. +- [Catch2](https://github.com/catchorg/Catch2) version 3.0.1 or later. > **ℹ️** **Important** Margo and Argobots use `pkg-config` to ensure they compile and link correctly @@ -75,8 +76,10 @@ The following CMake options can be used to configure how `scord` is built: communicate with the service. The value provided here will be used to set the `bind_port` configuration option in the `${PREFIX}/etc/scord.conf` installed alongside the service. -- `SCORD_BUILD_EXAMPLES`: This option instructs CMakes to build the programs +- `SCORD_BUILD_EXAMPLES`: This option instructs CMake to build the programs contained in the `examples` subdirectory. +- `SCORD_BUILD_TESTS`: This option instructs CMake to build the tests + contained in the `tests` subdirectory. Thus, let's assume that we want to build `scord` with the following configuration: @@ -98,6 +101,7 @@ mkdir build && cd build cmake -DCMAKE_PREFIX_PATH:STRING=/opt \ -DCMAKE_INSTALL_PREFIX:STRING=/usr/local \ -DSCORD_BUILD_EXAMPLES:BOOL=ON \ + -DSCORD_BUILD_TESTS:BOOL=ON \ -DSCORD_TRANSPORT_LIBRARY=libfabric \ -DSCORD_TRANSPORT_PROTOCOL=ofi+tcp \ -DSCORD_BIND_ADDRESS=192.168.0.111 \ @@ -106,6 +110,22 @@ cmake -DCMAKE_PREFIX_PATH:STRING=/opt \ make ``` +## Running tests + +Tests are integrated in [CTest](https://cmake.org/cmake/help/book/mastering-cmake/chapter/Testing%20With%20CMake%20and%20CTest.html), CMake's testing facility. Once built, the +tests can be run in parallel using the `ctest` command line tool: + +```console +~/projects/scord/build $ ctest --parallel 4 +Test project /home/amiranda/var/projects/scord/repo/build + Start 1: Scenario: Error messages can be printed +1/1 Test #1: Scenario: Error messages can be printed ... Passed 0.14 sec + +100% tests passed, 0 tests failed out of 1 + +Total Test time (real) = 0.14 sec +``` + ## Installing Assuming that the CMAKE_INSTALL_PREFIX has been set (see previous step) and -- GitLab From 3cbc90666a4fe4c6309c0bab85255d119d23dc83 Mon Sep 17 00:00:00 2001 From: Alberto Miranda Date: Wed, 29 Jun 2022 12:01:19 +0200 Subject: [PATCH 4/4] Update .gitlab-ci.yml --- .gitlab-ci.yml | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 752983ac..329a0f0a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -13,7 +13,16 @@ build: - export PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/lib64/pkgconfig - mkdir -p build - cd build - - cmake -DCMAKE_PREFIX_PATH:STRING=/usr/local -DCMAKE_INSTALL_PREFIX:STRING=${CI_PROJECT_DIR}/compiled -DSCORD_BUILD_EXAMPLES:BOOL=ON -DSCORD_TRANSPORT_LIBRARY=libfabric -DSCORD_TRANSPORT_PROTOCOL=ofi+tcp -DSCORD_BIND_ADDRESS=127.0.0.1 -DSCORD_BIND_PORT=52000 .. + - cmake + -DCMAKE_PREFIX_PATH:STRING=/usr/local + -DCMAKE_INSTALL_PREFIX:STRING=${CI_PROJECT_DIR}/compiled + -DSCORD_BUILD_EXAMPLES:BOOL=ON + -DSCORD_BUILD_TESTS:BOOL=ON + -DSCORD_TRANSPORT_LIBRARY=libfabric + -DSCORD_TRANSPORT_PROTOCOL=ofi+tcp + -DSCORD_BIND_ADDRESS=127.0.0.1 + -DSCORD_BIND_PORT=52000 + .. - make -j$(nproc) install artifacts: paths: @@ -21,6 +30,7 @@ build: - compiled/etc/ - compiled/lib/ - build/examples/ + - build/tests/ # depending on your build setup it's most likely a good idea to cache outputs to reduce the build time cache: key: $CI_COMMIT_REF_SLUG @@ -29,14 +39,31 @@ build: - compiled/bin - compiled/etc -# run tests using the binary built before -test: +# run RPC tests using the binary built before +rpc: stage: test needs: [build] script: - export LD_LIBRARY_PATH=/usr/local/lib:/usr/local/lib64:${CI_PROJECT_DIR}/compiled/lib - compiled/bin/scord -f --force-console & - - build/examples/ping ofi+tcp://127.0.0.1:52000 + - build/examples/cxx/ping ofi+tcp://127.0.0.1:52000 - pkill -TERM scord cache: key: $CI_COMMIT_REF_SLUG + +# run unit tests +unit: + stage: test + needs: [build] + script: + - export LD_LIBRARY_PATH=/usr/local/lib:/usr/local/lib64:${CI_PROJECT_DIR}/compiled/lib + - cd build/tests + - ctest -j$(nproc) --output-junit report.xml + cache: + key: $CI_COMMIT_REF_SLUG + artifacts: + expire_in: 1 week + paths: + - build/tests/report.xml + reports: + junit: build/tests/report.xml -- GitLab