From fd04ba837a6bb068ba9a483e2f98949e8456412a Mon Sep 17 00:00:00 2001 From: fossabot Date: Fri, 24 Oct 2025 14:27:46 -0700 Subject: [PATCH 01/11] Add license scan report and status Signed off by: fossabot --- README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a248cb7..0ec5728 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,7 @@ # STEPToPoints [![CI-Build](https://github.com/simogasp/STEPToPoints/actions/workflows/build.yml/badge.svg)](https://github.com/simogasp/STEPToPoints/actions/workflows/build.yml) +[![FOSSA Status](https://app.fossa.com/api/projects/git%2Bgithub.com%2Fsimogasp%2FSTEPToPoints.svg?type=shield)](https://app.fossa.com/projects/git%2Bgithub.com%2Fsimogasp%2FSTEPToPoints?ref=badge_shield) ## Description The program STEPToPoints is a command line utility to generate point clouds out of solids contained in STEP files. @@ -74,4 +75,7 @@ Examples are from the `examples` directory. | ![Image Point-Cloud-Basic-Shapes](examples/basic_shapes/point_cloud.png) | ## Remarks -This code has been tested with an OpenCASCADE 7.5.0 prebuilt binary (`opencascade-7.5.0-vc14-64.exe`) on Windows, as well as OpenCASCADE system packages on openSUSE Linux. With changes in the configuration section in the `CMakeLists.txt` file the build should also work with other OpenCASCADE versions. \ No newline at end of file +This code has been tested with an OpenCASCADE 7.5.0 prebuilt binary (`opencascade-7.5.0-vc14-64.exe`) on Windows, as well as OpenCASCADE system packages on openSUSE Linux. With changes in the configuration section in the `CMakeLists.txt` file the build should also work with other OpenCASCADE versions. + +## License +[![FOSSA Status](https://app.fossa.com/api/projects/git%2Bgithub.com%2Fsimogasp%2FSTEPToPoints.svg?type=large)](https://app.fossa.com/projects/git%2Bgithub.com%2Fsimogasp%2FSTEPToPoints?ref=badge_large) \ No newline at end of file From 3ce0249b13726d9caa4703af7865f01919a5a271 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Wed, 19 Nov 2025 18:34:39 +0100 Subject: [PATCH 02/11] add range to solid selection at cli --- src/STEPToPoints.cpp | 98 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 86 insertions(+), 12 deletions(-) diff --git a/src/STEPToPoints.cpp b/src/STEPToPoints.cpp index a441b0c..e5357a4 100644 --- a/src/STEPToPoints.cpp +++ b/src/STEPToPoints.cpp @@ -435,21 +435,86 @@ auto sampleShape(const TopoDS_Shape& shape, const double sampling) -> std::vecto } /** - * @brief Attempts to parse a string as a solid index (1-based). + * @brief Generates a vector of indices from a range. + * + * @param start The start index (1-based, inclusive). + * @param end The end index (1-based, inclusive). + * @return std::vector Vector of zero-based indices. + */ +auto generateRange(std::size_t start, std::size_t end) -> std::vector +{ + std::vector indices; + indices.reserve(end - start + 1); + for(auto i = start; i <= end; ++i) + { + indices.push_back(i - 1); // Convert to zero-based + } + return indices; +} + +/** + * @brief Parses a range string (e.g., "3-7") into start and end indices. * * @param sel The string to parse. + * @return std::optional> Pair of (start, end) if valid, std::nullopt otherwise. + */ +auto parseRange(const std::string& sel) -> std::optional> +{ + if(const auto dashPos = sel.find('-'); dashPos != std::string::npos && dashPos > 0) + { + try + { + const auto start = std::stoul(sel.substr(0, dashPos)); + const auto end = std::stoul(sel.substr(dashPos + 1)); + + if(start >= 1 && start <= end) + { + return std::make_pair(start, end); + } + } + catch(const std::invalid_argument&) + { + std::cerr << "Invalid range format: " << sel << "\n"; + } + catch(const std::out_of_range&) + { + std::cerr << "Range values out of bounds: " << sel << "\n"; + } + } + return std::nullopt; +} + +/** + * @brief Attempts to parse a string as a solid index (1-based) or a range. + * + * @param sel The string to parse containing either an index (e.g."3") or a range (e.g."3-7"). * @param maxIndex The maximum valid index (size of the namedSolids vector). - * @return std::optional The zero-based index if valid, std::nullopt otherwise. + * @return std::optional> Vector of zero-based indices if valid, std::nullopt otherwise. */ -auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional +auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional> { + // Try parsing as range first + if(const auto range = parseRange(sel)) + { + const auto [start, end] = range.value(); + + if(end > maxIndex) + { + std::cerr << "Range end exceeds max index: " << end << " > " << maxIndex << "\n"; + return std::nullopt; + } + + return generateRange(start, end); + } + try { if(const auto index = std::stoul(sel); index >= 1 && index <= maxIndex) { - return index - 1; // Convert to zero-based + return {{index - 1}}; } + std::cerr << "Index out of valid range: " << sel << "\n"; } catch(const std::invalid_argument&) { @@ -486,13 +551,13 @@ auto findSolidByName(const std::vector& namedSolids, const std::stri /** * @brief Resolves a selection string to a solid. * - * @param sel The selection string (either a name starting with '/' or a 1-based index). + * @param sel The selection string (either a name starting with '/' or a 1-based index or range). * @param namedSolids The vector of all available named solids. - * @return const NamedSolid& Reference to the selected solid. + * @return std::vector> List of references to the selected solid(s). * @throws std::invalid_argument If the selection cannot be resolved. */ auto resolveSolidSelection(const std::string& sel, const std::vector& namedSolids) - -> const NamedSolid& + -> std::vector> { if(sel.empty()) { @@ -504,15 +569,21 @@ auto resolveSolidSelection(const std::string& sel, const std::vector { if(const auto found = findSolidByName(namedSolids, sel)) { - return found->get(); + return {found->get()}; } throw std::invalid_argument{std::format("Could not find solid with name '{}'", sel)}; } // Try to parse as index - if(const auto index = parseSolidIndex(sel, namedSolids.size()); index.has_value()) + if(const auto indices = parseSolidIndex(sel, namedSolids.size()); indices.has_value()) { - return namedSolids[index.value()]; + std::vector> result; + result.reserve(indices->size()); + for(const auto idx : indices.value()) + { + result.emplace_back(std::cref(namedSolids[idx])); + } + return result; } throw std::invalid_argument{std::format("Invalid selection: '{}' (not a valid name or index)", sel)}; @@ -550,8 +621,11 @@ auto buildCompoundFromSelections(const std::vector& namedSolids, if(!sel.empty()) { const auto& selectedSolid = resolveSolidSelection(sel, namedSolids); - std::cout << "Adding solid: " << selectedSolid.name << "\n"; - builder.Add(compound, selectedSolid.solid); + for(const auto& solid : selectedSolid) + { + std::cout << "Adding solid: " << solid.get().name << "\n"; + builder.Add(compound, solid.get().solid); + } } } } From 80cd1bbcc4870d1a519519994dcc4f82085a7bae Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Wed, 19 Nov 2025 18:43:24 +0100 Subject: [PATCH 03/11] refactoring splitting function in other files --- CMakeLists.txt | 2 +- src/STEPToPoints.cpp | 94 +------------------------------------- src/solid_index_parser.cpp | 79 ++++++++++++++++++++++++++++++++ src/solid_index_parser.hpp | 30 ++++++++++++ 4 files changed, 111 insertions(+), 94 deletions(-) create mode 100644 src/solid_index_parser.cpp create mode 100644 src/solid_index_parser.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index cfb3da3..c09bb08 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,7 +97,7 @@ else() message(STATUS "OpenMP not found") endif() -add_executable(STEPToPoints src/STEPToPoints.cpp) +add_executable(STEPToPoints src/STEPToPoints.cpp src/solid_index_parser.cpp) # @TODO: check which OpenCASCADE libraries are really needed target_link_libraries(STEPToPoints ${OpenCASCADE_FoundationClasses_LIBRARIES} ${OpenCASCADE_ModelingData_LIBRARIES} ${OpenCASCADE_ModelingAlgorithms_LIBRARIES} ${OpenCASCADE_Visualization_LIBRARIES} ${OpenCASCADE_ApplicationFramework_LIBRARIES} ${OpenCASCADE_DataExchange_LIBRARIES} ${OpenCASCADE_Draw_LIBRARIES} indicators::indicators) # diff --git a/src/STEPToPoints.cpp b/src/STEPToPoints.cpp index e5357a4..40127f0 100644 --- a/src/STEPToPoints.cpp +++ b/src/STEPToPoints.cpp @@ -47,6 +47,7 @@ #include "cxxopts.hpp" #include "Timer.hpp" #include "happly.hpp" +#include "solid_index_parser.hpp" #include #include #include @@ -434,99 +435,6 @@ auto sampleShape(const TopoDS_Shape& shape, const double sampling) -> std::vecto return result; } -/** - * @brief Generates a vector of indices from a range. - * - * @param start The start index (1-based, inclusive). - * @param end The end index (1-based, inclusive). - * @return std::vector Vector of zero-based indices. - */ -auto generateRange(std::size_t start, std::size_t end) -> std::vector -{ - std::vector indices; - indices.reserve(end - start + 1); - for(auto i = start; i <= end; ++i) - { - indices.push_back(i - 1); // Convert to zero-based - } - return indices; -} - -/** - * @brief Parses a range string (e.g., "3-7") into start and end indices. - * - * @param sel The string to parse. - * @return std::optional> Pair of (start, end) if valid, std::nullopt otherwise. - */ -auto parseRange(const std::string& sel) -> std::optional> -{ - if(const auto dashPos = sel.find('-'); dashPos != std::string::npos && dashPos > 0) - { - try - { - const auto start = std::stoul(sel.substr(0, dashPos)); - const auto end = std::stoul(sel.substr(dashPos + 1)); - - if(start >= 1 && start <= end) - { - return std::make_pair(start, end); - } - } - catch(const std::invalid_argument&) - { - std::cerr << "Invalid range format: " << sel << "\n"; - } - catch(const std::out_of_range&) - { - std::cerr << "Range values out of bounds: " << sel << "\n"; - } - } - return std::nullopt; -} - -/** - * @brief Attempts to parse a string as a solid index (1-based) or a range. - * - * @param sel The string to parse containing either an index (e.g."3") or a range (e.g."3-7"). - * @param maxIndex The maximum valid index (size of the namedSolids vector). - * @return std::optional> Vector of zero-based indices if valid, std::nullopt otherwise. - */ -auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional> -{ - // Try parsing as range first - if(const auto range = parseRange(sel)) - { - const auto [start, end] = range.value(); - - if(end > maxIndex) - { - std::cerr << "Range end exceeds max index: " << end << " > " << maxIndex << "\n"; - return std::nullopt; - } - - return generateRange(start, end); - } - - try - { - if(const auto index = std::stoul(sel); - index >= 1 && index <= maxIndex) - { - return {{index - 1}}; - } - std::cerr << "Index out of valid range: " << sel << "\n"; - } - catch(const std::invalid_argument&) - { - std::cerr << "Invalid index provided: " << sel << "\n"; - } - catch(const std::out_of_range&) - { - std::cerr << "Index out of range: " << sel << "\n"; - } - return std::nullopt; -} - /** * @brief Finds a solid by its full name path. * diff --git a/src/solid_index_parser.cpp b/src/solid_index_parser.cpp new file mode 100644 index 0000000..482b2f6 --- /dev/null +++ b/src/solid_index_parser.cpp @@ -0,0 +1,79 @@ +#include "solid_index_parser.hpp" +#include +#include + +auto generateRange(std::size_t start, std::size_t end) -> std::vector +{ + std::vector indices; + indices.reserve(end - start + 1); + for(auto i = start; i <= end; ++i) + { + indices.push_back(i - 1); // Convert to zero-based + } + return indices; +} + + +auto parseRange(const std::string& sel) -> std::optional> +{ + if(const auto dashPos = sel.find('-'); dashPos != std::string::npos && dashPos > 0) + { + try + { + const auto start = std::stoul(sel.substr(0, dashPos)); + const auto end = std::stoul(sel.substr(dashPos + 1)); + + if(start >= 1 && start <= end) + { + return std::make_pair(start, end); + } + } + catch(const std::invalid_argument&) + { + std::cerr << "Invalid range format: " << sel << "\n"; + } + catch(const std::out_of_range&) + { + std::cerr << "Range values out of bounds: " << sel << "\n"; + } + } + return std::nullopt; +} + + +auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional> +{ + // Try parsing as range first + if(const auto range = parseRange(sel)) + { + const auto [start, end] = range.value(); + + if(end > maxIndex) + { + std::cerr << "Range end exceeds max index: " << end << " > " << maxIndex << "\n"; + return std::nullopt; + } + + return generateRange(start, end); + } + + try + { + if(const auto index = std::stoul(sel); + index >= 1 && index <= maxIndex) + { + return {{index - 1}}; + } + std::cerr << "Index out of valid range: " << sel << "\n"; + } + catch(const std::invalid_argument&) + { + std::cerr << "Invalid index provided: " << sel << "\n"; + } + catch(const std::out_of_range&) + { + std::cerr << "Index out of range: " << sel << "\n"; + } + return std::nullopt; +} + diff --git a/src/solid_index_parser.hpp b/src/solid_index_parser.hpp new file mode 100644 index 0000000..7b91d63 --- /dev/null +++ b/src/solid_index_parser.hpp @@ -0,0 +1,30 @@ +#pragma once +#include +#include +#include + +/** + * @brief Generates a vector of indices from a range. + * + * @param start The start index (1-based, inclusive). + * @param end The end index (1-based, inclusive). + * @return std::vector Vector of zero-based indices. + */ +auto generateRange(std::size_t start, std::size_t end) -> std::vector; + +/** + * @brief Parses a range string (e.g., "3-7") into start and end indices. + * + * @param sel The string to parse. + * @return std::optional> Pair of (start, end) if valid, std::nullopt otherwise. + */ +auto parseRange(const std::string& sel) -> std::optional>; + +/** + * @brief Attempts to parse a string as a solid index (1-based) or a range. + * + * @param sel The string to parse containing either an index (e.g."3") or a range (e.g."3-7"). + * @param maxIndex The maximum valid index (size of the namedSolids vector). + * @return std::optional> Vector of zero-based indices if valid, std::nullopt otherwise. + */ +auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional>; \ No newline at end of file From 3a8f94c2b9827c655c02711e5fd273ee393f1166 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:27:40 +0100 Subject: [PATCH 04/11] [doc] markdown linting --- INSTALL.md | 1 + README.md | 23 ++++++++++++++--------- build_with_presets.md | 9 +++++---- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/INSTALL.md b/INSTALL.md index 2b159a7..6c8ac0f 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -27,3 +27,4 @@ cmake -DCMAKE_TOOLCHAIN_FILE=$VCPKG_ROOT/scripts/buildsystems/vcpkg.cmake .. # this will take a while as it will download and build all the necessary dependencies # now we can build the project cmake --build . --config Release +``` diff --git a/README.md b/README.md index a248cb7..661ea10 100644 --- a/README.md +++ b/README.md @@ -3,20 +3,23 @@ [![CI-Build](https://github.com/simogasp/STEPToPoints/actions/workflows/build.yml/badge.svg)](https://github.com/simogasp/STEPToPoints/actions/workflows/build.yml) ## Description + The program STEPToPoints is a command line utility to generate point clouds out of solids contained in STEP files. The supported output file format is xyz (vertex positions and normal vectors). -A popular viewer for the supported file format is MeshLab (https://www.meshlab.net). -STEPToPoints is based on OpenCASCADE (https://www.opencascade.com). -The program uses cxxops (https://github.com/jarro2783/cxxopts) for parsing the command line. +A popular viewer for the supported file format is MeshLab (). +STEPToPoints is based on OpenCASCADE (). +The program uses cxxops () for parsing the command line. ## Requirements - * CMake installation (https://cmake.org) - * OpenCASCADE installation (https://old.opencascade.com/content/latest-release, download needs registration) -For OpenCASCADE if it is not found it will be fetched and built via CMake FetchContent. +* CMake installation () +* OpenCASCADE installation (, download needs registration) + +For OpenCASCADE if it is not found it will be fetched and built via CMake FetchContent. For that, you need the following additional libs installed: - * tcl-dev - * tk-dev + +* tcl-dev +* tk-dev On Linux you can install these packages via the package manager, e.g. on ubuntu: @@ -25,6 +28,7 @@ sudo apt-get install tcl-dev tk-dev ``` ## Usage + Listing the contents (solids) of a STEP file: ```bash @@ -74,4 +78,5 @@ Examples are from the `examples` directory. | ![Image Point-Cloud-Basic-Shapes](examples/basic_shapes/point_cloud.png) | ## Remarks -This code has been tested with an OpenCASCADE 7.5.0 prebuilt binary (`opencascade-7.5.0-vc14-64.exe`) on Windows, as well as OpenCASCADE system packages on openSUSE Linux. With changes in the configuration section in the `CMakeLists.txt` file the build should also work with other OpenCASCADE versions. \ No newline at end of file + +This code has been tested with an OpenCASCADE 7.5.0 prebuilt binary (`opencascade-7.5.0-vc14-64.exe`) on Windows, as well as OpenCASCADE system packages on openSUSE Linux. With changes in the configuration section in the `CMakeLists.txt` file the build should also work with other OpenCASCADE versions. diff --git a/build_with_presets.md b/build_with_presets.md index 5af4751..fadb9b7 100644 --- a/build_with_presets.md +++ b/build_with_presets.md @@ -17,19 +17,20 @@ How to build the project using CMake presets and VCPKG for dependency management ``` 3. go back to the project directory and configure the project using CMake presets: - + ```bash cd .. cmake --preset release-preset cmake --preset debug-preset ``` + This will create the build directories `build/`. 4. **Build the project**: You can now build the project using the configured presets. - + ```bash cmake --build --preset release-preset cmake --build --preset debug-preset ``` - -Note that the dependencies will be installed in a common directory `vcpkg_installed` in the project root (check `VCPKG_INSTALLED_DIR` in the presets) so that it will only build the dependencies in release/debug once. \ No newline at end of file + +Note that the dependencies will be installed in a common directory `vcpkg_installed` in the project root (check `VCPKG_INSTALLED_DIR` in the presets) so that it will only build the dependencies in release/debug once. From 24ced5a42db9da066af086a4ba1f60565521f75a Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:27:52 +0100 Subject: [PATCH 05/11] add doc to cli --- src/STEPToPoints.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/STEPToPoints.cpp b/src/STEPToPoints.cpp index 40127f0..351f6df 100644 --- a/src/STEPToPoints.cpp +++ b/src/STEPToPoints.cpp @@ -569,7 +569,7 @@ int main(int argc, char* argv[]) cxxopts::value()) ("c,content", "List content (solids)") ("s,select", - "Select solids by name or index (comma seperated list, index starts with 1)", + "Select solids by name or index (comma seperated list, index starts with 1) or range index (e.g. 3-7)", cxxopts::value>()) ("g,sampling", "Sampling distance", cxxopts::value()) ("b,binary", "Write binary file (only for .ply files)", cxxopts::value()->default_value("false")) From 69114bf5aa5a254409a37392029c70b10a55d4e4 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:28:12 +0100 Subject: [PATCH 06/11] make parser more robust --- src/solid_index_parser.cpp | 52 +++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/src/solid_index_parser.cpp b/src/solid_index_parser.cpp index 482b2f6..07b1fb3 100644 --- a/src/solid_index_parser.cpp +++ b/src/solid_index_parser.cpp @@ -1,4 +1,5 @@ #include "solid_index_parser.hpp" +#include #include #include @@ -20,8 +21,35 @@ auto parseRange(const std::string& sel) -> std::optional(startStr[0])) || + !std::isdigit(static_cast(endStr[0]))) + { + std::cerr << "Invalid range format: " << sel << "\n"; + return std::nullopt; + } + + std::size_t startPos = 0; + std::size_t endPos = 0; + const auto start = std::stoul(startStr, &startPos); + const auto end = std::stoul(endStr, &endPos); + + // Ensure the entire string was consumed (no trailing characters) + if(startPos != startStr.length() || endPos != endStr.length()) + { + std::cerr << "Invalid range format: " << sel << "\n"; + return std::nullopt; + } if(start >= 1 && start <= end) { @@ -59,8 +87,24 @@ auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optio try { - if(const auto index = std::stoul(sel); - index >= 1 && index <= maxIndex) + // Check for empty string or leading whitespace/non-digit + if(sel.empty() || !std::isdigit(static_cast(sel[0]))) + { + std::cerr << "Invalid index provided: " << sel << "\n"; + return std::nullopt; + } + + std::size_t pos = 0; + const auto index = std::stoul(sel, &pos); + + // Ensure the entire string was consumed (no trailing characters) + if(pos != sel.length()) + { + std::cerr << "Invalid index provided: " << sel << "\n"; + return std::nullopt; + } + + if(index >= 1 && index <= maxIndex) { return {{index - 1}}; } From 21e9e237f780aa59032875b2036c165ce7cccb09 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:28:37 +0100 Subject: [PATCH 07/11] add unit tests with gtest --- CMakeLists.txt | 9 +- tests/CMakeLists.txt | 41 ++++ tests/solid_index_parser_tests.cpp | 336 +++++++++++++++++++++++++++++ 3 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 tests/CMakeLists.txt create mode 100644 tests/solid_index_parser_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c09bb08..6604beb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,6 +26,8 @@ cmake_minimum_required (VERSION 3.18) project (STEPToPoints LANGUAGES CXX VERSION 2025.0.0) +option(BUILD_TESTS "Build tests" ON) + if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE "Release" CACHE STRING "Build type" FORCE) set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug;Release;RelWithDebInfo;MinSizeRel") @@ -108,4 +110,9 @@ endif() target_compile_options(STEPToPoints PRIVATE ${MY_COMPILE_OPTIONS}) target_compile_definitions(STEPToPoints PUBLIC ${MY_COMPILE_DEFINITIONS}) -target_compile_features(STEPToPoints PRIVATE ${MY_CXX_FEATURE}) \ No newline at end of file +target_compile_features(STEPToPoints PRIVATE ${MY_CXX_FEATURE}) + +if(BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt new file mode 100644 index 0000000..b8b7d54 --- /dev/null +++ b/tests/CMakeLists.txt @@ -0,0 +1,41 @@ +# Test configuration +find_package(GTest CONFIG QUIET) +if (NOT GTest_FOUND) + message(STATUS "GTest not found. Fetching GoogleTest...") + set(GTEST_VERSION_TAG "v1.17.0") + include(FetchContent) + FetchContent_Declare( + googletest + GIT_TAG ${GTEST_VERSION_TAG} + GIT_REPOSITORY https://github.com/google/googletest.git + ) + + # For Windows: Prevent overriding the parent project's compiler/linker settings + set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) + FetchContent_MakeAvailable(googletest) +endif() +message(STATUS "GoogleTest version: ${GTest_VERSION}") + +# Create test executable +add_executable(solid_index_parser_tests + solid_index_parser_tests.cpp + ${CMAKE_SOURCE_DIR}/src/solid_index_parser.cpp +) + +target_include_directories(solid_index_parser_tests PRIVATE + ${CMAKE_SOURCE_DIR}/src +) + +target_link_libraries(solid_index_parser_tests + GTest::gtest + GTest::gtest_main +) + +target_compile_options(solid_index_parser_tests PRIVATE ${MY_COMPILE_OPTIONS}) +target_compile_definitions(solid_index_parser_tests PUBLIC ${MY_COMPILE_DEFINITIONS}) +target_compile_features(solid_index_parser_tests PRIVATE ${MY_CXX_FEATURE}) + +# Register tests with CTest +include(GoogleTest) +gtest_discover_tests(solid_index_parser_tests) + diff --git a/tests/solid_index_parser_tests.cpp b/tests/solid_index_parser_tests.cpp new file mode 100644 index 0000000..c5fee2e --- /dev/null +++ b/tests/solid_index_parser_tests.cpp @@ -0,0 +1,336 @@ +// filepath: /Users/simone/dev/sandbox/pointclouds/STEPToPoints/tests/solid_index_parser_tests.cpp + +#include +#include "solid_index_parser.hpp" +#include +#include + +// ============================================================================ +// Tests for generateRange() +// ============================================================================ + +TEST(GenerateRangeTest, SingleElement) +{ + const auto result = generateRange(1, 1); + ASSERT_EQ(result.size(), 1); + EXPECT_EQ(result[0], 0); // 1-based to 0-based +} + +TEST(GenerateRangeTest, SmallRange) +{ + const auto result = generateRange(1, 5); + const std::vector expected{0, 1, 2, 3, 4}; + EXPECT_EQ(result, expected); +} + +TEST(GenerateRangeTest, LargeRange) +{ + const auto result = generateRange(10, 20); + ASSERT_EQ(result.size(), 11); + EXPECT_EQ(result.front(), 9); // 10 - 1 + EXPECT_EQ(result.back(), 19); // 20 - 1 +} + +TEST(GenerateRangeTest, MiddleRange) +{ + const auto result = generateRange(5, 7); + const std::vector expected{4, 5, 6}; + EXPECT_EQ(result, expected); +} + +TEST(GenerateRangeTest, ConsecutiveIndices) +{ + const auto result = generateRange(100, 101); + const std::vector expected{99, 100}; + EXPECT_EQ(result, expected); +} + +// ============================================================================ +// Tests for parseRange() +// ============================================================================ + +struct ParseRangeTestCase +{ + std::string input; + bool expectSuccess; + std::size_t expectedStart; + std::size_t expectedEnd; + std::string description; +}; + +class ParseRangeParameterizedTest : public ::testing::TestWithParam +{ +}; + +TEST_P(ParseRangeParameterizedTest, ParseRangeVariousInputs) +{ + const auto& testCase = GetParam(); + const auto result = parseRange(testCase.input); + + if(testCase.expectSuccess) + { + ASSERT_TRUE(result.has_value()) << "Failed for: " << testCase.description; + EXPECT_EQ(result->first, testCase.expectedStart) << testCase.description; + EXPECT_EQ(result->second, testCase.expectedEnd) << testCase.description; + } + else + { + EXPECT_FALSE(result.has_value()) << "Should fail for: " << testCase.description; + } +} + +INSTANTIATE_TEST_SUITE_P( + ValidRanges, + ParseRangeParameterizedTest, + ::testing::Values( + ParseRangeTestCase{"1-5", true, 1, 5, "Simple range"}, + ParseRangeTestCase{"1-1", true, 1, 1, "Single element range"}, + ParseRangeTestCase{"10-20", true, 10, 20, "Two digit numbers"}, + ParseRangeTestCase{"100-200", true, 100, 200, "Three digit numbers"}, + ParseRangeTestCase{"1-1000", true, 1, 1000, "Large range"}, + ParseRangeTestCase{"999-1001", true, 999, 1001, "Large indices"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + InvalidRanges, + ParseRangeParameterizedTest, + ::testing::Values( + ParseRangeTestCase{"5-3", false, 0, 0, "End before start"}, + ParseRangeTestCase{"0-5", false, 0, 0, "Start is zero"}, + ParseRangeTestCase{"0-0", false, 0, 0, "Both zero"}, + ParseRangeTestCase{"-5", false, 0, 0, "Missing start"}, + ParseRangeTestCase{"5-", false, 0, 0, "Missing end"}, + ParseRangeTestCase{"-", false, 0, 0, "Only dash"}, + ParseRangeTestCase{"--", false, 0, 0, "Double dash"}, + ParseRangeTestCase{"a-5", false, 0, 0, "Non-numeric start"}, + ParseRangeTestCase{"5-b", false, 0, 0, "Non-numeric end"}, + ParseRangeTestCase{"a-b", false, 0, 0, "Both non-numeric"}, + ParseRangeTestCase{"1.5-3.5", false, 0, 0, "Decimal numbers"}, + ParseRangeTestCase{"1 - 5", false, 0, 0, "Spaces in range"}, + ParseRangeTestCase{"", false, 0, 0, "Empty string"}, + ParseRangeTestCase{"5", false, 0, 0, "Single number without dash"}, + ParseRangeTestCase{"1-2-3", false, 0, 0, "Multiple dashes"}, + ParseRangeTestCase{"-1-5", false, 0, 0, "Negative start"}, + ParseRangeTestCase{"1--5", false, 0, 0, "Double dash in middle"} + ) +); + +TEST(ParseRangeTest, VeryLargeNumbers) +{ + // Test with numbers that might overflow + const auto result = parseRange("999999999999999999999-999999999999999999999"); + EXPECT_FALSE(result.has_value()); // Should fail due to out_of_range +} + +// ============================================================================ +// Tests for parseSolidIndex() +// ============================================================================ + +struct ParseSolidIndexTestCase +{ + std::string input; + std::size_t maxIndex; + bool expectSuccess; + std::vector expectedIndices; + std::string description; +}; + +class ParseSolidIndexParameterizedTest : public ::testing::TestWithParam +{ +}; + +TEST_P(ParseSolidIndexParameterizedTest, ParseSolidIndexVariousInputs) +{ + const auto& testCase = GetParam(); + const auto result = parseSolidIndex(testCase.input, testCase.maxIndex); + + if(testCase.expectSuccess) + { + ASSERT_TRUE(result.has_value()) << "Failed for: " << testCase.description; + EXPECT_EQ(*result, testCase.expectedIndices) << testCase.description; + } + else + { + EXPECT_FALSE(result.has_value()) << "Should fail for: " << testCase.description; + } +} + +INSTANTIATE_TEST_SUITE_P( + ValidSingleIndices, + ParseSolidIndexParameterizedTest, + ::testing::Values( + ParseSolidIndexTestCase{"1", 10, true, {0}, "First index"}, + ParseSolidIndexTestCase{"5", 10, true, {4}, "Middle index"}, + ParseSolidIndexTestCase{"10", 10, true, {9}, "Last index"}, + ParseSolidIndexTestCase{"1", 1, true, {0}, "Only one element available"}, + ParseSolidIndexTestCase{"100", 100, true, {99}, "Large single index"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + ValidRangeIndices, + ParseSolidIndexParameterizedTest, + ::testing::Values( + ParseSolidIndexTestCase{"1-5", 10, true, {0, 1, 2, 3, 4}, "Simple range"}, + ParseSolidIndexTestCase{"1-1", 10, true, {0}, "Single element range"}, + ParseSolidIndexTestCase{"8-10", 10, true, {7, 8, 9}, "Range at end"}, + ParseSolidIndexTestCase{"1-10", 10, true, {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, "Full range"}, + ParseSolidIndexTestCase{"5-7", 20, true, {4, 5, 6}, "Range in middle"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + InvalidIndices, + ParseSolidIndexParameterizedTest, + ::testing::Values( + ParseSolidIndexTestCase{"0", 10, false, {}, "Zero index"}, + ParseSolidIndexTestCase{"11", 10, false, {}, "Index exceeds max"}, + ParseSolidIndexTestCase{"100", 10, false, {}, "Index far exceeds max"}, + ParseSolidIndexTestCase{"-1", 10, false, {}, "Negative index"}, + ParseSolidIndexTestCase{"", 10, false, {}, "Empty string"}, + ParseSolidIndexTestCase{"abc", 10, false, {}, "Non-numeric string"}, + ParseSolidIndexTestCase{"1.5", 10, false, {}, "Decimal number"}, + ParseSolidIndexTestCase{"1 5", 10, false, {}, "Space in number"}, + ParseSolidIndexTestCase{" 5", 10, false, {}, "Leading space"}, + ParseSolidIndexTestCase{"5 ", 10, false, {}, "Trailing space"}, + ParseSolidIndexTestCase{"1,5", 10, false, {}, "Comma separator"}, + ParseSolidIndexTestCase{"1+5", 10, false, {}, "Plus sign"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + InvalidRangeIndices, + ParseSolidIndexParameterizedTest, + ::testing::Values( + ParseSolidIndexTestCase{"1-15", 10, false, {}, "Range end exceeds max"}, + ParseSolidIndexTestCase{"5-3", 10, false, {}, "Range end before start"}, + ParseSolidIndexTestCase{"0-5", 10, false, {}, "Range starts at zero"}, + ParseSolidIndexTestCase{"11-15", 10, false, {}, "Range completely out of bounds"}, + ParseSolidIndexTestCase{"-5", 10, false, {}, "Missing range start"}, + ParseSolidIndexTestCase{"5-", 10, false, {}, "Missing range end"}, + ParseSolidIndexTestCase{"-", 10, false, {}, "Only dash"}, + ParseSolidIndexTestCase{"a-5", 10, false, {}, "Non-numeric range start"}, + ParseSolidIndexTestCase{"5-b", 10, false, {}, "Non-numeric range end"} + ) +); + +TEST(ParseSolidIndexTest, MaxIndexZero) +{ + // Edge case: no solids available + const auto result = parseSolidIndex("1", 0); + EXPECT_FALSE(result.has_value()); +} + +TEST(ParseSolidIndexTest, LargeValidRange) +{ + const auto result = parseSolidIndex("1-100", 100); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->size(), 100); + EXPECT_EQ(result->front(), 0); + EXPECT_EQ(result->back(), 99); +} + +TEST(ParseSolidIndexTest, ConsecutiveCalls) +{ + // Test that function doesn't maintain state between calls + const auto result1 = parseSolidIndex("1", 10); + const auto result2 = parseSolidIndex("5", 10); + const auto result3 = parseSolidIndex("1-3", 10); + + ASSERT_TRUE(result1.has_value()); + ASSERT_TRUE(result2.has_value()); + ASSERT_TRUE(result3.has_value()); + + EXPECT_EQ(*result1, std::vector{0}); + EXPECT_EQ(*result2, std::vector{4}); + EXPECT_EQ(*result3, std::vector({0, 1, 2})); +} + +// ============================================================================ +// Integration Tests +// ============================================================================ + +TEST(SolidIndexParserIntegrationTest, RangeToIndexConversion) +{ + // Test that parseRange -> generateRange -> matches parseSolidIndex + const std::string input = "5-10"; + const std::size_t maxIndex = 20; + + const auto rangeOpt = parseRange(input); + ASSERT_TRUE(rangeOpt.has_value()); + + const auto [start, end] = *rangeOpt; + const auto generatedIndices = generateRange(start, end); + + const auto parsedIndices = parseSolidIndex(input, maxIndex); + ASSERT_TRUE(parsedIndices.has_value()); + + EXPECT_EQ(generatedIndices, *parsedIndices); +} + +TEST(SolidIndexParserIntegrationTest, BoundaryConditions) +{ + struct BoundaryTest + { + std::string input; + std::size_t maxIndex; + bool shouldSucceed; + }; + + const std::vector tests = { + {"1", 1, true}, // Minimum valid case + {"1", 2, true}, // First of multiple + {"2", 2, true}, // Last of multiple + {"3", 2, false}, // Just over boundary + {"1-1", 1, true}, // Single element range + {"1-2", 2, true}, // Full range + {"1-3", 2, false}, // Range exceeds max + {"2-2", 2, true}, // Range at boundary + {"2-3", 2, false}, // Range starts at boundary, exceeds + }; + + for(const auto& test : tests) + { + const auto result = parseSolidIndex(test.input, test.maxIndex); + if(test.shouldSucceed) + { + EXPECT_TRUE(result.has_value()) + << "Failed for input: " << test.input << " maxIndex: " << test.maxIndex; + } + else + { + EXPECT_FALSE(result.has_value()) + << "Should have failed for input: " << test.input << " maxIndex: " << test.maxIndex; + } + } +} + +TEST(SolidIndexParserIntegrationTest, ResultSizeConsistency) +{ + struct SizeTest + { + std::string input; + std::size_t maxIndex; + std::size_t expectedSize; + }; + + const std::vector tests = { + {"1", 10, 1}, + {"5", 10, 1}, + {"1-1", 10, 1}, + {"1-5", 10, 5}, + {"1-10", 10, 10}, + {"5-9", 10, 5}, + {"8-10", 10, 3}, + }; + + for(const auto& test : tests) + { + const auto result = parseSolidIndex(test.input, test.maxIndex); + ASSERT_TRUE(result.has_value()) << "Failed for: " << test.input; + EXPECT_EQ(result->size(), test.expectedSize) + << "Size mismatch for input: " << test.input; + } +} + From 8eae66cefefd35bb93668273df0767e8c99879d1 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:46:03 +0100 Subject: [PATCH 08/11] [ci] add permissions --- .github/workflows/build.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f3b5c4c..119fa05 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -7,6 +7,11 @@ on: schedule: - cron: "40 7 * * 1" +permissions: + contents: read + checks: write + pull-requests: write + env: VCPKG_BINARY_SOURCES: 'clear;x-gha,readwrite' From ceae3d2012121bff5083f2e45840e42a69f2ee1e Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 13:46:32 +0100 Subject: [PATCH 09/11] [cmake][ci] add presets for test and add tests to ci --- .github/workflows/build.yml | 10 ++-- CMakePresets.json | 91 +++++++++++++++++++++++++++++++++---- 2 files changed, 89 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 119fa05..41eaee6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -54,11 +54,13 @@ jobs: - name: Build (Release) uses: lukka/run-cmake@v10 with: - configurePreset: 'release-preset' - buildPreset: 'release-preset' + configurePreset: 'vcpkg-release' + buildPreset: 'vcpkg-release' + testPreset: 'vcpkg-release' - name: Build (Debug) uses: lukka/run-cmake@v10 with: - configurePreset: 'debug-preset' - buildPreset: 'debug-preset' + configurePreset: 'vcpkg-debug' + buildPreset: 'vcpkg-debug' + testPreset: 'vcpkg-debug' diff --git a/CMakePresets.json b/CMakePresets.json index d278a6b..8eab7d5 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -8,6 +8,36 @@ "configurePresets": [ { "name": "default", + "description": "default preset that disable vcpkg, so that the dependencies are not built automatically", + "hidden": true, + "binaryDir": "${sourceDir}/build/${presetName}", + "cacheVariables": { + "CMAKE_TOOLCHAIN_FILE": { + "type": "FILEPATH", + "value": "" + } + } + }, + { + "name": "debug", + "description": "Debug build", + "inherits": "default", + "binaryDir": "${sourceDir}/build/debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug" + } + }, + { + "name": "release", + "description": "Release build", + "inherits": "default", + "binaryDir": "${sourceDir}/build/release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release" + } + }, + { + "name": "vcpkg-default", "hidden": true, "binaryDir": "${sourceDir}/build/${presetName}", "cacheVariables": { @@ -22,8 +52,8 @@ } }, { - "name": "release-preset", - "inherits": "default", + "name": "vcpkg-release", + "inherits": "vcpkg-default", "displayName": "Release Build", "description": "Release build configuration", "cacheVariables": { @@ -31,8 +61,8 @@ } }, { - "name": "debug-preset", - "inherits": "default", + "name": "vcpkg-debug", + "inherits": "vcpkg-default", "displayName": "Debug Build", "description": "Debug build configuration", "cacheVariables": { @@ -42,16 +72,61 @@ ], "buildPresets": [ { - "name": "release-preset", - "configurePreset": "release-preset", + "name": "release", + "configurePreset": "release", + "description": "Build in Release mode" + }, + { + "name": "debug", + "configurePreset": "debug", + "description": "Build in Debug mode" + }, + { + "name": "vcpkg-release", + "configurePreset": "vcpkg-release", "displayName": "Release Build", "description": "Build in Release mode" }, { - "name": "debug-preset", - "configurePreset": "debug-preset", + "name": "vcpkg-debug", + "configurePreset": "vcpkg-debug", "displayName": "Debug Build", "description": "Build in Debug mode" } + ], + "testPresets": [ + { + "name": "test-default", + "description": "default test", + "hidden": true, + "output": {"outputOnFailure": true}, + "execution": {"noTestsAction": "error", "stopOnFailure": true} + }, + { + "name": "release", + "description": "Test in Release mode", + "configurePreset": "release", + "inherits": "test-default" + }, + { + "name": "debug", + "description": "Test in Debug mode", + "configurePreset": "debug", + "inherits": "test-default" + }, + { + "name": "vcpkg-release", + "description": "Test in Release mode with vcpkg", + "configurePreset": "vcpkg-release", + "configuration": "Release", + "inherits": "test-default" + }, + { + "name": "vcpkg-debug", + "description": "Test in Debug mode with vcpkg", + "configurePreset": "vcpkg-debug", + "configuration": "Debug", + "inherits": "test-default" + } ] } From d2927986627f45bd8250cd947d2035472d3a90aa Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 14:16:19 +0100 Subject: [PATCH 10/11] reduce cyclomatic complexity --- src/solid_index_parser.cpp | 161 +++++++++++++--------- src/solid_index_parser.hpp | 25 ++++ tests/solid_index_parser_tests.cpp | 207 +++++++++++++++++++++++++++++ 3 files changed, 328 insertions(+), 65 deletions(-) diff --git a/src/solid_index_parser.cpp b/src/solid_index_parser.cpp index 07b1fb3..bc9867e 100644 --- a/src/solid_index_parser.cpp +++ b/src/solid_index_parser.cpp @@ -15,85 +15,85 @@ auto generateRange(std::size_t start, std::size_t end) -> std::vector std::optional> +auto validateRangeString(const std::string& str) -> bool +{ + return !str.empty() && std::isdigit(static_cast(str[0])); +} + + +auto parseRangeValue(const std::string& str, const std::string& fullInput) -> std::optional { - if(const auto dashPos = sel.find('-'); dashPos != std::string::npos && dashPos > 0) + try { - try - { - const auto startStr = sel.substr(0, dashPos); - const auto endStr = sel.substr(dashPos + 1); - - // Check for empty strings or strings with only whitespace - if(startStr.empty() || endStr.empty()) - { - std::cerr << "Invalid range format: " << sel << "\n"; - return std::nullopt; - } - - // Check for leading/trailing whitespace or non-digit characters at start - if(!std::isdigit(static_cast(startStr[0])) || - !std::isdigit(static_cast(endStr[0]))) - { - std::cerr << "Invalid range format: " << sel << "\n"; - return std::nullopt; - } - - std::size_t startPos = 0; - std::size_t endPos = 0; - const auto start = std::stoul(startStr, &startPos); - const auto end = std::stoul(endStr, &endPos); - - // Ensure the entire string was consumed (no trailing characters) - if(startPos != startStr.length() || endPos != endStr.length()) - { - std::cerr << "Invalid range format: " << sel << "\n"; - return std::nullopt; - } - - if(start >= 1 && start <= end) - { - return std::make_pair(start, end); - } - } - catch(const std::invalid_argument&) - { - std::cerr << "Invalid range format: " << sel << "\n"; - } - catch(const std::out_of_range&) + std::size_t pos = 0; + const auto value = std::stoul(str, &pos); + + // Ensure the entire string was consumed (no trailing characters) + if(pos != str.length()) { - std::cerr << "Range values out of bounds: " << sel << "\n"; + std::cerr << "Invalid range format: " << fullInput << "\n"; + return std::nullopt; } + + return value; + } + catch(const std::invalid_argument&) + { + std::cerr << "Invalid range format: " << fullInput << "\n"; + } + catch(const std::out_of_range&) + { + std::cerr << "Range values out of bounds: " << fullInput << "\n"; } return std::nullopt; } -auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional> +auto parseRange(const std::string& sel) -> std::optional> { - // Try parsing as range first - if(const auto range = parseRange(sel)) + const auto dashPos = sel.find('-'); + if(dashPos == std::string::npos || dashPos == 0) { - const auto [start, end] = range.value(); + return std::nullopt; + } - if(end > maxIndex) - { - std::cerr << "Range end exceeds max index: " << end << " > " << maxIndex << "\n"; - return std::nullopt; - } + const auto startStr = sel.substr(0, dashPos); + const auto endStr = sel.substr(dashPos + 1); - return generateRange(start, end); + if(!validateRangeString(startStr) || !validateRangeString(endStr)) + { + std::cerr << "Invalid range format: " << sel << "\n"; + return std::nullopt; } - try + const auto start = parseRangeValue(startStr, sel); + const auto end = parseRangeValue(endStr, sel); + + if(!start.has_value() || !end.has_value()) { - // Check for empty string or leading whitespace/non-digit - if(sel.empty() || !std::isdigit(static_cast(sel[0]))) - { - std::cerr << "Invalid index provided: " << sel << "\n"; - return std::nullopt; - } + return std::nullopt; + } + if(start.value() >= 1 && start.value() <= end.value()) + { + return std::make_pair(start.value(), end.value()); + } + + return std::nullopt; +} + + +auto parseSingleIndex(const std::string& sel) -> std::optional +{ + // Check for empty string or leading whitespace/non-digit + if(sel.empty() || !std::isdigit(static_cast(sel[0]))) + { + std::cerr << "Invalid index provided: " << sel << "\n"; + return std::nullopt; + } + + try + { std::size_t pos = 0; const auto index = std::stoul(sel, &pos); @@ -104,11 +104,11 @@ auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optio return std::nullopt; } - if(index >= 1 && index <= maxIndex) + if(index >= 1) { - return {{index - 1}}; + return index; } - std::cerr << "Index out of valid range: " << sel << "\n"; + std::cerr << "Index must be at least 1: " << sel << "\n"; } catch(const std::invalid_argument&) { @@ -121,3 +121,34 @@ auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optio return std::nullopt; } + +auto parseSolidIndex(const std::string& sel, std::size_t maxIndex) -> std::optional> +{ + // Try parsing as range first + if(const auto range = parseRange(sel)) + { + const auto [start, end] = range.value(); + + if(end > maxIndex) + { + std::cerr << "Range end exceeds max index: " << end << " > " << maxIndex << "\n"; + return std::nullopt; + } + + return generateRange(start, end); + } + + // Try parsing as single index + if(const auto index = parseSingleIndex(sel); index.has_value()) + { + if(index.value() <= maxIndex) + { + return {{index.value() - 1}}; + } + std::cerr << "Index out of valid range: " << sel << "\n"; + } + + return std::nullopt; +} + + diff --git a/src/solid_index_parser.hpp b/src/solid_index_parser.hpp index 7b91d63..2cdff44 100644 --- a/src/solid_index_parser.hpp +++ b/src/solid_index_parser.hpp @@ -12,6 +12,23 @@ */ auto generateRange(std::size_t start, std::size_t end) -> std::vector; +/** + * @brief Validates that a string is non-empty and starts with a digit. + * + * @param str The string to validate. + * @return bool True if valid, false otherwise. + */ +auto validateRangeString(const std::string& str) -> bool; + +/** + * @brief Parses a string into a size_t value with validation. + * + * @param str The string to parse. + * @param fullInput The original full input string for error messages. + * @return std::optional The parsed value if valid, std::nullopt otherwise. + */ +auto parseRangeValue(const std::string& str, const std::string& fullInput) -> std::optional; + /** * @brief Parses a range string (e.g., "3-7") into start and end indices. * @@ -20,6 +37,14 @@ auto generateRange(std::size_t start, std::size_t end) -> std::vector std::optional>; +/** + * @brief Parses a single index string (e.g., "3") into a 1-based index. + * + * @param sel The string to parse. + * @return std::optional The 1-based index if valid, std::nullopt otherwise. + */ +auto parseSingleIndex(const std::string& sel) -> std::optional; + /** * @brief Attempts to parse a string as a solid index (1-based) or a range. * diff --git a/tests/solid_index_parser_tests.cpp b/tests/solid_index_parser_tests.cpp index c5fee2e..14c5a7a 100644 --- a/tests/solid_index_parser_tests.cpp +++ b/tests/solid_index_parser_tests.cpp @@ -45,6 +45,106 @@ TEST(GenerateRangeTest, ConsecutiveIndices) EXPECT_EQ(result, expected); } +// ============================================================================ +// Tests for validateRangeString() +// ============================================================================ + +TEST(ValidateRangeStringTest, ValidStrings) +{ + EXPECT_TRUE(validateRangeString("1")); + EXPECT_TRUE(validateRangeString("42")); + EXPECT_TRUE(validateRangeString("999")); + EXPECT_TRUE(validateRangeString("12345")); + EXPECT_TRUE(validateRangeString("0")); // Even though 0 is not a valid index, this just checks format +} + +TEST(ValidateRangeStringTest, InvalidStrings) +{ + EXPECT_FALSE(validateRangeString("")); + EXPECT_FALSE(validateRangeString(" ")); + EXPECT_FALSE(validateRangeString(" 5")); + EXPECT_FALSE(validateRangeString("a5")); + EXPECT_FALSE(validateRangeString("-5")); + EXPECT_FALSE(validateRangeString("+5")); + EXPECT_FALSE(validateRangeString(".5")); +} + +// ============================================================================ +// Tests for parseRangeValue() +// ============================================================================ + +struct ParseRangeValueTestCase +{ + std::string input; + std::string fullInput; + bool expectSuccess; + std::size_t expectedValue; + std::string description; +}; + +class ParseRangeValueParameterizedTest : public ::testing::TestWithParam +{ +}; + +TEST_P(ParseRangeValueParameterizedTest, ParseRangeValueVariousInputs) +{ + const auto& testCase = GetParam(); + const auto result = parseRangeValue(testCase.input, testCase.fullInput); + + if(testCase.expectSuccess) + { + ASSERT_TRUE(result.has_value()) << "Failed for: " << testCase.description; + EXPECT_EQ(result.value(), testCase.expectedValue) << testCase.description; + } + else + { + EXPECT_FALSE(result.has_value()) << "Should fail for: " << testCase.description; + } +} + +INSTANTIATE_TEST_SUITE_P( + ValidValues, + ParseRangeValueParameterizedTest, + ::testing::Values( + ParseRangeValueTestCase{"1", "1-5", true, 1, "Minimum value"}, + ParseRangeValueTestCase{"5", "1-5", true, 5, "Small value"}, + ParseRangeValueTestCase{"42", "42-100", true, 42, "Two digit value"}, + ParseRangeValueTestCase{"999", "999-1000", true, 999, "Three digit value"}, + ParseRangeValueTestCase{"12345", "12345-20000", true, 12345, "Large value"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + InvalidValues, + ParseRangeValueParameterizedTest, + ::testing::Values( + ParseRangeValueTestCase{"1abc", "1abc-5", false, 0, "Trailing letters"}, + ParseRangeValueTestCase{"1.5", "1.5-5", false, 0, "Decimal value"}, + ParseRangeValueTestCase{"1 ", "1 -5", false, 0, "Trailing space"} + ) +); + +TEST(ParseRangeValueTest, VeryLargeNumber) +{ + const auto result = parseRangeValue("999999999999999999999", "999999999999999999999-1"); + EXPECT_FALSE(result.has_value()); // Should fail due to out_of_range +} + +TEST(ParseRangeValueTest, ConsecutiveCalls) +{ + const auto result1 = parseRangeValue("1", "1-5"); + const auto result2 = parseRangeValue("42", "42-100"); + const auto result3 = parseRangeValue("999", "999-1000"); + + ASSERT_TRUE(result1.has_value()); + ASSERT_TRUE(result2.has_value()); + ASSERT_TRUE(result3.has_value()); + + EXPECT_EQ(result1.value(), 1); + EXPECT_EQ(result2.value(), 42); + EXPECT_EQ(result3.value(), 999); +} + // ============================================================================ // Tests for parseRange() // ============================================================================ @@ -123,6 +223,113 @@ TEST(ParseRangeTest, VeryLargeNumbers) EXPECT_FALSE(result.has_value()); // Should fail due to out_of_range } +// ============================================================================ +// Tests for parseSingleIndex() +// ============================================================================ + +struct ParseSingleIndexTestCase +{ + std::string input; + bool expectSuccess; + std::size_t expectedValue; + std::string description; +}; + +class ParseSingleIndexParameterizedTest : public ::testing::TestWithParam +{ +}; + +TEST_P(ParseSingleIndexParameterizedTest, ParseSingleIndexVariousInputs) +{ + const auto& testCase = GetParam(); + const auto result = parseSingleIndex(testCase.input); + + if(testCase.expectSuccess) + { + ASSERT_TRUE(result.has_value()) << "Failed for: " << testCase.description; + EXPECT_EQ(result.value(), testCase.expectedValue) << testCase.description; + } + else + { + EXPECT_FALSE(result.has_value()) << "Should fail for: " << testCase.description; + } +} + +INSTANTIATE_TEST_SUITE_P( + ValidIndices, + ParseSingleIndexParameterizedTest, + ::testing::Values( + ParseSingleIndexTestCase{"1", true, 1, "Minimum valid index"}, + ParseSingleIndexTestCase{"5", true, 5, "Small index"}, + ParseSingleIndexTestCase{"10", true, 10, "Two digit index"}, + ParseSingleIndexTestCase{"100", true, 100, "Three digit index"}, + ParseSingleIndexTestCase{"999", true, 999, "Large index"}, + ParseSingleIndexTestCase{"12345", true, 12345, "Very large index"} + ) +); + +INSTANTIATE_TEST_SUITE_P( + InvalidIndices, + ParseSingleIndexParameterizedTest, + ::testing::Values( + ParseSingleIndexTestCase{"0", false, 0, "Zero index"}, + ParseSingleIndexTestCase{"-1", false, 0, "Negative index"}, + ParseSingleIndexTestCase{"-10", false, 0, "Large negative index"}, + ParseSingleIndexTestCase{"", false, 0, "Empty string"}, + ParseSingleIndexTestCase{" ", false, 0, "Single space"}, + ParseSingleIndexTestCase{" ", false, 0, "Multiple spaces"}, + ParseSingleIndexTestCase{" 5", false, 0, "Leading space"}, + ParseSingleIndexTestCase{"5 ", false, 0, "Trailing space"}, + ParseSingleIndexTestCase{" 5 ", false, 0, "Leading and trailing spaces"}, + ParseSingleIndexTestCase{"abc", false, 0, "Alphabetic string"}, + ParseSingleIndexTestCase{"1abc", false, 0, "Number with trailing letters"}, + ParseSingleIndexTestCase{"abc1", false, 0, "Letters with trailing number"}, + ParseSingleIndexTestCase{"1.5", false, 0, "Decimal number"}, + ParseSingleIndexTestCase{"1.0", false, 0, "Decimal with zero fraction"}, + ParseSingleIndexTestCase{"1,5", false, 0, "Comma separator"}, + ParseSingleIndexTestCase{"1+5", false, 0, "Plus sign"}, + ParseSingleIndexTestCase{"1-5", false, 0, "Minus/dash (looks like range)"}, + ParseSingleIndexTestCase{"1 5", false, 0, "Space between digits"}, + ParseSingleIndexTestCase{"1\t5", false, 0, "Tab between digits"}, + ParseSingleIndexTestCase{"\n5", false, 0, "Newline before number"}, + ParseSingleIndexTestCase{"5\n", false, 0, "Newline after number"} + ) +); + +TEST(ParseSingleIndexTest, VeryLargeNumber) +{ + // Test with a number that might overflow + const auto result = parseSingleIndex("999999999999999999999"); + EXPECT_FALSE(result.has_value()); // Should fail due to out_of_range +} + +TEST(ParseSingleIndexTest, ConsecutiveCalls) +{ + // Test that function doesn't maintain state between calls + const auto result1 = parseSingleIndex("1"); + const auto result2 = parseSingleIndex("42"); + const auto result3 = parseSingleIndex("100"); + + ASSERT_TRUE(result1.has_value()); + ASSERT_TRUE(result2.has_value()); + ASSERT_TRUE(result3.has_value()); + + EXPECT_EQ(result1.value(), 1); + EXPECT_EQ(result2.value(), 42); + EXPECT_EQ(result3.value(), 100); +} + +TEST(ParseSingleIndexTest, BoundaryValue) +{ + // Test the boundary at 1 + const auto resultZero = parseSingleIndex("0"); + const auto resultOne = parseSingleIndex("1"); + + EXPECT_FALSE(resultZero.has_value()); + ASSERT_TRUE(resultOne.has_value()); + EXPECT_EQ(resultOne.value(), 1); +} + // ============================================================================ // Tests for parseSolidIndex() // ============================================================================ From e8679be7df9efbacfb4c48e0de9af4aed71f9c05 Mon Sep 17 00:00:00 2001 From: Simone Gasparini Date: Thu, 20 Nov 2025 14:28:41 +0100 Subject: [PATCH 11/11] further reduce cyclomatic complexity --- src/solid_index_parser.cpp | 10 ++++++++-- src/solid_index_parser.hpp | 9 +++++++++ tests/solid_index_parser_tests.cpp | 21 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/solid_index_parser.cpp b/src/solid_index_parser.cpp index bc9867e..51842e6 100644 --- a/src/solid_index_parser.cpp +++ b/src/solid_index_parser.cpp @@ -49,6 +49,12 @@ auto parseRangeValue(const std::string& str, const std::string& fullInput) -> st } +auto validateRangeBounds(std::size_t start, std::size_t end) -> bool +{ + return start >= 1 && start <= end; +} + + auto parseRange(const std::string& sel) -> std::optional> { const auto dashPos = sel.find('-'); @@ -74,9 +80,9 @@ auto parseRange(const std::string& sel) -> std::optional= 1 && start.value() <= end.value()) + if(validateRangeBounds(start.value(), end.value())) { - return std::make_pair(start.value(), end.value()); + return {{start.value(), end.value()}}; } return std::nullopt; diff --git a/src/solid_index_parser.hpp b/src/solid_index_parser.hpp index 2cdff44..0ebdca6 100644 --- a/src/solid_index_parser.hpp +++ b/src/solid_index_parser.hpp @@ -29,6 +29,15 @@ auto validateRangeString(const std::string& str) -> bool; */ auto parseRangeValue(const std::string& str, const std::string& fullInput) -> std::optional; +/** + * @brief Validates that a range has valid bounds (start >= 1 and start <= end). + * + * @param start The start value. + * @param end The end value. + * @return bool True if bounds are valid, false otherwise. + */ +auto validateRangeBounds(std::size_t start, std::size_t end) -> bool; + /** * @brief Parses a range string (e.g., "3-7") into start and end indices. * diff --git a/tests/solid_index_parser_tests.cpp b/tests/solid_index_parser_tests.cpp index 14c5a7a..2d9e106 100644 --- a/tests/solid_index_parser_tests.cpp +++ b/tests/solid_index_parser_tests.cpp @@ -145,6 +145,27 @@ TEST(ParseRangeValueTest, ConsecutiveCalls) EXPECT_EQ(result3.value(), 999); } +// ============================================================================ +// Tests for validateRangeBounds() +// ============================================================================ + +TEST(ValidateRangeBoundsTest, ValidBounds) +{ + EXPECT_TRUE(validateRangeBounds(1, 1)); // Single element + EXPECT_TRUE(validateRangeBounds(1, 5)); // Normal range + EXPECT_TRUE(validateRangeBounds(1, 100)); // Large range + EXPECT_TRUE(validateRangeBounds(5, 10)); // Mid-range + EXPECT_TRUE(validateRangeBounds(100, 200)); // Large values +} + +TEST(ValidateRangeBoundsTest, InvalidBounds) +{ + EXPECT_FALSE(validateRangeBounds(0, 5)); // Start is zero + EXPECT_FALSE(validateRangeBounds(0, 0)); // Both zero + EXPECT_FALSE(validateRangeBounds(5, 3)); // End before start + EXPECT_FALSE(validateRangeBounds(10, 5)); // End well before start +} + // ============================================================================ // Tests for parseRange() // ============================================================================