From d278b4c3338b672b76a16dc4b0ad596f5099e194 Mon Sep 17 00:00:00 2001 From: codereader Date: Sun, 7 Aug 2022 06:15:05 +0200 Subject: [PATCH] #6031: More decl removal unit test cases --- test/DeclManager.cpp | 144 ++++++++++++++++-- test/MaterialExport.cpp | 42 ++--- test/algorithm/FileUtils.h | 14 ++ .../tdm/testdecls/removal_tests.decl | 8 +- 4 files changed, 167 insertions(+), 41 deletions(-) diff --git a/test/DeclManager.cpp b/test/DeclManager.cpp index d4a5b18b04..827e19a9b5 100644 --- a/test/DeclManager.cpp +++ b/test/DeclManager.cpp @@ -749,9 +749,8 @@ TEST_F(DeclManagerTest, RemoveDeclarationFromPhysicalFile) auto fileContents = algorithm::loadTextFromVfsFile(decl->getDeclFilePath()); EXPECT_NE(fileContents.find(originalSyntax.contents), std::string::npos) << "Decl source not found"; - // Create a backup copy of the material file we're going to manipulate - fs::path declFile = _context.getTestProjectPath() + decl->getDeclFilePath(); - BackupCopy backup(declFile); + // Create a backup copy of the decl file we're going to manipulate + BackupCopy backup(_context.getTestProjectPath() + decl->getDeclFilePath()); // Remove the decl, it should remove the source from the file GlobalDeclarationManager().removeDeclaration(decl->getDeclType(), decl->getDeclName()); @@ -761,21 +760,146 @@ TEST_F(DeclManagerTest, RemoveDeclarationFromPhysicalFile) EXPECT_EQ(contentsAfterRemoval.find(originalSyntax.contents), std::string::npos) << "Decl source should be gone"; } -// Removing a decl will also remove leading comment lines -TEST_F(DeclManagerTest, RemoveDeclarationIncludesLeadingComment) +// Removing a decl will keep leading comment lines that are separated with an empty line +TEST_F(DeclManagerTest, RemoveDeclarationPreservesSeparatedLeadingComments) { - // TODO + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + // Create a backup copy of the decl file we're going to manipulate + auto fullPath = _context.getTestProjectPath() + "testdecls/removal_tests.decl"; + BackupCopy backup(fullPath); + + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Comments at the beginning of the file with an empty line in between")); + + // Remove decl/removal/0. This should remove the decl, but the comment at the beginning of the file should remain intact + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/0"); + + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Comments at the beginning of the file with an empty line in between")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(decl/removal/0 +{ + diffusemap textures/removal/0 +})")); + + // Similar situation with decls/removal/5 + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Comment with a line before the start of the decl decl/removal/5")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(testdecl decl/removal/5 { // a comment in the same line as the opening brace + diffusemap textures/removal/5 +})")); + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/5"); + + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(testdecl decl/removal/5 { // a comment in the same line as the opening brace + diffusemap textures/removal/5 +})")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Comment with a line before the start of the decl decl/removal/5")); } -// Removing decls with weird line break configurations -TEST_F(DeclManagerTest, RemoveDeclarationWithNonstandardLinebreaks) +// Removing a decl will keep trailing comment lines and remove leading C-style comments +TEST_F(DeclManagerTest, RemoveDeclarationRemovesLeadingCStyleComment) { - // TODO + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + // Create a backup copy of the decl file we're going to manipulate + auto fullPath = _context.getTestProjectPath() + "testdecls/removal_tests.decl"; + BackupCopy backup(fullPath); + + // decl/removal/1 has leading C-style comment, and a trailing single line comment + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "/* Without \"testdecl\" typename, C-style comment */")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Some comments after decl/removal/1")); + + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/1"); + + // Leading comment should be gone, trailing comment should stay + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(decl/removal/1 +{ + diffusemap textures/removal/1 +})")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, "/* Without \"testdecl\" typename, C-style comment */")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Some comments after decl/removal/1")); +} + +// Removing a decl will keep trailing comment lines and remove leading single-line comments +TEST_F(DeclManagerTest, RemoveDeclarationRemovesLeadingSingleLineComment) +{ + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + // Create a backup copy of the decl file we're going to manipulate + auto fullPath = _context.getTestProjectPath() + "testdecls/removal_tests.decl"; + BackupCopy backup(fullPath); + + // decl/removal/3 has leading single-line comment + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(// Comment before decl/removal/3)")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(TestDecl decl/removal/3 +{ + diffusemap textures/removal/3 +})")); + + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/3"); + + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(// Comment before decl/removal/3)")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(TestDecl decl/removal/3 +{ + diffusemap textures/removal/3 +})")); + // This one is a trailing comment, and separated with an empty line too + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Comment with a line before the start of the decl decl/removal/5")); + + // decl/removal/6 has leading single-line comment, decl 7 is right afterwards + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(// Everything in the same line (before decl/removal/6))")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "testdecl decl/removal/6 { diffusemap textures/removal/6 }")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "testdecl decl/removal/7 { diffusemap textures/removal/7 }")); + + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/6"); + + // Leading comment should be gone, trailing decl should remain intact + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(// Everything in the same line (before decl/removal/6))")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, "testdecl decl/removal/6 { diffusemap textures/removal/6 }")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "testdecl decl/removal/7 { diffusemap textures/removal/7 }")); +} + +// Removing a decl will keep trailing comment lines and remove leading multi-line comments +TEST_F(DeclManagerTest, RemoveDeclarationRemovesLeadingMultilineComment) +{ + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + // Create a backup copy of the decl file we're going to manipulate + auto fullPath = _context.getTestProjectPath() + "testdecls/removal_tests.decl"; + BackupCopy backup(fullPath); + + // decl/removal/2 has leading multiline comment, and a trailing single line comment + EXPECT_TRUE(algorithm::fileContainsText(fullPath, R"(/** + A multiline comment before this decl + */)")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Some comments after decl/removal/2")); + + GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/removal/2"); + + // Leading comment should be gone, trailing comment should stay + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(testdecl decl/removal/2 +{ + diffusemap textures/removal/2 +})")); + EXPECT_FALSE(algorithm::fileContainsText(fullPath, R"(/** + A multiline comment before this decl + */)")); + EXPECT_TRUE(algorithm::fileContainsText(fullPath, "// Some comments after decl/removal/2")); } TEST_F(DeclManagerTest, RemoveUnsavedDeclaration) { - // TODO + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + auto decl = GlobalDeclarationManager().findOrCreateDeclaration(decl::Type::TestDecl, "decl/newlycreated/1"); + expectDeclIsPresent(decl::Type::TestDecl, "decl/newlycreated/1"); + + // Removing an in-memory decl should succeed + EXPECT_NO_THROW(GlobalDeclarationManager().removeDeclaration(decl::Type::TestDecl, "decl/newlycreated/1")); + + expectDeclIsNotPresent(decl::Type::TestDecl, "decl/newlycreated/1"); } TEST_F(DeclManagerTest, RenameDeclaration) diff --git a/test/MaterialExport.cpp b/test/MaterialExport.cpp index 14f82e8467..72e6c52b13 100644 --- a/test/MaterialExport.cpp +++ b/test/MaterialExport.cpp @@ -1223,18 +1223,6 @@ TEST_F(MaterialExportTest, BlendShortcuts) material->revertModifications(); } -bool fileContainsText(const fs::path& path, const std::string& textToFind) -{ - std::stringstream contentStream; - std::ifstream input(path); - - contentStream << input.rdbuf(); - - std::string contents = string::replace_all_copy(contentStream.str(), "\r\n", "\n"); - - return contents.find(textToFind) != std::string::npos; -} - TEST_F(MaterialExportTest, MaterialDefDetectionRegex) { std::smatch matches; @@ -1272,16 +1260,16 @@ TEST_F(MaterialExportTest, WritingMaterialFiles) auto originalDefinition = "textures/exporttest/renderBump1 { // Opening brace in the same line as the name (DON'T REMOVE THIS)\n" " renderBump textures/output.tga models/hipoly \n" "}"; - EXPECT_TRUE(fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; auto material = GlobalMaterialManager().getMaterial("textures/exporttest/renderBump1"); material->setDescription(description); GlobalMaterialManager().saveMaterial(material->getName()); - EXPECT_TRUE(fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) << "New definition not found in file"; - EXPECT_FALSE(fileContainsText(exportTestFile, originalDefinition)) + EXPECT_FALSE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition still in file"; // RenderBump2 @@ -1289,16 +1277,16 @@ TEST_F(MaterialExportTest, WritingMaterialFiles) "{\n" " renderBump -size 100 200 textures/output.tga models/hipoly \n" "}"; - EXPECT_TRUE(fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; material = GlobalMaterialManager().getMaterial("textures/exporttest/renderBump2"); material->setDescription(description); GlobalMaterialManager().saveMaterial(material->getName()); - EXPECT_TRUE(fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) << "New definition not found in file"; - EXPECT_FALSE(fileContainsText(exportTestFile, originalDefinition)) + EXPECT_FALSE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition still in file"; // RenderBump3 @@ -1307,32 +1295,32 @@ TEST_F(MaterialExportTest, WritingMaterialFiles) "{\n" " renderBump -aa 2 textures/output.tga models/hipoly \n" "}"; - EXPECT_TRUE(fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; material = GlobalMaterialManager().getMaterial("textures/exporttest/renderBump3"); material->setDescription(description); GlobalMaterialManager().saveMaterial(material->getName()); - EXPECT_TRUE(fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) << "New definition not found in file"; - EXPECT_FALSE(fileContainsText(exportTestFile, originalDefinition)) + EXPECT_FALSE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition still in file"; // RenderBump4 originalDefinition = "textures/exporttest/renderBump4 {\n" " renderBump -aa 2 -size 10 10 textures/output.tga models/hipoly \n" "}"; - EXPECT_TRUE(fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition not found in file " << exportTestFile; material = GlobalMaterialManager().getMaterial("textures/exporttest/renderBump4"); material->setDescription(description); GlobalMaterialManager().saveMaterial(material->getName()); - EXPECT_TRUE(fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) << "New definition not found in file"; - EXPECT_FALSE(fileContainsText(exportTestFile, originalDefinition)) + EXPECT_FALSE(algorithm::fileContainsText(exportTestFile, originalDefinition)) << "Original definition still in file"; // Create a new material, which is definitely not present in the file @@ -1345,13 +1333,13 @@ TEST_F(MaterialExportTest, WritingMaterialFiles) newMaterial->setShaderFileName(exportTestFile.string()); EXPECT_TRUE(newMaterial->isModified()); - EXPECT_FALSE(fileContainsText(exportTestFile, newMaterial->getName())); + EXPECT_FALSE(algorithm::fileContainsText(exportTestFile, newMaterial->getName())); GlobalMaterialManager().saveMaterial(newMaterial->getName()); // After saving the material should no longer be "modified" EXPECT_FALSE(newMaterial->isModified()); - EXPECT_TRUE(fileContainsText(exportTestFile, newMaterial->getName() + "\n{" + newMaterial->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, newMaterial->getName() + "\n{" + newMaterial->getDefinition() + "}")) << "New definition not found in file"; } @@ -1369,7 +1357,7 @@ TEST_F(MaterialExportTest, SavedMaterialCanBeModified) GlobalMaterialManager().saveMaterial(material->getName()); - EXPECT_TRUE(fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) + EXPECT_TRUE(algorithm::fileContainsText(exportTestFile, material->getName() + "\n{" + material->getDefinition() + "}")) << "New definition not found in file"; EXPECT_TRUE(GlobalMaterialManager().materialCanBeModified(material->getName())) diff --git a/test/algorithm/FileUtils.h b/test/algorithm/FileUtils.h index 4d3eafd585..246acccaef 100644 --- a/test/algorithm/FileUtils.h +++ b/test/algorithm/FileUtils.h @@ -3,6 +3,7 @@ #include #include "ifilesystem.h" #include "os/fs.h" +#include "string/replace.h" namespace test { @@ -64,6 +65,19 @@ inline std::string loadTextFromVfsFile(const std::string& vfsPath) return textStream.str(); } +// True if the file (full physical path) contains the given text (ignoring CRLF vs. LF line break differences) +inline bool fileContainsText(const fs::path& path, const std::string& textToFind) +{ + std::stringstream contentStream; + std::ifstream input(path); + + contentStream << input.rdbuf(); + + std::string contents = string::replace_all_copy(contentStream.str(), "\r\n", "\n"); + + return contents.find(textToFind) != std::string::npos; +} + } } diff --git a/test/resources/tdm/testdecls/removal_tests.decl b/test/resources/tdm/testdecls/removal_tests.decl index 62ee9671a5..9717fcef2a 100644 --- a/test/resources/tdm/testdecls/removal_tests.decl +++ b/test/resources/tdm/testdecls/removal_tests.decl @@ -23,7 +23,7 @@ testdecl decl/removal/2 } // Some comments after decl/removal/2 -// CamelCase typename shouldn't make a difference +// Comment before decl/removal/3 TestDecl decl/removal/3 { diffusemap textures/removal/3 @@ -35,8 +35,8 @@ testdecl decl/removal/5 { // a comment in the same line as the opening brace diffusemap textures/removal/5 } -// Everything in the same line +// Everything in the same line (before decl/removal/6) testdecl decl/removal/6 { diffusemap textures/removal/6 } -testdecl decl/removal/7 { diffusemap textures/removal/6 } +testdecl decl/removal/7 { diffusemap textures/removal/7 } -testdecl decl/removal/8 { diffusemap textures/removal/6 } +testdecl decl/removal/8 { diffusemap textures/removal/8 }