From ed7d79b0f5e8d8d9aff7504360a3a8944a05a354 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Mon, 3 Apr 2023 16:54:09 -0500 Subject: [PATCH] App/Toponaming: Minor refactoring and tests for StringID --- src/App/StringHasher.cpp | 2 +- src/App/StringHasher.h | 29 +- tests/CMakeLists.txt | 1 + tests/src/App/CMakeLists.txt | 3 +- tests/src/App/StringHasher.cpp | 559 ++++++++++++++++++++++++++++++++- tests/src/Base/Reader.cpp | 27 +- 6 files changed, 592 insertions(+), 29 deletions(-) diff --git a/src/App/StringHasher.cpp b/src/App/StringHasher.cpp index 7fb8f3a83d..107b6d8216 100644 --- a/src/App/StringHasher.cpp +++ b/src/App/StringHasher.cpp @@ -102,7 +102,7 @@ PyObject* StringID::getPyObject() PyObject* StringID::getPyObjectWithIndex(int index) { - auto* res = new StringIDPy(this); + auto res = new StringIDPy(this); res->_index = index; return res; } diff --git a/src/App/StringHasher.h b/src/App/StringHasher.h index f99d515a96..1f24e97754 100644 --- a/src/App/StringHasher.h +++ b/src/App/StringHasher.h @@ -35,6 +35,8 @@ #include #include +#include + namespace Data { @@ -170,12 +172,19 @@ public: { return _data; } + /// Returns the postfix QByteArray postfix() const { return _postfix; } + /// Sets the postfix + void setPostfix(QByteArray postfix) + { + _postfix = std::move(postfix); + } + PyObject* getPyObject() override; /// Returns a Python tuple containing both the text and index PyObject* getPyObjectWithIndex(int index); @@ -186,7 +195,7 @@ public: * The format is #. And if index is non zero, then #:. Both * and are in hex format. */ - std::string toString(int index) const; + std::string toString(int index = 0) const; /// Light weight structure of holding a string ID and associated index struct IndexID @@ -244,20 +253,18 @@ public: std::string dataToText(int index) const; /** Get the content of this StringID as QByteArray - * @param bytes: output bytes * @param index: optional index. */ - void toBytes(QByteArray& bytes, int index) const + QByteArray dataToBytes(int index = 0) const { + QByteArray res(_data); + if (index != 0) { + res += QByteArray::number(index); + } if (_postfix.size() != 0) { - bytes = _data + _postfix; - } - else if (index != 0) { - bytes = _data + QByteArray::number(index); - } - else { - bytes = _data; + res += _postfix; } + return res; } /// Mark this StringID as used @@ -530,7 +537,7 @@ public: void toBytes(QByteArray& bytes) const { if (_sid) { - _sid->toBytes(bytes, _index); + bytes = _sid->dataToBytes(_index); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 72fada6d0f..d6d828382d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -37,6 +37,7 @@ endif() add_executable(Tests_run) add_subdirectory(lib) add_subdirectory(src) +target_include_directories(Tests_run PUBLIC ${Python3_INCLUDE_DIRS}) target_link_libraries(Tests_run gtest_main ${Google_Tests_LIBS} FreeCADApp) add_executable(Sketcher_tests_run) diff --git a/tests/src/App/CMakeLists.txt b/tests/src/App/CMakeLists.txt index b6b1782bdc..e14c01958f 100644 --- a/tests/src/App/CMakeLists.txt +++ b/tests/src/App/CMakeLists.txt @@ -1,6 +1,7 @@ target_sources( Tests_run PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR}/Application.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Branding.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Expression.cpp ${CMAKE_CURRENT_SOURCE_DIR}/ElementMap.cpp @@ -9,5 +10,5 @@ target_sources( ${CMAKE_CURRENT_SOURCE_DIR}/MappedElement.cpp ${CMAKE_CURRENT_SOURCE_DIR}/MappedName.cpp ${CMAKE_CURRENT_SOURCE_DIR}/Metadata.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/Application.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/StringHasher.cpp ) diff --git a/tests/src/App/StringHasher.cpp b/tests/src/App/StringHasher.cpp index d0e198c3ae..287767ae67 100644 --- a/tests/src/App/StringHasher.cpp +++ b/tests/src/App/StringHasher.cpp @@ -1,3 +1,556 @@ -// -// Created by Chris Hennes on 4/1/23. -// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "gtest/gtest.h" + +#include +#include + +#include + +class StringIDTest: public ::testing::Test +{ +protected: + // void SetUp() override {} + // void TearDown() override {} + + static App::StringID givenFlaggedStringID(App::StringID::Flag flag) + { + const long value {42}; + const QByteArray data {"data", 4}; + return App::StringID {value, data, flag}; + } +}; + +TEST_F(StringIDTest, stringIDManualConstructionNoFlags) +{ + // Arrange + const long expectedValue {42}; + const QByteArray expectedData {"data", 4}; + + // Act + auto id = App::StringID(expectedValue, expectedData); + + // Assert + EXPECT_EQ(expectedValue, id.value()); + EXPECT_EQ(expectedData, id.data()); + EXPECT_FALSE(id.isBinary()); +} + +TEST_F(StringIDTest, stringIDManualConstructionWithFlag) +{ + // Arrange + const long expectedValue {42}; + const QByteArray expectedData {"data", 4}; + const App::StringID::Flags expectedFlags {App::StringID::Flag::Binary}; + + // Act + auto id = App::StringID(expectedValue, expectedData, expectedFlags); + + // Assert + EXPECT_EQ(expectedValue, id.value()); + EXPECT_EQ(expectedData, id.data()); + EXPECT_TRUE(id.isBinary()); +} + +TEST_F(StringIDTest, stringIDDefaultConstruction) +{ + // Arrange & Act + auto id = App::StringID(); + + // Assert + EXPECT_EQ(0, id.value()); +} + +TEST_F(StringIDTest, value) +{ + // Arrange + const long expectedValueA {0}; + auto idA = App::StringID(expectedValueA, nullptr); + const long expectedValueB {42}; + auto idB = App::StringID(expectedValueB, nullptr); + const long expectedValueC {314159}; + auto idC = App::StringID(expectedValueC, nullptr); + + // Act + auto valueA = idA.value(); + auto valueB = idB.value(); + auto valueC = idC.value(); + + // Assert + EXPECT_EQ(expectedValueA, valueA); + EXPECT_EQ(expectedValueB, valueB); + EXPECT_EQ(expectedValueC, valueC); +} + +TEST_F(StringIDTest, relatedIDs) +{ + // Nothing to test -- relatedIDs are storage-only in this class +} + +TEST_F(StringIDTest, isBinary) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Binary); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isBinary()); + EXPECT_FALSE(controlID.isBinary()); +} + +TEST_F(StringIDTest, isHashed) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Hashed); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isHashed()); + EXPECT_FALSE(controlID.isHashed()); +} + +TEST_F(StringIDTest, isPostfixed) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Postfixed); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isPostfixed()); + EXPECT_FALSE(controlID.isPostfixed()); +} + +TEST_F(StringIDTest, isPostfixEncoded) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::PostfixEncoded); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isPostfixEncoded()); + EXPECT_FALSE(controlID.isPostfixEncoded()); +} + +TEST_F(StringIDTest, isIndexed) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Indexed); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isIndexed()); + EXPECT_FALSE(controlID.isIndexed()); +} + +TEST_F(StringIDTest, isPrefixID) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::PrefixID); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isPrefixID()); + EXPECT_FALSE(controlID.isPrefixID()); +} + +TEST_F(StringIDTest, isPrefixIDIndex) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::PrefixIDIndex); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isPrefixIDIndex()); + EXPECT_FALSE(controlID.isPrefixIDIndex()); +} + +TEST_F(StringIDTest, isMarked) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Marked); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isMarked()); + EXPECT_FALSE(controlID.isMarked()); +} + +TEST_F(StringIDTest, isPersistent) +{ + // Arrange + auto flaggedID = givenFlaggedStringID(App::StringID::Flag::Persistent); + auto controlID = App::StringID {}; + + // Act & Assert + EXPECT_TRUE(flaggedID.isPersistent()); + EXPECT_FALSE(controlID.isPersistent()); +} + +TEST_F(StringIDTest, isFromSameHasher) +{ + // Nothing to test except when used by StringHasher +} + +TEST_F(StringIDTest, getHasher) +{ + // Nothing to test except when used by StringHasher +} + +TEST_F(StringIDTest, data) +{ + // Arrange + QByteArray expectedData {"data", 4}; + auto id = App::StringID(1, expectedData); + + // Act + auto data = id.data(); + + // Assert + EXPECT_EQ(expectedData, data); +} + +TEST_F(StringIDTest, postfix) +{ + // Nothing to test except when used by StringHasher +} + +TEST_F(StringIDTest, getPyObject) +{ + // Arrange + Py_Initialize(); + auto id = new App::StringID(1, nullptr); + id->ref(); + + // Act + Py::Object py(id->getPyObject(), true); + id->unref(); + + // Assert + EXPECT_TRUE(PyObject_TypeCheck(py.ptr(), &App::StringIDPy::Type)); +} + +TEST_F(StringIDTest, getPyObjectWithIndex) +{ + // Arrange + Py_Initialize(); + auto id = new App::StringID(1, nullptr); + id->ref(); + + // Act + Py::Object py(id->getPyObjectWithIndex(2), true); + id->unref(); + + // Assert + ASSERT_TRUE(PyObject_TypeCheck(py.ptr(), &App::StringIDPy::Type)); +} + +TEST_F(StringIDTest, toStringWithoutIndex) +{ + // Arrange + const long bigHex = 0xfcad10; + auto idA = App::StringID(1, QByteArray {"data", 4}); + auto idB = App::StringID(bigHex, QByteArray {"data", 4}); + + // Act + auto resultA = idA.toString(); + auto resultB = idB.toString(); + + // Assert + EXPECT_EQ(std::string("#1"), resultA); + EXPECT_EQ(std::string("#fcad10"), resultB);// Make sure result is in hex +} + +TEST_F(StringIDTest, toStringWithIndex) +{ + // Arrange + const long bigHex = 0xfcad10; + auto id = App::StringID(1, QByteArray {"data", 4}); + + // Act + auto resultA = id.toString(bigHex); + auto resultB = id.toString(0); + + // Assert + EXPECT_EQ(std::string("#1:fcad10"), resultA); + EXPECT_EQ(std::string("#1"), resultB); +} + +TEST_F(StringIDTest, fromStringWithEOFAndLengthGood) +{ + // Arrange + const std::string testString {"#1:fcad"}; + + // Act + auto result = + App::StringID::fromString(testString.c_str(), true, static_cast(testString.length())); + + // Assert + EXPECT_EQ(result.id, 1); + EXPECT_EQ(result.index, 0xfcad); +} + +TEST_F(StringIDTest, fromStringExtraData) +{ + // Arrange + const std::string testString {"#1:fcad#2:bad"}; + + // Act + auto trueResult = + App::StringID::fromString(testString.c_str(), true, static_cast(testString.length())); + auto falseResult = + App::StringID::fromString(testString.c_str(), false, static_cast(testString.length())); + + // Assert + EXPECT_EQ(trueResult.id, -1); + EXPECT_EQ(falseResult.id, 1); +} + +TEST_F(StringIDTest, fromStringLengthUnspecified) +{ + // Arrange + const std::string testString {"#1:fcad#2:bad"}; + + // Act + auto trueResult = App::StringID::fromString(testString.c_str(), true); + auto falseResult = App::StringID::fromString(testString.c_str(), false); + + // Assert + EXPECT_EQ(trueResult.id, -1); + EXPECT_EQ(falseResult.id, 1); +} + +TEST_F(StringIDTest, fromStringShorterLength) +{ + // Arrange + const int dataLength {7}; + const std::string testString {"#1:fcad#2:bad"}; + + // Act + auto trueResult = App::StringID::fromString(testString.c_str(), true, dataLength); + auto falseResult = App::StringID::fromString(testString.c_str(), false, dataLength); + + // Assert + EXPECT_EQ(trueResult.id, 1); + EXPECT_EQ(falseResult.id, 1); +} + +TEST_F(StringIDTest, fromStringNoHashtag) +{ + // Arrange + const std::string testString {"1:fcad"}; + + // Act + auto result = App::StringID::fromString(testString.c_str(), true); + + // Assert + EXPECT_EQ(result.id, -1); +} + +TEST_F(StringIDTest, fromStringNotHex) +{ + // Arrange + const std::string testStringA {"1:freecad"}; + const std::string testStringB {"zoink:2"}; + + // Act + auto resultA = App::StringID::fromString(testStringA.c_str(), false); + auto resultB = App::StringID::fromString(testStringB.c_str(), false); + + // Assert + EXPECT_EQ(resultA.id, -1); + EXPECT_EQ(resultB.id, -1); +} + +TEST_F(StringIDTest, fromStringQByteArray) +{ + // Arrange + const QByteArray testString {"#1:fcad", 7}; + + // Act + auto result = App::StringID::fromString(testString, true); + + // Assert + EXPECT_EQ(result.id, 1); + EXPECT_EQ(result.index, 0xfcad); +} + +TEST_F(StringIDTest, dataToTextHashed) +{ + // Arrange + QByteArray buffer {"120ca87015d849dbea060eaf2295fcc4ee981427", 40}; // NOLINT + auto id = App::StringID(1, buffer, App::StringID::Flag::Hashed); + + // Act + auto result = id.dataToText(0); + + // Assert + EXPECT_EQ(result, buffer.toBase64().constData()); +} + +TEST_F(StringIDTest, dataToTextBinary) +{ + // Arrange + QByteArray buffer {"120ca87015d849dbea060eaf2295fcc4ee981427", 40}; // NOLINT + auto id = App::StringID(1, buffer, App::StringID::Flag::Binary); + + // Act + auto result = id.dataToText(0); + + // Assert + EXPECT_EQ(result, buffer.toBase64().constData()); +} + +TEST_F(StringIDTest, dataToTextNoIndex) +{ + // Arrange + QByteArray data{"data", 4}; + auto id = App::StringID(1, data); + + // Act + auto result = id.dataToText(0); + + // Assert + EXPECT_EQ(result, "data"); +} + +TEST_F(StringIDTest, dataToTextWithIndex) +{ + // Arrange + QByteArray data{"data", 4}; + auto id = App::StringID(1, data); + + // Act + auto resultA = id.dataToText(1); + auto resultB = id.dataToText(1024); // NOLINT + + // Assert + EXPECT_EQ(resultA, "data1"); + EXPECT_EQ(resultB, "data1024"); // Not hex! +} + +TEST_F(StringIDTest, dataToTextWithPostfix) +{ + // Arrange + QByteArray data{"data", 4}; + QByteArray postfix{"postfix", 7}; // NOLINT + auto id = App::StringID(1, data); + id.setPostfix(postfix); + + // Act + auto result = id.dataToText(1); + + // Assert + EXPECT_EQ(result, "data1postfix"); +} + +TEST_F(StringIDTest, dataToBytesNoIndex) +{ + // Arrange + QByteArray data{"data", 4}; + auto id = App::StringID(1, data); + + // Act + auto result = id.dataToBytes(); + + // Assert + EXPECT_EQ(data, result); +} + +TEST_F(StringIDTest, dataToBytesWithIndex) +{ + // Arrange + QByteArray data{"data", 4}; + const int index {1234}; + auto id = App::StringID(1, data); + + // Act + auto result = id.dataToBytes(index); + + // Assert + EXPECT_EQ(data + QByteArray::number(index), result); +} + +TEST_F(StringIDTest, dataToBytesWithPostfix) +{ + // Arrange + QByteArray data{"data", 4}; + QByteArray postfix{"postfix", 7}; // NOLINT + auto id = App::StringID(1, data); + id.setPostfix(postfix); + + // Act + auto result = id.dataToBytes(); + + // Assert + EXPECT_EQ(data + postfix, result); +} + +TEST_F(StringIDTest, dataToBytesWithIndexAndPostfix) +{ + // Arrange + QByteArray data{"data", 4}; + QByteArray postfix{"postfix", 7}; // NOLINT + const int index {1234}; + auto id = App::StringID(1, data); + id.setPostfix(postfix); + + // Act + auto result = id.dataToBytes(index); + + // Assert + EXPECT_EQ(data + QByteArray::number(index) + postfix, result); +} + +TEST_F(StringIDTest, mark) +{ + // Arrange + QByteArray data{"data", 4}; + auto id = App::StringID(1, data); + ASSERT_FALSE(id.isMarked()); + + // Act + id.mark(); + + // Assert + EXPECT_TRUE(id.isMarked()); +} + +TEST_F(StringIDTest, setPersistent) +{ + // Arrange + QByteArray data{"data", 4}; + auto id = App::StringID(1, data); + ASSERT_FALSE(id.isPersistent()); + + // Act + id.setPersistent(true); + + // Assert + EXPECT_TRUE(id.isPersistent()); +} + +TEST_F(StringIDTest, operatorLessThan) +{ + // Can't test without a _hasher +} + +TEST_F(StringIDTest, compare) +{ + // Can't test without a _hasher +} + + +class StringIDRefTest: public ::testing::Test +{ +protected: + // void SetUp() override {} + // void TearDown() override {} +}; + + +class StringHasherTest: public ::testing::Test +{ +protected: + // void SetUp() override {} + // void TearDown() override {} +}; \ No newline at end of file diff --git a/tests/src/Base/Reader.cpp b/tests/src/Base/Reader.cpp index 9db994be6a..47b5b926c9 100644 --- a/tests/src/Base/Reader.cpp +++ b/tests/src/Base/Reader.cpp @@ -30,7 +30,8 @@ protected: void givenDataAsXMLStream(const std::string& data) { - auto stringData = R"()" + data + ""; + auto stringData = + R"()" + data + ""; std::istringstream stream(stringData); std::ofstream fileStream(_tempFile); fileStream.write(stringData.data(), static_cast(stringData.length())); @@ -88,7 +89,7 @@ TEST_F(ReaderTest, beginCharStreamOpenClose) Reader()->readElement("data"); // Act - auto& result = Reader()->beginCharStream(); // Not an error, even though there is no data + auto& result = Reader()->beginCharStream();// Not an error, even though there is no data // Assert EXPECT_TRUE(result.good()); @@ -113,7 +114,7 @@ TEST_F(ReaderTest, charStreamGood) Reader()->beginCharStream(); // Act - auto &result = Reader()->charStream(); + auto& result = Reader()->charStream(); // Assert EXPECT_TRUE(result.good()); @@ -137,7 +138,7 @@ TEST_F(ReaderTest, endCharStreamGood) Reader()->beginCharStream(); // Act & Assert - Reader()->endCharStream(); // Does not throw + Reader()->endCharStream();// Does not throw } TEST_F(ReaderTest, endCharStreamBad) @@ -148,7 +149,7 @@ TEST_F(ReaderTest, endCharStreamBad) // Do not open the stream... // Act & Assert - Reader()->endCharStream(); // Does not throw, even with no open stream + Reader()->endCharStream();// Does not throw, even with no open stream } TEST_F(ReaderTest, readDataSmallerThanBuffer) @@ -159,7 +160,7 @@ TEST_F(ReaderTest, readDataSmallerThanBuffer) givenDataAsXMLStream("" + expectedData + ""); Reader()->readElement("data"); Reader()->beginCharStream(); - std::array buffer{}; + std::array buffer {}; // Act auto bytesRead = Reader()->read(buffer.data(), bufferSize); @@ -177,7 +178,7 @@ TEST_F(ReaderTest, readDataLargerThanBuffer) givenDataAsXMLStream("" + expectedData + ""); Reader()->readElement("data"); Reader()->beginCharStream(); - std::array buffer{}; + std::array buffer {}; // Act auto bytesRead = Reader()->read(buffer.data(), bufferSize); @@ -197,15 +198,15 @@ TEST_F(ReaderTest, readDataLargerThanBufferSecondRead) givenDataAsXMLStream("" + expectedData + ""); Reader()->readElement("data"); Reader()->beginCharStream(); - std::array buffer{}; - Reader()->read(buffer.data(), bufferSize); // Read the first five bytes + std::array buffer {}; + Reader()->read(buffer.data(), bufferSize);// Read the first five bytes // Act - auto bytesRead = Reader()->read(buffer.data(), bufferSize); // Second five bytes + auto bytesRead = Reader()->read(buffer.data(), bufferSize);// Second five bytes // Assert for (size_t i = 0; i < bufferSize; ++i) { - EXPECT_EQ(expectedData[i+bufferSize], buffer.at(i)); + EXPECT_EQ(expectedData[i + bufferSize], buffer.at(i)); } EXPECT_EQ(bufferSize, bytesRead); } @@ -218,11 +219,11 @@ TEST_F(ReaderTest, readDataNotStarted) std::string expectedData {"Test ASCII data"}; givenDataAsXMLStream("" + expectedData + ""); Reader()->readElement("data"); - std::array buffer{}; + std::array buffer {}; // Act auto bytesRead = Reader()->read(buffer.data(), bufferSize); // Assert - EXPECT_EQ(-1, bytesRead); // Because we didn't call beginCharStream + EXPECT_EQ(-1, bytesRead);// Because we didn't call beginCharStream }