This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/674-improve-properties in repository https://gitbox.apache.org/repos/asf/celix.git
commit 45b12107e172a269f99757e62043f8152e831303 Author: Pepijn Noltes <[email protected]> AuthorDate: Sun Jan 21 19:18:57 2024 +0100 Add addition properties unit tests --- libs/utils/gtest/src/CxxPropertiesTestSuite.cc | 24 +++++- .../gtest/src/PropertiesErrorInjectionTestSuite.cc | 94 +++++++++++++++++++++- libs/utils/gtest/src/PropertiesTestSuite.cc | 18 +++++ libs/utils/src/properties.c | 13 ++- libs/utils/src/version.c | 3 +- 5 files changed, 139 insertions(+), 13 deletions(-) diff --git a/libs/utils/gtest/src/CxxPropertiesTestSuite.cc b/libs/utils/gtest/src/CxxPropertiesTestSuite.cc index ea910f95..d41342a1 100644 --- a/libs/utils/gtest/src/CxxPropertiesTestSuite.cc +++ b/libs/utils/gtest/src/CxxPropertiesTestSuite.cc @@ -195,7 +195,7 @@ TEST_F(CxxPropertiesTestSuite, GetTest) { props.set("key4", true); //bool props.set("key5", celix::Version{1, 2, 3}); //version - //Test get with valid key + //Test getAs with valid key EXPECT_EQ(props.get("key1"), "value1"); EXPECT_EQ(props.getAsLong("key2", -1), 2); EXPECT_EQ(props.getAsDouble("key3", -1), 3.3); @@ -203,6 +203,21 @@ TEST_F(CxxPropertiesTestSuite, GetTest) { celix::Version checkVersion{1, 2, 3}; EXPECT_EQ(props.getAsVersion("key5", celix::Version{1, 2, 4}), checkVersion); + //Test get with valid key + EXPECT_EQ(props.get("key1"), "value1"); + EXPECT_EQ(props.getLong("key2", -1), 2); + EXPECT_EQ(props.getDouble("key3", -1), 3.3); + EXPECT_EQ(props.getBool("key4", false), true); + EXPECT_EQ(props.getVersion("key5", celix::Version{1, 2, 4}), checkVersion); + + // Test get type + EXPECT_EQ(props.getType("key1"), celix::Properties::ValueType::String); + EXPECT_EQ(props.getType("key2"), celix::Properties::ValueType::Long); + EXPECT_EQ(props.getType("key3"), celix::Properties::ValueType::Double); + EXPECT_EQ(props.getType("key4"), celix::Properties::ValueType::Bool); + EXPECT_EQ(props.getType("key5"), celix::Properties::ValueType::Version); + EXPECT_EQ(props.getType("non-existing"), celix::Properties::ValueType::Unset); + // Test get with invalid key and default value EXPECT_EQ(props.get("non_existent_key", "default_value"), "default_value"); EXPECT_EQ(props.getLong("non_existent_key", 1), 1); @@ -232,6 +247,13 @@ TEST_F(CxxPropertiesTestSuite, ArrayListTest) { props.setVersions("key5", {celix::Version{1, 2, 3}, celix::Version{2, 3, 4}}); EXPECT_EQ(5, props.size()); + // Test get type + EXPECT_EQ(props.getType("key1"), celix::Properties::ValueType::StringArray); + EXPECT_EQ(props.getType("key2"), celix::Properties::ValueType::LongArray); + EXPECT_EQ(props.getType("key3"), celix::Properties::ValueType::DoubleArray); + EXPECT_EQ(props.getType("key4"), celix::Properties::ValueType::BooleanArray); + EXPECT_EQ(props.getType("key5"), celix::Properties::ValueType::VersionArray); + // Test getAs with valid key auto strings = props.getAsStringVector("key1"); EXPECT_EQ(strings.size(), 2); diff --git a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc index b167f7f1..16778ec3 100644 --- a/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesErrorInjectionTestSuite.cc @@ -28,12 +28,12 @@ #include "celix_utils_private_constants.h" #include "celix_version.h" +#include "celix_array_list_ei.h" #include "celix_string_hash_map_ei.h" #include "celix_utils_ei.h" +#include "celix_version_ei.h" #include "malloc_ei.h" #include "stdio_ei.h" -#include "celix_utils_ei.h" -#include "celix_version_ei.h" class PropertiesErrorInjectionTestSuite : public ::testing::Test { public: @@ -49,6 +49,9 @@ class PropertiesErrorInjectionTestSuite : public ::testing::Test { celix_ei_expect_ftell(nullptr, 0, 0); celix_ei_expect_celix_utils_strdup(nullptr, 0, nullptr); celix_ei_expect_celix_stringHashMap_put(nullptr, 0, 0); + celix_ei_expect_celix_version_copy(nullptr, 0, nullptr); + celix_ei_expect_celix_arrayList_createWithOptions(nullptr, 0, nullptr); + celix_ei_expect_celix_arrayList_copy(nullptr, 0, nullptr); } /** @@ -130,6 +133,12 @@ TEST_F(PropertiesErrorInjectionTestSuite, SetFailureTest) { // Then the celix_properties_set call fails ASSERT_EQ(celix_properties_set(props, "key", "value"), CELIX_ENOMEM); + // When a celix_utils_strdup error injection is when calling celix_properties_set, for the creation of the + // key string. + celix_ei_expect_celix_utils_strdup((void*)celix_properties_createString, 0, nullptr, 2); + // Then the celix_properties_set call fails + ASSERT_EQ(celix_properties_set(props, "key", "value"), CELIX_ENOMEM); + // C++ API // Given a celix properties object with a filled optimization cache celix::Properties cxxProps{}; @@ -256,6 +265,87 @@ TEST_F(PropertiesErrorInjectionTestSuite, LoadFailureTest) { fclose(memStream); } +TEST_F(PropertiesErrorInjectionTestSuite, GetAsVersionWithVersionCopyFailedTest) { + //Given a properties set with a version + celix_autoptr(celix_properties_t) props = celix_properties_create(); + celix_version_t* v = celix_version_create(1, 2, 3, "qualifier"); + celix_properties_assignVersion(props, "key", v); + + // When a celix_version_copy error injection is set for celix_properties_getAsVersion + celix_ei_expect_celix_version_copy((void*)celix_properties_getAsVersion, 0, nullptr); + + // Then the celix_properties_getAsVersion call fails + celix_version_t* version = nullptr; + auto status = celix_properties_getAsVersion(props, "key", nullptr, &version); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, version); +} + +TEST_F(PropertiesErrorInjectionTestSuite, GetAsArrayWithArrayListCopyFailedTest) { + //Given a properties set with a string array, long array, double array, bool array and version array + const char* str1 = "string1"; + const char* str2 = "string2"; + const char* stringsArray[] = {str1, str2}; + long longsArray[] = {1, 2, 3}; + double doublesArray[] = {1.1, 2.2, 3.3}; + bool boolsArray[] = {true, false, true}; + celix_autoptr(celix_version_t) v = celix_version_create(1, 2, 3, "qualifier"); + const celix_version_t* versionsArray[] = {v, v, v}; + + celix_autoptr(celix_properties_t) props = celix_properties_create(); + celix_properties_setStrings(props, "stringArray", stringsArray, 2); + celix_properties_setLongs(props, "longArray", longsArray, 3); + celix_properties_setDoubles(props, "doubleArray", doublesArray, 3); + celix_properties_setBooleans(props, "boolArray", boolsArray, 3); + celix_properties_setVersions(props, "versionArray", versionsArray, 3); + + // When a celix_arrayList_createWithOptions error injection is set for celix_properties_getAsStringArrayList + celix_ei_expect_celix_arrayList_createWithOptions((void*)celix_properties_getAsStringArrayList, 1, nullptr); + + // Then the celix_properties_getAsStringArrayList call fails + celix_array_list_t* strings = nullptr; + auto status = celix_properties_getAsStringArrayList(props, "stringArray", nullptr, &strings); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, strings); + + //When a celix_arrayList_copy error injection is set for celix_properties_getAsLongArrayList + celix_ei_expect_celix_arrayList_copy((void*)celix_properties_getAsLongArrayList, 0, nullptr); + + // Then the celix_properties_getAsLongArrayList call fails + celix_array_list_t* longs = nullptr; + status = celix_properties_getAsLongArrayList(props, "longArray", nullptr, &longs); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, longs); + + //When a celix_arrayList_copy error injection is set for celix_properties_getAsDoubleArrayList + celix_ei_expect_celix_arrayList_copy((void*)celix_properties_getAsDoubleArrayList, 0, nullptr); + + // Then the celix_properties_getAsDoubleArrayList call fails + celix_array_list_t* doubles = nullptr; + status = celix_properties_getAsDoubleArrayList(props, "doubleArray", nullptr, &doubles); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, doubles); + + //When a celix_arrayList_copy error injection is set for celix_properties_getAsBoolArrayList + celix_ei_expect_celix_arrayList_copy((void*)celix_properties_getAsBoolArrayList, 0, nullptr); + + // Then the celix_properties_getAsBoolArrayList call fails + celix_array_list_t* bools = nullptr; + status = celix_properties_getAsBoolArrayList(props, "boolArray", nullptr, &bools); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, bools); + + //When a celix_arrayList_createWithOptions error injection is set for celix_properties_getAsVersionArrayList + celix_ei_expect_celix_arrayList_createWithOptions((void*)celix_properties_getAsVersionArrayList, 1, nullptr); + + // Then the celix_properties_getAsVersionArrayList call fails + celix_array_list_t* versions = nullptr; + status = celix_properties_getAsVersionArrayList(props, "versionArray", nullptr, &versions); + ASSERT_EQ(status, CELIX_ENOMEM); + ASSERT_EQ(nullptr, versions); +} + + TEST_F(PropertiesErrorInjectionTestSuite, AssignFailureTest) { //Given a filled properties and a key and value celix_autoptr(celix_properties_t) props = celix_properties_create(); diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 999b9a28..1b02bc15 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -624,6 +624,24 @@ TEST_F(PropertiesTestSuite, SetEntryTest) { celix_properties_destroy(props2); } +TEST_F(PropertiesTestSuite, SetEntryWithLargeStringValueTest) { + //Test if the version and double with a large string representation are correctly set + //(whitebox test to check if the fallback to string allocation works) + celix_autoptr(celix_properties_t) props1 = celix_properties_create(); + + double doubleVal = 1.23456789E+307; + celix_properties_setDouble(props1, "key1", doubleVal); + EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_DOUBLE, celix_properties_getType(props1, "key1")); + EXPECT_DOUBLE_EQ(doubleVal, celix_properties_getAsDouble(props1, "key1", 0.0)); + + celix_autoptr(celix_version_t) version = + celix_version_create(1, 2, 3, "a-qualifier-that-is-longer-than-20-characters"); + celix_properties_setVersion(props1, "key2", version); + EXPECT_EQ(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props1, "key2")); + EXPECT_EQ(0, celix_version_compareTo(version, celix_properties_getVersion(props1, "key2", nullptr))); +} + + TEST_F(PropertiesTestSuite, PropertiesAutoCleanupTest) { celix_autoptr(celix_properties_t) props = celix_properties_create(); } diff --git a/libs/utils/src/properties.c b/libs/utils/src/properties.c index 436493cd..d502cc2b 100644 --- a/libs/utils/src/properties.c +++ b/libs/utils/src/properties.c @@ -153,7 +153,7 @@ static celix_status_t celix_properties_fillEntry(celix_properties_t* properties, entry->value = celix_properties_createString(properties, convertedValueBuffer); } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) { int written = snprintf(convertedValueBuffer, sizeof(convertedValueBuffer), "%f", entry->typed.doubleValue); - if (written >= 0 || written < sizeof(convertedValueBuffer)) { + if (written >= 0 && written < sizeof(convertedValueBuffer)) { entry->value = celix_properties_createString(properties, convertedValueBuffer); } else { char* val = NULL; @@ -842,12 +842,9 @@ celix_status_t celix_properties_getAsVersion(const celix_properties_t* propertie return CELIX_SUCCESS; } if (entry != NULL && entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING) { - celix_version_t* createdVersion = celix_version_createVersionFromString(entry->value); - //TODO improve error detection for celix_version_createVersionFromString, so that ENOMEM can be returned - //maybe use and improve celix_utils_convertStringToVersion - if (createdVersion) { - *version = createdVersion; - return CELIX_SUCCESS; + celix_status_t parseStatus = celix_version_parse(entry->value, version); + if (parseStatus != CELIX_ILLEGAL_ARGUMENT) { + return parseStatus; } } if (defaultValue) { @@ -1195,7 +1192,7 @@ celix_properties_setStrings(celix_properties_t* properties, const char* key, con return celix_properties_createAndSetEntry(properties, key, &prototype); } -celix_array_list_t* celix_properties_deepCopyStringArrayList(const celix_array_list_t* list) { +static celix_array_list_t* celix_properties_deepCopyStringArrayList(const celix_array_list_t* list) { celix_array_list_create_options_t opts = CELIX_EMPTY_ARRAY_LIST_CREATE_OPTIONS; opts.simpleRemovedCallback = free; opts.initialCapacity = celix_arrayList_size(list); diff --git a/libs/utils/src/version.c b/libs/utils/src/version.c index 096bac05..31423401 100644 --- a/libs/utils/src/version.c +++ b/libs/utils/src/version.c @@ -178,7 +178,6 @@ celix_status_t celix_version_parse(const char *versionStr, celix_version_t** ver *version = NULL; if (celix_utils_isStringNullOrEmpty(versionStr)) { - celix_err_push("Invalid version string. Version string cannot be NULL or empty"); return CELIX_ILLEGAL_ARGUMENT; } @@ -186,7 +185,7 @@ celix_status_t celix_version_parse(const char *versionStr, celix_version_t** ver char* versionWrkStr = celix_utils_writeOrCreateString(buffer, sizeof(buffer), "%s", versionStr); if (!versionWrkStr) { celix_err_push("Failed to allocate memory for celix_version_createVersionFromString"); - return CELIX_ILLEGAL_ARGUMENT; + return CELIX_ENOMEM; } int versionsParts[3] = {0, 0, 0};
