From 15076a6d26a2f1bc74550695741e4574e6292da2 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Wed, 1 Apr 2020 19:17:50 +0100 Subject: [PATCH] Overhaul plugin metadata types' comparison operators Change their implementations to check full object equality, with case insensitivity for filename strings. The filename case insensitivity doesn't extend to filenames in condition strings, as conditions are compared as strings, but the risk of duplicates due to that is much less than the risk of treating non-duplicate objects as duplicates. --- docs/metadata/data_structures/cleaning.rst | 4 +- docs/metadata/data_structures/file.rst | 3 +- .../data_structures/localised_content.rst | 6 ++ docs/metadata/data_structures/location.rst | 3 +- docs/metadata/data_structures/message.rst | 7 +- docs/metadata/data_structures/tag.rst | 3 +- include/loot/metadata/file.h | 9 +- include/loot/metadata/location.h | 8 +- include/loot/metadata/message.h | 8 +- include/loot/metadata/message_content.h | 8 +- include/loot/metadata/plugin_cleaning_data.h | 9 +- include/loot/metadata/tag.h | 8 +- src/api/metadata/file.cpp | 21 +++- src/api/metadata/location.cpp | 12 ++- src/api/metadata/message.cpp | 19 +++- src/api/metadata/message_content.cpp | 12 ++- src/api/metadata/plugin_cleaning_data.cpp | 45 ++++++++- src/api/metadata/tag.cpp | 21 ++-- .../api/interface/database_interface_test.h | 18 +++- src/tests/api/internals/metadata/file_test.h | 95 ++++++++++++++++--- .../api/internals/metadata/location_test.h | 44 ++++++--- .../internals/metadata/message_content_test.h | 59 ++++++++---- .../api/internals/metadata/message_test.h | 93 ++++++++++++++---- .../metadata/plugin_cleaning_data_test.h | 68 +++++++++++-- .../internals/metadata/plugin_metadata_test.h | 4 +- src/tests/api/internals/metadata/tag_test.h | 55 +++++++---- src/tests/api/internals/metadata_list_test.h | 36 +++---- 27 files changed, 521 insertions(+), 157 deletions(-) diff --git a/docs/metadata/data_structures/cleaning.rst b/docs/metadata/data_structures/cleaning.rst index b590d178..cfa24069 100644 --- a/docs/metadata/data_structures/cleaning.rst +++ b/docs/metadata/data_structures/cleaning.rst @@ -42,7 +42,9 @@ This structure holds information on which versions of a plugin are dirty or clea Equality -------- -Two cleaning data structures are equal if the values of their ``crc`` keys are identical. +Two plugin cleaning data structures are equal if all their fields are equal. +`util` field equality is case-sensitive. If the `info` field is a string, it +is treated as a localised content data structure. Examples -------- diff --git a/docs/metadata/data_structures/file.rst b/docs/metadata/data_structures/file.rst index 49a15cad..30b9685a 100644 --- a/docs/metadata/data_structures/file.rst +++ b/docs/metadata/data_structures/file.rst @@ -26,7 +26,8 @@ The scalar form is simply the value of the map form's ``name`` key. Using the sc Equality -------- -Two file data structures are equal if the lowercased values of their ``name`` keys are identical. +Two file data structures are equal if all their fields are equal. ``name`` field +equality is case-insensitive, the other fields use case-sensitive equality. Examples -------- diff --git a/docs/metadata/data_structures/localised_content.rst b/docs/metadata/data_structures/localised_content.rst index 253a2c8f..f78238fc 100644 --- a/docs/metadata/data_structures/localised_content.rst +++ b/docs/metadata/data_structures/localised_content.rst @@ -27,3 +27,9 @@ The localised content data structure is a key-value string map. Spanish es Swedish sv ==================== ===== + +Equality +-------- + +Two localised content data structures are equal if all their fields are equal. +Field equality is case-sensitive. diff --git a/docs/metadata/data_structures/location.rst b/docs/metadata/data_structures/location.rst index 65d0b4e4..64e933e3 100644 --- a/docs/metadata/data_structures/location.rst +++ b/docs/metadata/data_structures/location.rst @@ -22,7 +22,8 @@ The scalar form is simply the value of the map form's ``link`` key. Using the sc Equality -------- -Two location data structures are equal if the values of their ``link`` keys are identical. +Two location data structures are equal if all their fields are equal. Field +equality is case-sensitive. Examples -------- diff --git a/docs/metadata/data_structures/message.rst b/docs/metadata/data_structures/message.rst index 712bc5fd..ed5aa861 100644 --- a/docs/metadata/data_structures/message.rst +++ b/docs/metadata/data_structures/message.rst @@ -57,8 +57,11 @@ If a message's ``content`` value is a string, the message will use the string as Equality -------- -The equality of two message data structures is determined by comparing the values of their ``content`` keys. If a content key is a string, it is treated as a localised content list -containing a single English-language string. The two message data structures are then equal if their localised content lists are identical. +Two message data structures are equal if their `type`, `content` and `condition` +fields are equal, after any `subs` values have been substituted into `content` +strings. If the `content` field is a string, it is treated as a localised +content list containing a single English-language string. String equality is +case sensitive. Examples -------- diff --git a/docs/metadata/data_structures/tag.rst b/docs/metadata/data_structures/tag.rst index 468f3623..541adc5b 100644 --- a/docs/metadata/data_structures/tag.rst +++ b/docs/metadata/data_structures/tag.rst @@ -22,7 +22,8 @@ The scalar form is simply the value of the map form's ``name`` key. Using the sc Equality -------- -Two tag data structures are equal if the values of their ``name`` keys are identical. +Two tag data structures are equal if all their fields are equal. String equality +is case-sensitive. Examples -------- diff --git a/include/loot/metadata/file.h b/include/loot/metadata/file.h index ef51b1cf..bf06e062 100644 --- a/include/loot/metadata/file.h +++ b/include/loot/metadata/file.h @@ -58,15 +58,14 @@ public: /** * A less-than operator implemented with no semantics so that File objects can * be stored in sets. - * @returns True if this File's name is case-insensitively lexicographically - * less than the given File's name, false otherwise. + * @returns True if this File is less than the given File, false otherwise. */ LOOT_API bool operator<(const File& rhs) const; /** - * Check if two File objects are equal by comparing their filenames. - * @returns True if the filenames are case-insensitively equal, false - * otherwise. + * Check if two File objects are equal by comparing their fields. + * @returns True if the filenames are case-insensitively equal and all other + * fields are case-sensitively equal, false otherwise. */ LOOT_API bool operator==(const File& rhs) const; diff --git a/include/loot/metadata/location.h b/include/loot/metadata/location.h index f39ed68e..af6dcb1f 100644 --- a/include/loot/metadata/location.h +++ b/include/loot/metadata/location.h @@ -54,14 +54,14 @@ public: /** * A less-than operator implemented with no semantics so that Location objects * can be stored in sets. - * @returns True if this Location's URL string is lexicographically less than - * the given Location's URL string, false otherwise. + * @returns True if this Location is less than the given Location, false + * otherwise. */ LOOT_API bool operator<(const Location& rhs) const; /** - * Check if two Location objects are equal by comparing their URLs. - * @returns True if the URL strings are equal, false otherwise. + * Check if two Location objects are equal by comparing their fields. + * @returns True if the objects' fields are equal, false otherwise. */ LOOT_API bool operator==(const Location& rhs) const; diff --git a/include/loot/metadata/message.h b/include/loot/metadata/message.h index 6df64fa1..9d2ac574 100644 --- a/include/loot/metadata/message.h +++ b/include/loot/metadata/message.h @@ -79,14 +79,14 @@ public: /** * A less-than operator implemented with no semantics so that Message objects * can be stored in sets. - * @returns Returns true if this Message's content is lexicographically less - * than the given Message's content, and false otherwise. + * @returns Returns true if this Message is less than the given Message, and + * false otherwise. */ LOOT_API bool operator<(const Message& rhs) const; /** - * Check if two Message objects are equal by comparing their content. - * @returns True if the contents are equal, false otherwise. + * Check if two Message objects are equal by comparing their fields. + * @returns True if the objects' fields are equal, false otherwise. */ LOOT_API bool operator==(const Message& rhs) const; diff --git a/include/loot/metadata/message_content.h b/include/loot/metadata/message_content.h index aefd4546..2d03b6be 100644 --- a/include/loot/metadata/message_content.h +++ b/include/loot/metadata/message_content.h @@ -73,14 +73,14 @@ public: /** * A less-than operator implemented with no semantics so that MessageContent * objects can be stored in sets. - * @returns True if this MessageContent's text is lexicographically less than - * the given MessageContent's text, false otherwise. + * @returns True if this MessageContent is less than the given + * MessageContent, false otherwise. */ LOOT_API bool operator<(const MessageContent& rhs) const; /** - * Check if two MessageContent objects are equal by comparing their texts. - * @returns True if the texts are equal, false otherwise. + * Check if two MessageContent objects are equal by comparing their fields. + * @returns True if the objects' fields are equal, false otherwise. */ LOOT_API bool operator==(const MessageContent& rhs) const; diff --git a/include/loot/metadata/plugin_cleaning_data.h b/include/loot/metadata/plugin_cleaning_data.h index d8045c1a..789c918c 100644 --- a/include/loot/metadata/plugin_cleaning_data.h +++ b/include/loot/metadata/plugin_cleaning_data.h @@ -84,14 +84,15 @@ public: /** * A less-than operator implemented with no semantics so that * PluginCleaningData objects can be stored in sets. - * @returns True if this PluginCleaningData's CRC is less than the given - * PluginCleaningData's CRC, false otherwise. + * @returns True if this PluginCleaningData is less than the given + * PluginCleaningData, false otherwise. */ LOOT_API bool operator<(const PluginCleaningData& rhs) const; /** - * Check if two PluginCleaningData objects are equal by comparing their CRCs. - * @returns True if the CRCs are equal, false otherwise. + * Check if two PluginCleaningData objects are equal by comparing their + * fields. + * @returns True if the objects' fields are equal, false otherwise. */ LOOT_API bool operator==(const PluginCleaningData& rhs) const; diff --git a/include/loot/metadata/tag.h b/include/loot/metadata/tag.h index a586f0f2..d4b8c6a2 100644 --- a/include/loot/metadata/tag.h +++ b/include/loot/metadata/tag.h @@ -60,17 +60,13 @@ public: /** * A less-than operator implemented with no semantics so that Tag objects * can be stored in sets. - * @returns True if this Tag is suggested for addition and the other is not. - * If both Tags are suggested for addition or both are suggested for - * removal, returns true if this Tag's name is lexicographically less - * than the given Tag's name, false otherwise. + * @returns True if this Tag is less than the given Tag, false otherwise. */ LOOT_API bool operator<(const Tag& rhs) const; /** * Check if two Tag objects are equal. - * @returns True if both Tags are suggested for addition or both are suggested - * for removal, and the Tag names are equal, false otherwise. + * @returns True if the objects' fields are equal, false otherwise. */ LOOT_API bool operator==(const Tag& rhs) const; diff --git a/src/api/metadata/file.cpp b/src/api/metadata/file.cpp index db906aed..79c159e9 100644 --- a/src/api/metadata/file.cpp +++ b/src/api/metadata/file.cpp @@ -24,8 +24,8 @@ #include "loot/metadata/file.h" -#include "api/metadata/yaml/file.h" #include "api/helpers/text.h" +#include "api/metadata/yaml/file.h" namespace loot { File::File() {} @@ -38,11 +38,28 @@ File::File(const std::string& name, ConditionalMetadata(condition) {} bool File::operator<(const File& rhs) const { + if (display_ < rhs.display_) { + return true; + } + + if (rhs.display_ < display_) { + return false; + } + + if (this->GetCondition() < rhs.GetCondition()) { + return true; + } + + if (rhs.GetCondition() < this->GetCondition()) { + return false; + } + return CompareFilenames(name_, rhs.name_) < 0; } bool File::operator==(const File& rhs) const { - return CompareFilenames(name_, rhs.name_) == 0; + return display_ == rhs.display_ && GetCondition() == rhs.GetCondition() && + CompareFilenames(name_, rhs.name_) == 0; } std::string File::GetName() const { return name_; } diff --git a/src/api/metadata/location.cpp b/src/api/metadata/location.cpp index e2416a90..b1c15bb1 100644 --- a/src/api/metadata/location.cpp +++ b/src/api/metadata/location.cpp @@ -32,11 +32,19 @@ Location::Location(const std::string& url, const std::string& name) : name_(name) {} bool Location::operator<(const Location& rhs) const { - return url_ < rhs.url_; + if (url_ < rhs.url_) { + return true; + } + + if (rhs.url_ < url_) { + return false; + } + + return name_ < rhs.name_; } bool Location::operator==(const Location& rhs) const { - return url_ == rhs.url_; + return url_ == rhs.url_ && name_ == rhs.name_; } std::string Location::GetURL() const { return url_; } diff --git a/src/api/metadata/message.cpp b/src/api/metadata/message.cpp index 0a130068..3597b083 100644 --- a/src/api/metadata/message.cpp +++ b/src/api/metadata/message.cpp @@ -59,11 +59,28 @@ Message::Message(const MessageType type, } bool Message::operator<(const Message& rhs) const { + if (type_ < rhs.type_) { + return true; + } + + if (rhs.type_ < type_) { + return false; + } + + if (GetCondition() < rhs.GetCondition()) { + return true; + } + + if (rhs.GetCondition() < GetCondition()) { + return false; + } + return content_ < rhs.GetContent(); } bool Message::operator==(const Message& rhs) const { - return content_ == rhs.GetContent(); + return type_ == rhs.type_ && GetCondition() == rhs.GetCondition() && + content_ == rhs.GetContent(); } MessageType Message::GetType() const { return type_; } diff --git a/src/api/metadata/message_content.cpp b/src/api/metadata/message_content.cpp index 019e0fd8..f640a51f 100644 --- a/src/api/metadata/message_content.cpp +++ b/src/api/metadata/message_content.cpp @@ -41,11 +41,19 @@ std::string MessageContent::GetText() const { return text_; } std::string MessageContent::GetLanguage() const { return language_; } bool MessageContent::operator<(const MessageContent& rhs) const { - return text_ < rhs.text_; + if (text_ < rhs.text_) { + return true; + } + + if (rhs.text_ < text_) { + return false; + } + + return language_ < rhs.language_; } bool MessageContent::operator==(const MessageContent& rhs) const { - return text_ == rhs.text_; + return text_ == rhs.text_ && language_ == rhs.language_; } MessageContent MessageContent::Choose(const std::vector content, const std::string& language) { diff --git a/src/api/metadata/plugin_cleaning_data.cpp b/src/api/metadata/plugin_cleaning_data.cpp index cad06bf8..52cd80b8 100644 --- a/src/api/metadata/plugin_cleaning_data.cpp +++ b/src/api/metadata/plugin_cleaning_data.cpp @@ -53,11 +53,52 @@ PluginCleaningData::PluginCleaningData(uint32_t crc, info_(info) {} bool PluginCleaningData::operator<(const PluginCleaningData& rhs) const { - return crc_ < rhs.GetCRC(); + if (crc_ < rhs.crc_) { + return true; + } + + if (rhs.crc_ < crc_) { + return false; + } + + if (utility_ < rhs.utility_) { + return true; + } + + if (rhs.utility_ < utility_) { + return false; + } + + if (itm_ < rhs.itm_) { + return true; + } + + if (rhs.itm_ < itm_) { + return false; + } + + if (ref_ < rhs.ref_) { + return true; + } + + if (rhs.ref_ < ref_) { + return false; + } + + if (nav_ < rhs.nav_) { + return true; + } + + if (rhs.nav_ < nav_) { + return false; + } + + return info_ < rhs.info_; } bool PluginCleaningData::operator==(const PluginCleaningData& rhs) const { - return crc_ == rhs.GetCRC(); + return crc_ == rhs.crc_ && utility_ == rhs.utility_ && info_ == rhs.info_ && + itm_ == rhs.itm_ && ref_ == rhs.ref_ && nav_ == rhs.nav_; } uint32_t PluginCleaningData::GetCRC() const { return crc_; } diff --git a/src/api/metadata/tag.cpp b/src/api/metadata/tag.cpp index 93440a02..be48540c 100644 --- a/src/api/metadata/tag.cpp +++ b/src/api/metadata/tag.cpp @@ -37,15 +37,24 @@ Tag::Tag(const std::string& tag, ConditionalMetadata(condition) {} bool Tag::operator<(const Tag& rhs) const { - if (addTag_ != rhs.IsAddition()) - return addTag_ && !rhs.IsAddition(); - else - return GetName() < rhs.GetName(); + if (addTag_ != rhs.addTag_) { + return addTag_ && !rhs.addTag_; + } + + if (name_ < rhs.name_) { + return true; + } + + if (rhs.name_ < name_) { + return false; + } + + return GetCondition() < rhs.GetCondition(); } bool Tag::operator==(const Tag& rhs) const { - return addTag_ == rhs.IsAddition() && - GetName() == rhs.GetName(); + return addTag_ == rhs.addTag_ && name_ == rhs.name_ && + GetCondition() == rhs.GetCondition(); } bool Tag::IsAddition() const { return addTag_; } diff --git a/src/tests/api/interface/database_interface_test.h b/src/tests/api/interface/database_interface_test.h index ca766546..01c0c50f 100644 --- a/src/tests/api/interface/database_interface_test.h +++ b/src/tests/api/interface/database_interface_test.h @@ -529,7 +529,9 @@ TEST_P(DatabaseInterfaceTest, auto messages = db_->GetGeneralMessages(); std::vector expectedMessages({ - Message(MessageType::say, generalMasterlistMessage), + Message(MessageType::say, + generalMasterlistMessage, + "file(\"" + missingEsp + "\")"), Message(MessageType::say, generalUserlistMessage), }); EXPECT_EQ(expectedMessages, messages); @@ -577,12 +579,15 @@ TEST_P(DatabaseInterfaceTest, std::set expectedTags({ Tag("Actors.ACBS"), + Tag("Actors.ACBS", true, "file(\"" + missingEsp + "\")"), Tag("Actors.AIData"), Tag("C.Water", false), }); EXPECT_EQ(expectedTags, metadata.GetTags()); - EXPECT_EQ("file(\"" + missingEsp + - "\")" , metadata.GetTags().find(Tag("Actors.ACBS"))->GetCondition()); + EXPECT_EQ("file(\"" + missingEsp + "\")", + metadata.GetTags() + .find(Tag("Actors.ACBS", true, "file(\"" + missingEsp + "\")")) + ->GetCondition()); } TEST_P( @@ -739,7 +744,8 @@ TEST_P(DatabaseInterfaceTest, auto messages = db_->GetGeneralMessages(); std::vector expectedMessages({ - Message(MessageType::say, generalMasterlistMessage), + Message(MessageType::say, + generalMasterlistMessage, "file(\"" + missingEsp + "\")"), Message(MessageType::say, generalUserlistMessage), }); EXPECT_EQ(expectedMessages, messages); @@ -786,7 +792,9 @@ TEST_P( auto messages = db_->GetGeneralMessages(); std::vector expectedMessages({ - Message(MessageType::say, generalMasterlistMessage), + Message(MessageType::say, + generalMasterlistMessage, + "file(\"" + missingEsp + "\")"), }); EXPECT_EQ(expectedMessages, messages); diff --git a/src/tests/api/internals/metadata/file_test.h b/src/tests/api/internals/metadata/file_test.h index ccc102a5..65f55c16 100644 --- a/src/tests/api/internals/metadata/file_test.h +++ b/src/tests/api/internals/metadata/file_test.h @@ -49,24 +49,63 @@ TEST(File, stringsConstructorShouldStoreGivenStrings) { EXPECT_EQ("condition", file.GetCondition()); } -TEST(File, filesWithCaseInsensitiveEqualNameStringsShouldBeEqual) { - File file1("name", "display1", "condition1"); - File file2("Name", "display2", "condition2"); +TEST( + File, + equalityShouldBeCaseInsensitiveOnNameAndDisplay) { + File file1("name", "display", "condition"); + File file2("name", "display", "condition"); EXPECT_TRUE(file1 == file2); -} -TEST(File, filesWithDifferentNamesShouldBeUnequal) { - File file1("name1"); - File file2("name2"); + file1 = File("name", "display", "condition"); + file2 = File("Name", "display", "condition"); + + EXPECT_TRUE(file1 == file2); + + file1 = File("name1", "display", "condition"); + file2 = File("name2", "display", "condition"); EXPECT_FALSE(file1 == file2); } -TEST(File, - lessThanOperatorShouldUseCaseInsensitiveLexicographicalNameComparison) { - File file1("name", "display1", "condition1"); - File file2("Name", "display2", "condition2"); +TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) { + File file1("name", "display", "condition"); + File file2("name", "display", "condition"); + + EXPECT_TRUE(file1 == file2); + + file1 = File("name", "display", "condition"); + file2 = File("name", "Display", "condition"); + + EXPECT_FALSE(file1 == file2); + + file1 = File("name", "display", "condition"); + file2 = File("name", "display", "Condition"); + + EXPECT_FALSE(file1 == file2); + + file1 = File("name", "display1", "condition"); + file2 = File("name", "display2", "condition"); + + EXPECT_FALSE(file1 == file2); + + file1 = File("name", "display", "condition1"); + file2 = File("name", "display", "condition2"); + + EXPECT_FALSE(file1 == file2); +} + +TEST( + File, + lessThanOperatorShouldUseCaseInsensitiveLexicographicalComparisonForName) { + File file1("name", "display", "condition"); + File file2("name", "display", "condition"); + + EXPECT_FALSE(file1 < file2); + EXPECT_FALSE(file2 < file1); + + file1 = File("name", "display", "condition"); + file2 = File("Name", "display", "condition"); EXPECT_FALSE(file1 < file2); EXPECT_FALSE(file2 < file1); @@ -78,6 +117,40 @@ TEST(File, EXPECT_FALSE(file2 < file1); } +TEST( + File, + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForDisplayAndCondition) { + File file1("name", "display", "condition"); + File file2("name", "display", "condition"); + + EXPECT_FALSE(file1 < file2); + EXPECT_FALSE(file2 < file1); + + file1 = File("name", "display", "condition"); + file2 = File("name", "Display", "condition"); + + EXPECT_TRUE(file2 < file1); + EXPECT_FALSE(file1 < file2); + + file1 = File("name", "display", "condition"); + file2 = File("name", "display", "Condition"); + + EXPECT_TRUE(file2 < file1); + EXPECT_FALSE(file1 < file2); + + file1 = File("name", "display1"); + file2 = File("name", "display2"); + + EXPECT_TRUE(file1 < file2); + EXPECT_FALSE(file2 < file1); + + file1 = File("name", "display", "condition1"); + file2 = File("name", "display", "condition2"); + + EXPECT_TRUE(file1 < file2); + EXPECT_FALSE(file2 < file1); +} + TEST(File, emittingAsYamlShouldSingleQuoteValues) { File file("name1", "display1", "condition1"); YAML::Emitter emitter; diff --git a/src/tests/api/internals/metadata/location_test.h b/src/tests/api/internals/metadata/location_test.h index 806b1d7c..b8da8a4d 100644 --- a/src/tests/api/internals/metadata/location_test.h +++ b/src/tests/api/internals/metadata/location_test.h @@ -47,29 +47,37 @@ TEST(Location, stringsConstructorShouldStoreGivenStrings) { EXPECT_EQ("example", location.GetName()); } -TEST(Location, locationsWithCaseSensitiveEqualUrlsShouldBeEqual) { - Location location1("http://www.example.com", "example1"); - Location location2("http://www.example.com", "example2"); +TEST(Location, equalityShouldBeCaseSensitiveOnUrlAndName) { + Location location1("http://www.example.com", "example"); + Location location2("http://www.example.com", "example"); EXPECT_TRUE(location1 == location2); - location1 = Location("http://www.example.com"); - location2 = Location("HTTP://WWW.EXAMPLE.COM"); + location1 = Location("http://www.example.com", "example"); + location2 = Location("HTTP://WWW.EXAMPLE.COM", "example"); EXPECT_FALSE(location1 == location2); -} -TEST(Location, locationsWithDifferentUrlsShouldBeUnequal) { - Location location1("http://www.example1.com"); - Location location2("http://www.example2.com"); + location1 = Location("http://www.example.com", "example"); + location2 = Location("http://www.example.com", "Example"); + + EXPECT_FALSE(location1 == location2); + + location1 = Location("http://www.example1.com", "example"); + location2 = Location("http://www.example2.com", "example"); + + EXPECT_FALSE(location1 == location2); + + location1 = Location("http://www.example.com", "example1"); + location2 = Location("http://www.example.com", "example2"); EXPECT_FALSE(location1 == location2); } TEST(Location, - lessThanOperatorShouldUseCaseSensitiveLexicographicalUrlComparison) { - Location location1("http://www.example.com", "example1"); - Location location2("http://www.example.com", "example2"); + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForNameAndUrl) { + Location location1("http://www.example.com", "example"); + Location location2("http://www.example.com", "example"); EXPECT_FALSE(location1 < location2); EXPECT_FALSE(location2 < location1); @@ -80,11 +88,23 @@ TEST(Location, EXPECT_FALSE(location1 < location2); EXPECT_TRUE(location2 < location1); + location1 = Location("http://www.example.com", "example"); + location2 = Location("http://www.example.com", "Example"); + + EXPECT_FALSE(location1 < location2); + EXPECT_TRUE(location2 < location1); + location1 = Location("http://www.example1.com"); location2 = Location("http://www.example2.com"); EXPECT_TRUE(location1 < location2); EXPECT_FALSE(location2 < location1); + + location1 = Location("http://www.example.com", "example1"); + location2 = Location("http://www.example.com", "example2"); + + EXPECT_FALSE(location2 < location1); + EXPECT_TRUE(location1 < location2); } TEST(Location, emittingAsYamlShouldOutputAScalarIfTheNameStringIsEmpty) { diff --git a/src/tests/api/internals/metadata/message_content_test.h b/src/tests/api/internals/metadata/message_content_test.h index dfce00f5..bc4e5bf0 100644 --- a/src/tests/api/internals/metadata/message_content_test.h +++ b/src/tests/api/internals/metadata/message_content_test.h @@ -49,45 +49,64 @@ TEST(MessageContent, contentConstructorShouldStoreGivenStringAndLanguage) { EXPECT_EQ(french, content.GetLanguage()); } -TEST(MessageContent, contentShouldBeEqualIfStringsAreCaseInsensitivelyEqual) { - MessageContent content1("content"); - MessageContent content2("content", french); +TEST(MessageContent, equalityShouldRequireCaseSensitiveEqualityOnTextAndLanguage) { + MessageContent content1("content", "fr"); + MessageContent content2("content", "fr"); EXPECT_TRUE(content1 == content2); - content1 = MessageContent("content"); - content2 = MessageContent("Content", french); + content1 = MessageContent("content", "fr"); + content2 = MessageContent("Content", "fr"); + + EXPECT_FALSE(content1 == content2); + + content1 = MessageContent("content", "fr"); + content2 = MessageContent("content", "Fr"); + + EXPECT_FALSE(content1 == content2); + + content1 = MessageContent("content1", "fr"); + content2 = MessageContent("content2", "fr"); + + EXPECT_FALSE(content1 == content2); + + content1 = MessageContent("content", "fr"); + content2 = MessageContent("content", "de"); EXPECT_FALSE(content1 == content2); } TEST(MessageContent, - contentShouldBeUnequalIfStringsAreNotCaseInsensitivelyEqual) { - MessageContent content1("content1", french); - MessageContent content2("content2", french); - - EXPECT_FALSE(content1 == content2); -} - -TEST(MessageContent, - lessThanOperatorShouldUseCaseSensitiveLexicographicalComparison) { - MessageContent content1("content"); - MessageContent content2("content", french); + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForTextAndLanguage) { + MessageContent content1("content", "fr"); + MessageContent content2("content", "fr"); EXPECT_FALSE(content1 < content2); EXPECT_FALSE(content2 < content1); - content1 = MessageContent("content", french); - content2 = MessageContent("Content"); + content1 = MessageContent("content", "fr"); + content2 = MessageContent("Content", "fr"); EXPECT_FALSE(content1 < content2); EXPECT_TRUE(content2 < content1); - content1 = MessageContent("content1", french); - content2 = MessageContent("content2"); + content1 = MessageContent("content", "fr"); + content2 = MessageContent("content", "Fr"); + + EXPECT_TRUE(content2 < content1); + EXPECT_FALSE(content1 < content2); + + content1 = MessageContent("content1", "fr"); + content2 = MessageContent("content2", "fr"); EXPECT_TRUE(content1 < content2); EXPECT_FALSE(content2 < content1); + + content1 = MessageContent("content", "fr"); + content2 = MessageContent("content", "de"); + + EXPECT_TRUE(content2 < content1); + EXPECT_FALSE(content1 < content2); } TEST(MessageContent, emittingAsYamlShouldOutputDataCorrectly) { diff --git a/src/tests/api/internals/metadata/message_test.h b/src/tests/api/internals/metadata/message_test.h index 9ced0ac6..1a0783fb 100644 --- a/src/tests/api/internals/metadata/message_test.h +++ b/src/tests/api/internals/metadata/message_test.h @@ -82,38 +82,93 @@ TEST_P( std::invalid_argument); } -TEST_P(MessageTest, messagesWithDifferentContentStringsShouldBeUnequal) { - Message message1(MessageType::say, "content1", "condition1"); - Message message2(MessageType::say, "content2", "condition1"); +TEST_P(MessageTest, equalityShouldRequireEqualMessageTypes) { + Message message1(MessageType::say, "content"); + Message message2(MessageType::say, "content"); + + EXPECT_TRUE(message1 == message2); + + message1 = Message(MessageType::say, "content"); + message2 = Message(MessageType::warn, "content"); EXPECT_FALSE(message1 == message2); } -TEST_P(MessageTest, messagesWithEqualContentStringsShouldBeEqual) { - Message message1(MessageType::say, - MessageContents({MessageContent("content1")}), - "condition1"); - Message message2(MessageType::warn, - MessageContents({MessageContent("content1", french)}), - "condition2"); +TEST_P(MessageTest, equalityShouldRequireCaseSensitiveEqualityOnCondition) { + Message message1(MessageType::say, "content", "condition"); + Message message2(MessageType::say, "content", "condition"); EXPECT_TRUE(message1 == message2); + + message1 = Message(MessageType::say, "content", "condition"); + message2 = Message(MessageType::say, "content", "Condition"); + + EXPECT_FALSE(message1 == message2); + + message1 = Message(MessageType::say, "content", "condition1"); + message2 = Message(MessageType::say, "content", "condition2"); + + EXPECT_FALSE(message1 == message2); +} + +TEST_P(MessageTest, equalityShouldRequireEqualContent) { + Message message1(MessageType::say, "content"); + Message message2(MessageType::say, "content"); + + EXPECT_TRUE(message1 == message2); + + message1 = Message(MessageType::say, "content1"); + message2 = Message(MessageType::say, "content2"); + + EXPECT_FALSE(message1 == message2); +} + +TEST_P(MessageTest, lessThanOperatorShouldCompareMessageTypes) { + Message message1(MessageType::say, "content"); + Message message2(MessageType::say, "content"); + + EXPECT_FALSE(message1 < message2); + EXPECT_FALSE(message2 < message1); + + message1 = Message(MessageType::say, "content"); + message2 = Message(MessageType::warn, "content"); + + EXPECT_TRUE(message1 < message2); + EXPECT_FALSE(message2 < message1); +} + +TEST_P(MessageTest, lessThanOperatorShouldCompareContent) { + Message message1(MessageType::say, "content"); + Message message2(MessageType::say, "content"); + + EXPECT_FALSE(message1 < message2); + EXPECT_FALSE(message2 < message1); + + message1 = Message(MessageType::say, "content1"); + message2 = Message(MessageType::say, "content2"); + + EXPECT_TRUE(message1 < message2); + EXPECT_FALSE(message2 < message1); } TEST_P( MessageTest, - lessThanOperatorShouldUseCaseInsensitiveLexicographicalContentStringComparison) { - Message message1(MessageType::say, - MessageContents({MessageContent("content1")}), - "condition1"); - Message message2(MessageType::warn, - MessageContents({MessageContent("content1", french)}), - "condition2"); + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForConditions) { + Message message1(MessageType::say, "content", "condition"); + Message message2(MessageType::say, "content", "condition"); + EXPECT_FALSE(message1 < message2); EXPECT_FALSE(message2 < message1); - message1 = Message(MessageType::say, "content1", "condition1"); - message2 = Message(MessageType::say, "content2", "condition1"); + message1 = Message(MessageType::say, "content", "condition"); + message2 = Message(MessageType::say, "content", "Condition"); + + EXPECT_TRUE(message2 < message1); + EXPECT_FALSE(message1 < message2); + + message1 = Message(MessageType::say, "content", "condition1"); + message2 = Message(MessageType::say, "content", "condition2"); + EXPECT_TRUE(message1 < message2); EXPECT_FALSE(message2 < message1); } diff --git a/src/tests/api/internals/metadata/plugin_cleaning_data_test.h b/src/tests/api/internals/metadata/plugin_cleaning_data_test.h index 167e2f2c..9596d0f8 100644 --- a/src/tests/api/internals/metadata/plugin_cleaning_data_test.h +++ b/src/tests/api/internals/metadata/plugin_cleaning_data_test.h @@ -70,19 +70,44 @@ TEST_P(PluginCleaningDataTest, contentConstructorShouldStoreAllGivenData) { EXPECT_EQ(info_, info.GetInfo()); } -TEST_P(PluginCleaningDataTest, dirtyInfoShouldBeEqualIfCrcValuesAreEqual) { - PluginCleaningData info1(0x12345678, "cleaner1", info_, 2, 10, 30); - PluginCleaningData info2(0x12345678, "cleaner2", info_, 4, 20, 60); +TEST_P(PluginCleaningDataTest, equalityShouldCheckEqualityOfAllFields) { + PluginCleaningData info1(0x12345678, "cleaner", info_, 2, 10, 30); + PluginCleaningData info2(0x12345678, "cleaner", info_, 2, 10, 30); EXPECT_TRUE(info1 == info2); info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); info2 = PluginCleaningData(0x87654321, "cleaner", info_, 2, 10, 30); EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "Cleaner", info_, 2, 10, 30); + EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner1", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner2", info_, 2, 10, 30); + EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData( + 0x12345678, "cleaner", std::vector(), 2, 10, 30); + EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 4, 10, 30); + EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 20, 30); + EXPECT_FALSE(info1 == info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 60); + EXPECT_FALSE(info1 == info2); } -TEST_P(PluginCleaningDataTest, LessThanOperatorShouldCompareCrcValues) { - PluginCleaningData info1(0x12345678, "cleaner1", info_, 2, 10, 30); - PluginCleaningData info2(0x12345678, "cleaner2", info_, 4, 20, 60); +TEST_P(PluginCleaningDataTest, lessThanOperatorShouldCompareAllFields) { + PluginCleaningData info1(0x12345678, "cleaner", info_, 2, 10, 30); + PluginCleaningData info2(0x12345678, "cleaner", info_, 2, 10, 30); EXPECT_FALSE(info1 < info2); EXPECT_FALSE(info2 < info1); @@ -90,6 +115,37 @@ TEST_P(PluginCleaningDataTest, LessThanOperatorShouldCompareCrcValues) { info2 = PluginCleaningData(0x87654321, "cleaner", info_, 2, 10, 30); EXPECT_TRUE(info1 < info2); EXPECT_FALSE(info2 < info1); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "Cleaner", info_, 2, 10, 30); + EXPECT_TRUE(info2 < info1); + EXPECT_FALSE(info1 < info2); + + info1 = PluginCleaningData(0x12345678, "cleaner1", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner2", info_, 2, 10, 30); + EXPECT_TRUE(info1 < info2); + EXPECT_FALSE(info2 < info1); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData( + 0x12345678, "cleaner", std::vector(), 2, 10, 30); + EXPECT_TRUE(info2 < info1); + EXPECT_FALSE(info1 < info2); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 4, 10, 30); + EXPECT_TRUE(info1 < info2); + EXPECT_FALSE(info2 < info1); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 20, 30); + EXPECT_TRUE(info1 < info2); + EXPECT_FALSE(info2 < info1); + + info1 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 30); + info2 = PluginCleaningData(0x12345678, "cleaner", info_, 2, 10, 60); + EXPECT_TRUE(info1 < info2); + EXPECT_FALSE(info2 < info1); } TEST_P(PluginCleaningDataTest, diff --git a/src/tests/api/internals/metadata/plugin_metadata_test.h b/src/tests/api/internals/metadata/plugin_metadata_test.h index 83e5daaa..714390e3 100644 --- a/src/tests/api/internals/metadata/plugin_metadata_test.h +++ b/src/tests/api/internals/metadata/plugin_metadata_test.h @@ -978,7 +978,7 @@ TEST_P(PluginMetadataTest, decodingFromYamlShouldStoreAllGivenData) { plugin.GetMessages()); EXPECT_EQ(std::set({Tag("Relev")}), plugin.GetTags()); EXPECT_EQ(std::set( - {PluginCleaningData(5, "utility", info_, 0, 1, 2)}), + {PluginCleaningData(5, "utility", {}, 0, 1, 2)}), plugin.GetDirtyInfo()); EXPECT_EQ(std::set({PluginCleaningData(6, "utility")}), plugin.GetCleanInfo()); @@ -999,7 +999,7 @@ TEST_P(PluginMetadataTest, EXPECT_EQ("Blank\\.esp", plugin.GetName()); EXPECT_EQ(std::set( - {PluginCleaningData(5, "utility", info_, 0, 1, 2)}), + {PluginCleaningData(5, "utility", {}, 0, 1, 2)}), plugin.GetDirtyInfo()); } diff --git a/src/tests/api/internals/metadata/tag_test.h b/src/tests/api/internals/metadata/tag_test.h index 97bd7570..6ecddf62 100644 --- a/src/tests/api/internals/metadata/tag_test.h +++ b/src/tests/api/internals/metadata/tag_test.h @@ -50,10 +50,9 @@ TEST(Tag, dataConstructorShouldSetFieldsToGivenValues) { EXPECT_EQ("condition", tag.GetCondition()); } -TEST(Tag, - tagsWithCaseInsensitiveEqualNamesAndEqualAdditionStatesShouldBeEqual) { - Tag tag1("name", true, "condition1"); - Tag tag2("name", true, "condition2"); +TEST(Tag, equalityShouldBeCaseSensitiveOnNameAndCondition) { + Tag tag1("name", true, "condition"); + Tag tag2("name", true, "condition"); EXPECT_TRUE(tag1 == tag2); @@ -61,27 +60,39 @@ TEST(Tag, tag2 = Tag("Name"); EXPECT_FALSE(tag1 == tag2); -} -TEST(Tags, tagsWithUnequalNamesShouldNotBeEqual) { - Tag tag1("name1"); - Tag tag2("name2"); + tag1 = Tag("name", true, "condition"); + tag2 = Tag("name", true, "Condition"); + + EXPECT_FALSE(tag1 == tag2); + + tag1 = Tag("name1"); + tag2 = Tag("name2"); + + EXPECT_FALSE(tag1 == tag2); + + tag1 = Tag("name", true, "condition1"); + tag2 = Tag("name", true, "condition2"); EXPECT_FALSE(tag1 == tag2); } -TEST(Tag, tagsWithUnequalAdditionStatesShouldNotBeEqual) { - Tag tag1("Name", true); - Tag tag2("name", false); +TEST(Tag, equalityShouldRequireEqualAdditionStates) { + Tag tag1("name", true, "condition"); + Tag tag2("name", true, "condition"); + + EXPECT_TRUE(tag1 == tag2); + + tag1 = Tag("name", true); + tag2 = Tag("name", false); EXPECT_FALSE(tag1 == tag2); } -TEST( - Tag, - lessThanOperatorShouldCaseSensitivelyLexicographicallyCompareNameStrings) { - Tag tag1("name"); - Tag tag2("name"); +TEST(Tag, + lessThanOperatorShouldUseCaseSensitiveLexicographicalComparisonForNameAndCondition) { + Tag tag1("name", true, "condition"); + Tag tag2("name", true, "condition"); EXPECT_FALSE(tag1 < tag2); EXPECT_FALSE(tag2 < tag1); @@ -92,11 +103,23 @@ TEST( EXPECT_FALSE(tag1 < tag2); EXPECT_TRUE(tag2 < tag1); + tag1 = Tag("name", true, "condition"); + tag2 = Tag("name", true, "Condition"); + + EXPECT_FALSE(tag1 < tag2); + EXPECT_TRUE(tag2 < tag1); + tag1 = Tag("name1"); tag2 = Tag("name2"); EXPECT_TRUE(tag1 < tag2); EXPECT_FALSE(tag2 < tag1); + + tag1 = Tag("name", true, "condition1"); + tag2 = Tag("name", true, "condition2"); + + EXPECT_TRUE(tag1 < tag2); + EXPECT_FALSE(tag2 < tag1); } TEST(Tag, lessThanOperatorShouldTreatTagAdditionsAsBeingLessThanRemovals) { diff --git a/src/tests/api/internals/metadata_list_test.h b/src/tests/api/internals/metadata_list_test.h index d91d05d4..d04c5f05 100644 --- a/src/tests/api/internals/metadata_list_test.h +++ b/src/tests/api/internals/metadata_list_test.h @@ -59,9 +59,11 @@ protected: ASSERT_FALSE(exists(missingMetadataPath)); } - void copyInvalidMetadataFile(const std::filesystem::path& sourceDirectory, const std::string& file) { + void copyInvalidMetadataFile(const std::filesystem::path& sourceDirectory, + const std::string& file) { std::filesystem::create_directories(metadataFilesPath / "invalid"); - std::filesystem::copy(sourceDirectory / "invalid" / file, metadataFilesPath / "invalid" / file); + std::filesystem::copy(sourceDirectory / "invalid" / file, + metadataFilesPath / "invalid" / file); ASSERT_TRUE(std::filesystem::exists(metadataFilesPath / "invalid" / file)); } @@ -147,13 +149,13 @@ TEST_P(MetadataListTest, loadYamlParsingShouldSupportMergeKeys) { std::ofstream out(metadataPath); out << "common:" << endl - << " - &earlier" << endl - << " name: earlier" << endl - << " after:" << endl - << " - earliest" << endl - << "groups:" << endl - << " - name: default" << endl - << " <<: *earlier" << endl; + << " - &earlier" << endl + << " name: earlier" << endl + << " after:" << endl + << " - earliest" << endl + << "groups:" << endl + << " - name: default" << endl + << " <<: *earlier" << endl; out.close(); @@ -165,8 +167,8 @@ TEST_P(MetadataListTest, loadYamlParsingShouldSupportMergeKeys) { EXPECT_EQ(1, groups.size()); EXPECT_EQ(1, groups.count(Group("default"))); - EXPECT_EQ(std::unordered_set({ "earliest" }), - groups.find(Group("default"))->GetAfterGroups()); + EXPECT_EQ(std::unordered_set({"earliest"}), + groups.find(Group("default"))->GetAfterGroups()); } TEST_P(MetadataListTest, loadShouldThrowIfAnInvalidMetadataFileIsGiven) { @@ -291,8 +293,7 @@ TEST_P( MetadataList metadataList; ASSERT_NO_THROW(metadataList.Load(metadataPath)); - PluginMetadata plugin = - metadataList.FindPlugin(blankDifferentEsp).value(); + PluginMetadata plugin = metadataList.FindPlugin(blankDifferentEsp).value(); EXPECT_EQ(blankDifferentEsp, plugin.GetName()); EXPECT_EQ(std::set({ @@ -328,8 +329,7 @@ TEST_P(MetadataListTest, addPluginShouldStoreGivenRegexPluginMetadata) { plugin.SetGroup("group1"); metadataList.AddPlugin(plugin); - plugin = - metadataList.FindPlugin(blankPluginDependentEsp).value(); + plugin = metadataList.FindPlugin(blankPluginDependentEsp).value(); EXPECT_EQ("group1", plugin.GetGroup()); } @@ -363,8 +363,7 @@ TEST_P( MetadataListTest, evalAllConditionsShouldEvaluateTheConditionsForThePluginsStoredInTheMetadataList) { Game game(GetParam(), dataPath.parent_path(), localPath); - ConditionEvaluator evaluator(game.Type(), - game.DataPath()); + ConditionEvaluator evaluator(game.Type(), game.DataPath()); MetadataList metadataList; ASSERT_NO_THROW(metadataList.Load(metadataPath)); @@ -374,7 +373,8 @@ TEST_P( std::vector({ Message(MessageType::warn, "This is a warning."), Message(MessageType::say, - "This message should be removed when evaluating conditions."), + "This message should be removed when evaluating conditions.", + "active(\"Blank - Different.esm\")"), }), plugin.GetMessages());