From 616e02b1ff23c5a8b56fb401544a25016d248441 Mon Sep 17 00:00:00 2001 From: Ravindra Kumar Bundela Date: Tue, 13 Oct 2026 23:35:08 +0530 Subject: [PATCH 1/4] [core] Add essential unit tests for TMap --- core/cont/test/CMakeLists.txt | 1 + core/cont/test/TMapTest.cxx | 286 ++++++++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+) create mode 100644 core/cont/test/TMapTest.cxx diff --git a/core/cont/test/CMakeLists.txt b/core/cont/test/CMakeLists.txt index 16836149f21e5..0df93f4c646d9 100644 --- a/core/cont/test/CMakeLists.txt +++ b/core/cont/test/CMakeLists.txt @@ -10,3 +10,4 @@ ROOT_ADD_GTEST(testTypedIteration testTypedIteration.cxx LIBRARIES Core) ROOT_ADD_GTEST(TSeqTests TSeqTests.cxx LIBRARIES Core) ROOT_ADD_GTEST(testIter testIter.cxx LIBRARIES Core) ROOT_ADD_GTEST(TListTest TListTest.cxx LIBRARIES Core) +ROOT_ADD_GTEST(TMapTest TMapTest.cxx LIBRARIES Core) diff --git a/core/cont/test/TMapTest.cxx b/core/cont/test/TMapTest.cxx new file mode 100644 index 0000000000000..46da70a5d1381 --- /dev/null +++ b/core/cont/test/TMapTest.cxx @@ -0,0 +1,286 @@ +#include "gtest/gtest.h" +#include "TMap.h" +#include "TNamed.h" + +namespace { +class TrackNamed : public TNamed { +public: + static int fgCount; + TrackNamed(const char *name, const char *title) : TNamed(name, title) { fgCount++; } + ~TrackNamed() override { fgCount--; } +}; +int TrackNamed::fgCount = 0; +} // namespace + +TEST(TMapTest, DefaultConstructor) +{ + TMap map; + EXPECT_EQ(map.GetSize(), 0); + EXPECT_TRUE(map.IsEmpty()); + EXPECT_GE(map.Capacity(), 0); + EXPECT_EQ(map.GetValue("nonexistent"), nullptr); + EXPECT_EQ(map.FindObject("nonexistent"), nullptr); +} + +TEST(TMapTest, AddAndLookup) +{ + TMap map; + map.SetOwnerKeyValue(kTRUE, kTRUE); + + EXPECT_TRUE(map.IsOwner()); + EXPECT_TRUE(map.IsOwnerValue()); + + auto *k1 = new TNamed("key1", "k1"); + auto *v1 = new TNamed("val1", "v1"); + auto *k2 = new TNamed("key2", "k2"); + auto *v2 = new TNamed("val2", "v2"); + + map.Add(k1, v1); + map.Add(k2, v2); + + EXPECT_EQ(map.GetSize(), 2); + EXPECT_FALSE(map.IsEmpty()); + EXPECT_NE(map.GetTable(), nullptr); + EXPECT_EQ(map.GetTable()->GetSize(), 2); + + // Lookup by string + EXPECT_EQ(map.GetValue("key1"), v1); + EXPECT_EQ(map.GetValue("key2"), v2); + EXPECT_EQ(map.GetValue("key3"), nullptr); + + // Lookup by object pointer + EXPECT_EQ(map.GetValue(k1), v1); + EXPECT_EQ(map.GetValue(k2), v2); + + TNamed dummy("key3", ""); + EXPECT_EQ(map.GetValue(&dummy), nullptr); + + // Function call operator operator() + EXPECT_EQ(map("key1"), v1); + EXPECT_EQ(map("key2"), v2); + EXPECT_EQ(map(k1), v1); + EXPECT_EQ(map(k2), v2); + + // FindObject returns TPair* + auto *p1 = static_cast(map.FindObject("key1")); + ASSERT_NE(p1, nullptr); + EXPECT_EQ(p1->Key(), k1); + EXPECT_EQ(p1->Value(), v1); + + auto *p2 = static_cast(map.FindObject(k2)); + ASSERT_NE(p2, nullptr); + EXPECT_EQ(p2->Key(), k2); + EXPECT_EQ(p2->Value(), v2); + + // Collisions check + EXPECT_GE(map.Collisions("key1"), 1); + EXPECT_GE(map.Collisions(k2), 1); + EXPECT_EQ(map.Collisions("nonexistent"), 0); +} + +TEST(TMapTest, Remove) +{ + TMap map; + + auto *k1 = new TNamed("k1", ""); + auto *v1 = new TNamed("v1", ""); + auto *k2 = new TNamed("k2", ""); + auto *v2 = new TNamed("v2", ""); + + map.Add(k1, v1); + map.Add(k2, v2); + + // Remove removes entry from map and returns the key + TObject *removedKey = map.Remove(k1); + EXPECT_EQ(removedKey, k1); + EXPECT_EQ(map.GetSize(), 1); + EXPECT_EQ(map.GetValue("k1"), nullptr); + EXPECT_EQ(map.GetValue("k2"), v2); + + map.Clear(); + delete k1; + delete v1; + delete k2; + delete v2; +} + +TEST(TMapTest, RemoveEntry) +{ + TMap map; + + auto *k1 = new TNamed("k1", ""); + auto *v1 = new TNamed("v1", ""); + + map.Add(k1, v1); + + TPair *entry = map.RemoveEntry(k1); + ASSERT_NE(entry, nullptr); + EXPECT_EQ(entry->Key(), k1); + EXPECT_EQ(entry->Value(), v1); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(map.GetValue("k1"), nullptr); + + delete entry; + delete k1; + delete v1; +} + +TEST(TMapTest, DeleteEntry) +{ + TrackNamed::fgCount = 0; + + { + TMap map; + map.SetOwnerKeyValue(kTRUE, kTRUE); + + auto *k1 = new TrackNamed("k1", ""); + auto *v1 = new TrackNamed("v1", ""); + auto *k2 = new TrackNamed("k2", ""); + auto *v2 = new TrackNamed("v2", ""); + + map.Add(k1, v1); + map.Add(k2, v2); + + EXPECT_EQ(TrackNamed::fgCount, 4); + + // DeleteEntry deletes both key and value because ownership is enabled + Bool_t deleted = map.DeleteEntry(k1); + EXPECT_TRUE(deleted); + EXPECT_EQ(map.GetSize(), 1); + EXPECT_EQ(map.GetValue("k1"), nullptr); + EXPECT_EQ(TrackNamed::fgCount, 2); + + // Try deleting non-existent key + TNamed nonexistent("dummy", ""); + EXPECT_FALSE(map.DeleteEntry(&nonexistent)); + } + + EXPECT_EQ(TrackNamed::fgCount, 0); +} + +TEST(TMapTest, Rehash) +{ + TMap map(5); + map.SetOwnerKeyValue(kTRUE, kTRUE); + + for (int i = 0; i < 20; ++i) { + map.Add(new TNamed(Form("key_%d", i), ""), new TNamed(Form("val_%d", i), "")); + } + + EXPECT_EQ(map.GetSize(), 20); + + map.Rehash(50); + EXPECT_GE(map.Capacity(), 50); + + for (int i = 0; i < 20; ++i) { + TObject *val = map.GetValue(Form("key_%d", i)); + ASSERT_NE(val, nullptr); + EXPECT_STREQ(val->GetName(), Form("val_%d", i)); + } +} + +TEST(TMapTest, ClearAndDelete) +{ + TrackNamed::fgCount = 0; + + // 1. Non-owner Clear(): removes entries from map without deleting keys or values + { + TMap map; + auto *k1 = new TrackNamed("k1", ""); + auto *v1 = new TrackNamed("v1", ""); + map.Add(k1, v1); + + EXPECT_EQ(TrackNamed::fgCount, 2); + EXPECT_EQ(map.GetSize(), 1); + + map.Clear(); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(TrackNamed::fgCount, 2); + + delete k1; + delete v1; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 2. DeleteKeys(): deletes only key objects + { + TMap map; + auto *k1 = new TrackNamed("k1", ""); + auto *v1 = new TrackNamed("v1", ""); + map.Add(k1, v1); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + map.DeleteKeys(); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(TrackNamed::fgCount, 1); // Value still alive + + delete v1; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 3. DeleteValues(): deletes only value objects + { + TMap map; + auto *k1 = new TrackNamed("k1", ""); + auto *v1 = new TrackNamed("v1", ""); + map.Add(k1, v1); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + map.DeleteValues(); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(TrackNamed::fgCount, 1); // Key still alive + + delete k1; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 4. DeleteAll(): deletes both key and value objects + { + TMap map; + map.Add(new TrackNamed("k1", ""), new TrackNamed("v1", "")); + map.Add(new TrackNamed("k2", ""), new TrackNamed("v2", "")); + + EXPECT_EQ(TrackNamed::fgCount, 4); + + map.DeleteAll(); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 5. Owner Clear(): deletes both keys and values when SetOwnerKeyValue(kTRUE, kTRUE) is set + { + TMap map; + map.SetOwnerKeyValue(kTRUE, kTRUE); + map.Add(new TrackNamed("k1", ""), new TrackNamed("v1", "")); + map.Add(new TrackNamed("k2", ""), new TrackNamed("v2", "")); + + EXPECT_EQ(TrackNamed::fgCount, 4); + + map.Clear(); + EXPECT_EQ(map.GetSize(), 0); + EXPECT_EQ(TrackNamed::fgCount, 0); + } +} + +TEST(TMapTest, Iteration) +{ + TMap map; + map.SetOwnerKeyValue(kTRUE, kTRUE); + + map.Add(new TNamed("alpha", "1"), new TNamed("valA", "")); + map.Add(new TNamed("beta", "2"), new TNamed("valB", "")); + map.Add(new TNamed("gamma", "3"), new TNamed("valC", "")); + + int count = 0; + TIter next(&map); + TObject *key = nullptr; + while ((key = next())) { + TObject *val = map.GetValue(key); + EXPECT_NE(val, nullptr); + count++; + } + + EXPECT_EQ(count, 3); +} From 917cb871d5594a8c4ede93bb5e0c692c6f64e991 Mon Sep 17 00:00:00 2001 From: Ravindra Kumar Bundela Date: Wed, 14 Oct 2026 01:27:15 +0530 Subject: [PATCH 2/4] [core] Enhance TMap unit tests with deliberate collision and ownership coverage --- core/cont/test/TMapTest.cxx | 136 ++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/core/cont/test/TMapTest.cxx b/core/cont/test/TMapTest.cxx index 46da70a5d1381..b773bfe94effa 100644 --- a/core/cont/test/TMapTest.cxx +++ b/core/cont/test/TMapTest.cxx @@ -10,6 +10,14 @@ class TrackNamed : public TNamed { ~TrackNamed() override { fgCount--; } }; int TrackNamed::fgCount = 0; + +class CollideNamed : public TNamed { + ULong_t fForcedHash; + +public: + CollideNamed(const char *name, const char *title, ULong_t h = 42) : TNamed(name, title), fForcedHash(h) {} + ULong_t Hash() const override { return fForcedHash; } +}; } // namespace TEST(TMapTest, DefaultConstructor) @@ -78,6 +86,48 @@ TEST(TMapTest, AddAndLookup) EXPECT_EQ(map.Collisions("nonexistent"), 0); } +TEST(TMapTest, HashCollision) +{ + TMap map; + map.SetOwnerKeyValue(kTRUE, kTRUE); + + auto *k1 = new CollideNamed("col_a", "First", 99); + auto *v1 = new TNamed("val_a", "Value A"); + auto *k2 = new CollideNamed("col_b", "Second", 99); + auto *v2 = new TNamed("val_b", "Value B"); + + map.Add(k1, v1); + map.Add(k2, v2); + + EXPECT_EQ(map.GetSize(), 2); + + // Both keys force the exact same hash (99), creating a deliberate collision in the bucket + EXPECT_EQ(map.Collisions("col_a"), 2); + EXPECT_EQ(map.Collisions("col_b"), 2); + EXPECT_EQ(map.Collisions(k1), 2); + EXPECT_EQ(map.Collisions(k2), 2); + + // Despite hash collision, lookups should correctly resolve both distinct items + EXPECT_EQ(map.GetValue("col_a"), v1); + EXPECT_EQ(map.GetValue("col_b"), v2); + EXPECT_EQ(map.GetValue(k1), v1); + EXPECT_EQ(map.GetValue(k2), v2); + + // Removing one collided item should leave the other intact and reduce collision count + TPair *entry = map.RemoveEntry(k1); + ASSERT_NE(entry, nullptr); + EXPECT_EQ(entry->Key(), k1); + EXPECT_EQ(entry->Value(), v1); + delete entry; + delete k1; + delete v1; + + EXPECT_EQ(map.GetSize(), 1); + EXPECT_EQ(map.Collisions("col_b"), 1); + EXPECT_EQ(map.GetValue("col_a"), nullptr); + EXPECT_EQ(map.GetValue("col_b"), v2); +} + TEST(TMapTest, Remove) { TMap map; @@ -179,6 +229,92 @@ TEST(TMapTest, Rehash) } } +TEST(TMapTest, OwnershipIndependent) +{ + TrackNamed::fgCount = 0; + + // 1. Only keys owned: SetOwner(kTRUE), SetOwnerValue(kFALSE) + { + TMap map; + map.SetOwner(kTRUE); + map.SetOwnerValue(kFALSE); + EXPECT_TRUE(map.IsOwner()); + EXPECT_FALSE(map.IsOwnerValue()); + + auto *k = new TrackNamed("k_only", ""); + auto *v = new TrackNamed("v_only", ""); + map.Add(k, v); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + map.Clear(); // deletes key, leaves value alive + EXPECT_EQ(TrackNamed::fgCount, 1); + + delete v; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 2. Only values owned: SetOwner(kFALSE), SetOwnerValue(kTRUE) + { + TMap map; + map.SetOwner(kFALSE); + map.SetOwnerValue(kTRUE); + EXPECT_FALSE(map.IsOwner()); + EXPECT_TRUE(map.IsOwnerValue()); + + auto *k = new TrackNamed("k_only", ""); + auto *v = new TrackNamed("v_only", ""); + map.Add(k, v); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + map.Clear(); // deletes value, leaves key alive + EXPECT_EQ(TrackNamed::fgCount, 1); + + delete k; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 3. Removal with value ownership enabled deletes value + { + TMap map; + map.SetOwnerValue(kTRUE); + + auto *k = new TrackNamed("k_rm", ""); + auto *v = new TrackNamed("v_rm", ""); + map.Add(k, v); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + TObject *retKey = map.Remove(k); + EXPECT_EQ(retKey, k); + // Remove() deletes value if IsOwnerValue() is true + EXPECT_EQ(TrackNamed::fgCount, 1); + + delete k; + EXPECT_EQ(TrackNamed::fgCount, 0); + } + + // 4. DeleteEntry with independent ownership + { + TMap map; + map.SetOwner(kTRUE); + map.SetOwnerValue(kFALSE); + + auto *k = new TrackNamed("k_del", ""); + auto *v = new TrackNamed("v_del", ""); + map.Add(k, v); + + EXPECT_EQ(TrackNamed::fgCount, 2); + + EXPECT_TRUE(map.DeleteEntry(k)); // deletes key, value stays alive + EXPECT_EQ(TrackNamed::fgCount, 1); + + delete v; + EXPECT_EQ(TrackNamed::fgCount, 0); + } +} + TEST(TMapTest, ClearAndDelete) { TrackNamed::fgCount = 0; From 210301d3c1d3da2379edb0c91bc80b461b630309 Mon Sep 17 00:00:00 2001 From: Ravindra Kumar Bundela Date: Wed, 14 Oct 2026 11:09:42 +0530 Subject: [PATCH 3/4] [core] Fix HashCollision test in TMapTest --- core/cont/test/TMapTest.cxx | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/core/cont/test/TMapTest.cxx b/core/cont/test/TMapTest.cxx index b773bfe94effa..d6e706b48c97c 100644 --- a/core/cont/test/TMapTest.cxx +++ b/core/cont/test/TMapTest.cxx @@ -10,14 +10,6 @@ class TrackNamed : public TNamed { ~TrackNamed() override { fgCount--; } }; int TrackNamed::fgCount = 0; - -class CollideNamed : public TNamed { - ULong_t fForcedHash; - -public: - CollideNamed(const char *name, const char *title, ULong_t h = 42) : TNamed(name, title), fForcedHash(h) {} - ULong_t Hash() const override { return fForcedHash; } -}; } // namespace TEST(TMapTest, DefaultConstructor) @@ -88,28 +80,30 @@ TEST(TMapTest, AddAndLookup) TEST(TMapTest, HashCollision) { - TMap map; + // Use a small fixed capacity table without automatic rehash so collisions are guaranteed + TMap map(3, 0); map.SetOwnerKeyValue(kTRUE, kTRUE); - auto *k1 = new CollideNamed("col_a", "First", 99); - auto *v1 = new TNamed("val_a", "Value A"); - auto *k2 = new CollideNamed("col_b", "Second", 99); - auto *v2 = new TNamed("val_b", "Value B"); + // "k0" (Hash=27504, 27504 % 3 == 0) and "k3" (Hash=27507, 27507 % 3 == 0) collide in slot 0 + auto *k1 = new TNamed("k0", "First"); + auto *v1 = new TNamed("v0", "Value 0"); + auto *k2 = new TNamed("k3", "Second"); + auto *v2 = new TNamed("v3", "Value 3"); map.Add(k1, v1); map.Add(k2, v2); EXPECT_EQ(map.GetSize(), 2); - // Both keys force the exact same hash (99), creating a deliberate collision in the bucket - EXPECT_EQ(map.Collisions("col_a"), 2); - EXPECT_EQ(map.Collisions("col_b"), 2); + // Both keys hash to the same bucket (slot 0) in the 3-slot table, creating a deliberate collision + EXPECT_EQ(map.Collisions("k0"), 2); + EXPECT_EQ(map.Collisions("k3"), 2); EXPECT_EQ(map.Collisions(k1), 2); EXPECT_EQ(map.Collisions(k2), 2); // Despite hash collision, lookups should correctly resolve both distinct items - EXPECT_EQ(map.GetValue("col_a"), v1); - EXPECT_EQ(map.GetValue("col_b"), v2); + EXPECT_EQ(map.GetValue("k0"), v1); + EXPECT_EQ(map.GetValue("k3"), v2); EXPECT_EQ(map.GetValue(k1), v1); EXPECT_EQ(map.GetValue(k2), v2); @@ -123,9 +117,9 @@ TEST(TMapTest, HashCollision) delete v1; EXPECT_EQ(map.GetSize(), 1); - EXPECT_EQ(map.Collisions("col_b"), 1); - EXPECT_EQ(map.GetValue("col_a"), nullptr); - EXPECT_EQ(map.GetValue("col_b"), v2); + EXPECT_EQ(map.Collisions("k3"), 1); + EXPECT_EQ(map.GetValue("k0"), nullptr); + EXPECT_EQ(map.GetValue("k3"), v2); } TEST(TMapTest, Remove) From 564e1bbd12785f0bc4d522fa0c75c437e2e6437e Mon Sep 17 00:00:00 2001 From: Ravindra Kumar Bundela Date: Wed, 14 Oct 2026 22:56:05 +0530 Subject: [PATCH 4/4] [core] Fix HashCollision in TMapTest to use CollideNamed with matching hash --- core/cont/test/TMapTest.cxx | 39 +++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/core/cont/test/TMapTest.cxx b/core/cont/test/TMapTest.cxx index d6e706b48c97c..2bbaf8bc3d524 100644 --- a/core/cont/test/TMapTest.cxx +++ b/core/cont/test/TMapTest.cxx @@ -10,6 +10,22 @@ class TrackNamed : public TNamed { ~TrackNamed() override { fgCount--; } }; int TrackNamed::fgCount = 0; + +class CollideNamed : public TNamed { + ULong_t fForcedHash; + +public: + CollideNamed(const char *name, const char *title, ULong_t h = 42) : TNamed(name, title), fForcedHash(h) {} + ULong_t Hash() const override { return fForcedHash; } + Bool_t IsEqual(const TObject *obj) const override + { + if (this == obj) + return kTRUE; + if (CollideNamed::Class() == obj->IsA()) + return (fName == ((CollideNamed *)obj)->fName); + return TNamed::IsEqual(obj); + } +}; } // namespace TEST(TMapTest, DefaultConstructor) @@ -80,30 +96,24 @@ TEST(TMapTest, AddAndLookup) TEST(TMapTest, HashCollision) { - // Use a small fixed capacity table without automatic rehash so collisions are guaranteed - TMap map(3, 0); + TMap map; map.SetOwnerKeyValue(kTRUE, kTRUE); - // "k0" (Hash=27504, 27504 % 3 == 0) and "k3" (Hash=27507, 27507 % 3 == 0) collide in slot 0 - auto *k1 = new TNamed("k0", "First"); - auto *v1 = new TNamed("v0", "Value 0"); - auto *k2 = new TNamed("k3", "Second"); - auto *v2 = new TNamed("v3", "Value 3"); + auto *k1 = new CollideNamed("col_a", "First", 42); + auto *v1 = new TNamed("val_a", "Value A"); + auto *k2 = new CollideNamed("col_b", "Second", 42); + auto *v2 = new TNamed("val_b", "Value B"); map.Add(k1, v1); map.Add(k2, v2); EXPECT_EQ(map.GetSize(), 2); - // Both keys hash to the same bucket (slot 0) in the 3-slot table, creating a deliberate collision - EXPECT_EQ(map.Collisions("k0"), 2); - EXPECT_EQ(map.Collisions("k3"), 2); + // Both keys have identical Hash() (42) and therefore collide in the same bucket EXPECT_EQ(map.Collisions(k1), 2); EXPECT_EQ(map.Collisions(k2), 2); // Despite hash collision, lookups should correctly resolve both distinct items - EXPECT_EQ(map.GetValue("k0"), v1); - EXPECT_EQ(map.GetValue("k3"), v2); EXPECT_EQ(map.GetValue(k1), v1); EXPECT_EQ(map.GetValue(k2), v2); @@ -117,9 +127,8 @@ TEST(TMapTest, HashCollision) delete v1; EXPECT_EQ(map.GetSize(), 1); - EXPECT_EQ(map.Collisions("k3"), 1); - EXPECT_EQ(map.GetValue("k0"), nullptr); - EXPECT_EQ(map.GetValue("k3"), v2); + EXPECT_EQ(map.Collisions(k2), 1); + EXPECT_EQ(map.GetValue(k2), v2); } TEST(TMapTest, Remove)