#5127: Add unit test coverage for changed signals - and adjust the im…

…plementation
codereader · Jan 4, 2021 · 61104d5 · 61104d5
1 parent d2f55f4
commit 61104d5
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 4 deletions.
diff --git a/radiantcore/decl/FavouritesManager.cpp b/radiantcore/decl/FavouritesManager.cpp
@@ -28,8 +28,11 @@ void FavouritesManager::addFavourite(decl::Type type, const std::string& path)
         set = _favouritesByType.emplace(type, FavouriteSet()).first;
     }
 
-    set->second.get().emplace(path);
-    set->second.signal_setChanged().emit();
+    if (set->second.get().emplace(path).second)
+    {
+        // Fire signal only when something got added
+        set->second.signal_setChanged().emit();
+    }
 }
 
 void FavouritesManager::removeFavourite(decl::Type type, const std::string& path)
@@ -43,8 +46,11 @@ void FavouritesManager::removeFavourite(decl::Type type, const std::string& path
         return;
     }
 
-    set->second.get().erase(path);
-    set->second.signal_setChanged().emit();
+    if (set->second.get().erase(path) > 0)
+    {
+        // Fire signal only when something got removed
+        set->second.signal_setChanged().emit();
+    }
 }
 
 bool FavouritesManager::isFavourite(decl::Type type, const std::string& path)

diff --git a/test/Favourites.cpp b/test/Favourites.cpp
@@ -202,4 +202,37 @@ TEST_F(FavouritesTestWithLegacyFavourites, LegacyFavouritesAreImported)
     EXPECT_TRUE(legacyNodes.empty());
 }
 
+TEST_F(FavouritesTest, ChangedSignals)
+{
+    EXPECT_TRUE(GlobalFavouritesManager().getFavourites(decl::Type::Material).empty());
+
+    bool signalFired = false;
+
+    GlobalFavouritesManager().getSignalForType(decl::Type::Material).connect([&]()
+    {
+        signalFired = true;
+    });
+
+    // Add caulk
+    GlobalFavouritesManager().addFavourite(decl::Type::Material, "textures/common/caulk");
+    EXPECT_TRUE(signalFired);
+
+    signalFired = false; // reset
+
+    // Add the same again => shouldn't fire
+    GlobalFavouritesManager().addFavourite(decl::Type::Material, "textures/common/caulk");
+    EXPECT_FALSE(signalFired);
+
+    // Remove non-existent => shouldn't fire
+    GlobalFavouritesManager().removeFavourite(decl::Type::Material, "textures/doesntexist");
+    EXPECT_FALSE(signalFired);
+
+    // Add a different type => shouldn't fire
+    GlobalFavouritesManager().addFavourite(decl::Type::EntityDef, "my_entity");
+    EXPECT_FALSE(signalFired);
+
+    GlobalFavouritesManager().removeFavourite(decl::Type::Material, "textures/common/caulk");
+    EXPECT_TRUE(signalFired);
+}
+
 }