From 6b45cdaf9f737e5f40dbb944ccef61c12d93d038 Mon Sep 17 00:00:00 2001 From: codereader Date: Sun, 7 Aug 2022 05:35:30 +0200 Subject: [PATCH] #6031: Start writing unit tests to cover the desired behaviour of IDeclarationManager::removeDeclaration --- include/ideclmanager.h | 7 ++- test/DeclManager.cpp | 59 +++++++++++++++++++ .../tdm/testdecls/removal_tests.decl | 42 +++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 test/resources/tdm/testdecls/removal_tests.decl diff --git a/include/ideclmanager.h b/include/ideclmanager.h index 3870bdabb3..cf8abd4e2f 100644 --- a/include/ideclmanager.h +++ b/include/ideclmanager.h @@ -137,9 +137,10 @@ class IDeclarationManager : // Returns true if the old declaration existed and could successfully be renamed, false on any failure. virtual bool renameDeclaration(Type type, const std::string& oldName, const std::string& newName) = 0; - // Removes the given declaration from the internal dictionaries. - // This doesn't remove the declaration from the source files, so this operation - // will not survive a reloadDeclarations command. Used to remove temporary decls in edit scenarios. + // Removes the given declaration both from memory and the decl file, if there is one. + // Only declarations stored in physical files (or unsaved ones) can be removed. + // Attempting to remove a read-only declaration (e.g. decls saved in PK4 archives) + // will cause a std::logic_error to be thrown. virtual void removeDeclaration(Type type, const std::string& name) = 0; // Re-load all declarations. diff --git a/test/DeclManager.cpp b/test/DeclManager.cpp index 76385aa08c..d4a5b18b04 100644 --- a/test/DeclManager.cpp +++ b/test/DeclManager.cpp @@ -719,6 +719,65 @@ TEST_F(DeclManagerTest, RemoveDeclaration) expectDeclIsNotPresent(decl::Type::TestDecl, "decl/precedence_test/1"); } +// Removing a decl defined in a PK4 file will throw +TEST_F(DeclManagerTest, RemoveDeclarationInPk4File) +{ + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + auto decl = GlobalDeclarationManager().findDeclaration(decl::Type::TestDecl, "decl/export/0"); + EXPECT_TRUE(decl) << "decl/export/0 must be present"; + EXPECT_FALSE(decl->getBlockSyntax().fileInfo.getIsPhysicalFile()) << "decl/export/0 should be in a PK4 file"; + + // Attempting to remove the decl will throw + EXPECT_THROW(GlobalDeclarationManager().removeDeclaration(decl->getDeclType(), decl->getDeclName()), + std::logic_error) << "Removing a PK4 decl should throw"; +} + +// Removing a decl defined in a physical file will succeed +TEST_F(DeclManagerTest, RemoveDeclarationFromPhysicalFile) +{ + GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); + GlobalDeclarationManager().registerDeclFolder(decl::Type::TestDecl, TEST_DECL_FOLDER, ".decl"); + + auto decl = GlobalDeclarationManager().findDeclaration(decl::Type::TestDecl, "decl/removal/1"); + expectDeclIsPresent(decl::Type::TestDecl, "decl/removal/1"); + + auto originalSyntax = decl->getBlockSyntax(); + EXPECT_TRUE(originalSyntax.fileInfo.getIsPhysicalFile()) << "decl/removal/1 must be in a physical file"; + + 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); + + // Remove the decl, it should remove the source from the file + GlobalDeclarationManager().removeDeclaration(decl->getDeclType(), decl->getDeclName()); + + auto contentsAfterRemoval = algorithm::loadTextFromVfsFile(decl->getDeclFilePath()); + EXPECT_NE(contentsAfterRemoval, fileContents) << "File contents should have been changed"; + 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) +{ + // TODO +} + +// Removing decls with weird line break configurations +TEST_F(DeclManagerTest, RemoveDeclarationWithNonstandardLinebreaks) +{ + // TODO +} + +TEST_F(DeclManagerTest, RemoveUnsavedDeclaration) +{ + // TODO +} + TEST_F(DeclManagerTest, RenameDeclaration) { GlobalDeclarationManager().registerDeclType("testdecl", std::make_shared()); diff --git a/test/resources/tdm/testdecls/removal_tests.decl b/test/resources/tdm/testdecls/removal_tests.decl new file mode 100644 index 0000000000..62ee9671a5 --- /dev/null +++ b/test/resources/tdm/testdecls/removal_tests.decl @@ -0,0 +1,42 @@ +// Test declarations used for some DeclarationManager unit tests +// Don't even alter the whitespace in this file. +// Comments at the beginning of the file with an empty line in between + +decl/removal/0 +{ + diffusemap textures/removal/0 +} + +/* Without "testdecl" typename, C-style comment */ +decl/removal/1 +{ + diffusemap textures/removal/1 +} +// Some comments after decl/removal/1 + +/** + A multiline comment before this decl + */ +testdecl decl/removal/2 +{ + diffusemap textures/removal/2 +} +// Some comments after decl/removal/2 + +// CamelCase typename shouldn't make a difference +TestDecl decl/removal/3 +{ + diffusemap textures/removal/3 +} + +// Comment with a line before the start of the decl decl/removal/5 + +testdecl decl/removal/5 { // a comment in the same line as the opening brace + diffusemap textures/removal/5 +} + +// Everything in the same line +testdecl decl/removal/6 { diffusemap textures/removal/6 } +testdecl decl/removal/7 { diffusemap textures/removal/6 } + +testdecl decl/removal/8 { diffusemap textures/removal/6 }