Add HashTable coverage tests and fix try! usage

Kyle-Ye · Kyle-Ye · commit 3fcfaa65f065 · 2026-02-25T14:37:41.000+08:00
Add CustomTableTestHelper with string-based hash/compare to test
non-pointer code paths: custom constructor, string_hash, duplicate
insert/replace, non-pointer lookup/remove, found_key_out parameter,
destructor callbacks, and remove callbacks.

Replace try! #require with try #require across all UtilitiesTests.
diff --git a/Sources/Utilities/include/Utilities/TestingSupport.hpp b/Sources/Utilities/include/Utilities/TestingSupport.hpp
@@ -10,6 +10,7 @@
 
 #include <Utilities/ForwardList.hpp>
 #include <Utilities/Heap.hpp>
+#include <Utilities/HashTable.hpp>
 #include <Utilities/cf_ptr.hpp>
 #include <CoreFoundation/CFData.h>
 
@@ -55,6 +56,72 @@ OAG_INLINE uint64_t *_Nonnull heap_alloc_uint64(Heap *heap, size_t count = 1) SW
     return heap->alloc<uint64_t>(count);
 }
 
+// MARK: - HashTable
+
+OAG_INLINE uint64_t test_string_hash(const char *str) {
+    return string_hash(str);
+}
+
+class CustomTableTestHelper {
+    UntypedTable _table;
+
+    static uint64_t custom_hash(void const *key) {
+        return string_hash(static_cast<char const *>(key));
+    }
+
+    static bool custom_compare(void const *a, void const *b) {
+        return strcmp(static_cast<char const *>(a), static_cast<char const *>(b)) == 0;
+    }
+
+    static void on_remove_key(const void *_Nonnull key) {
+        _remove_key_count++;
+    }
+
+    static void on_remove_value(const void *_Nullable value) {
+        _remove_value_count++;
+    }
+
+    static int _remove_key_count;
+    static int _remove_value_count;
+
+public:
+    static CustomTableTestHelper *create() {
+        return new CustomTableTestHelper();
+    }
+
+    static void destroy(CustomTableTestHelper *value) { delete value; }
+
+    static int remove_key_count() { return _remove_key_count; }
+    static int remove_value_count() { return _remove_value_count; }
+    static void reset_counters() { _remove_key_count = 0; _remove_value_count = 0; }
+
+    bool empty() const { return _table.empty(); }
+    uint64_t count() const { return _table.count(); }
+
+    bool insert(const char *key, const void *_Nullable value) {
+        return _table.insert(key, value);
+    }
+
+    const void *_Nullable lookup(const char *key, const char *_Nullable *_Nullable found_key) {
+        return _table.lookup(key, (UntypedTable::nullable_key_type *)found_key);
+    }
+
+    bool remove(const char *key) {
+        return _table.remove(key);
+    }
+
+    void for_each(UntypedTable::entry_callback body, void *_Nullable context) const {
+        _table.for_each(body, context);
+    }
+
+private:
+    CustomTableTestHelper()
+        : _table(custom_hash, custom_compare, on_remove_key, on_remove_value, nullptr) {}
+} SWIFT_UNSAFE_REFERENCE;
+
+int CustomTableTestHelper::_remove_key_count = 0;
+int CustomTableTestHelper::_remove_value_count = 0;
+
 // MARK: - cf_ptr
 
 using cf_data_ptr = cf_ptr<CFDataRef>;
diff --git a/Tests/UtilitiesTests/CFPtrTests.swift b/Tests/UtilitiesTests/CFPtrTests.swift
@@ -39,7 +39,7 @@ struct CFPtrTests {
 
     @Test("Reset clears value")
     @available(iOS 16.4, *)
-    func resetClears() {
+    func resetClears() throws {
         let bytes: [UInt8] = [10, 20]
         let helper = bytes.withUnsafeBufferPointer { buf in
             util.CFPtrTestHelper.create(buf.baseAddress, Int(buf.count))
@@ -48,7 +48,7 @@ struct CFPtrTests {
             util.CFPtrTestHelper.destroy(helper)
         }
 
-        try! #require(helper.has_value() == true)
+        try #require(helper.has_value() == true)
 
         helper.reset()
 
@@ -110,7 +110,7 @@ struct CFPtrTests {
 
     @Test("Move transfers ownership")
     @available(iOS 16.4, *)
-    func moveTransfersOwnership() {
+    func moveTransfersOwnership() throws {
         let bytes: [UInt8] = [1, 2, 3, 4]
         let source = bytes.withUnsafeBufferPointer { buf in
             util.CFPtrTestHelper.create(buf.baseAddress, Int(buf.count))
@@ -119,7 +119,7 @@ struct CFPtrTests {
             util.CFPtrTestHelper.destroy(source)
         }
 
-        try! #require(source.has_value() == true)
+        try #require(source.has_value() == true)
 
         let dest = source.move_to_new()
         defer {
diff --git a/Tests/UtilitiesTests/HashTableTests.swift b/Tests/UtilitiesTests/HashTableTests.swift
@@ -5,7 +5,12 @@
 import Utilities
 import Testing
 
+// CustomTableTestHelper uses static counters for C function pointer callbacks
+// (C callbacks can't capture per-instance state). @MainActor ensures tests
+// touching those shared counters run serially on the main actor, preventing
+// data races while keeping the rest of the suite concurrent-safe.
 @Suite("HashTable tests")
+@MainActor
 struct HashTableTests {
 
     @Test("Initialize empty table")
@@ -108,7 +113,7 @@ struct HashTableTests {
     
     @Test("Remove entry")
     @available(iOS 16.4, *)
-    func removeEntry() {
+    func removeEntry() throws {
         class Value {
             let prop: String
             init(prop: String) {
@@ -123,12 +128,12 @@ struct HashTableTests {
 
         let key = "key1"
         let value = Value(prop: "valueProp")
-        withUnsafePointer(to: key) { keyPointer in
-            withUnsafePointer(to: value) { valuePointer in
+        try withUnsafePointer(to: key) { keyPointer in
+            try withUnsafePointer(to: value) { valuePointer in
                 let inserted = table.insert(keyPointer, valuePointer)
 
-                try! #require(inserted == true)
-                try! #require(table.count() == 1)
+                try #require(inserted == true)
+                try #require(table.count() == 1)
 
                 let removed = table.remove(keyPointer)
 
@@ -291,6 +296,228 @@ struct HashTableTests {
         #expect(iterationCount == 2, "for_each should visit exactly 2 items after reinsertion")
     }
 
+    // MARK: - Custom table tests (non-pointer compare path)
+
+    @Test("Custom table: insert and lookup by string content")
+    @available(iOS 16.4, *)
+    func customTableInsertAndLookup() throws {
+        util.CustomTableTestHelper.reset_counters()
+        let table = util.CustomTableTestHelper.create()
+        defer {
+            util.CustomTableTestHelper.destroy(table)
+        }
+
+        #expect(table.empty())
+
+        // Use strdup for persistent C strings the table can store safely
+        let key1 = strdup("hello")!
+        let inserted = table.insert(key1, nil)
+        #expect(inserted == true)
+        #expect(table.count() == 1)
+
+        // Lookup with a different pointer but same string content
+        let key2 = strdup("hello")!
+        defer { free(key2) }
+
+        var foundKey: UnsafePointer<CChar>? = nil
+        _ = table.__lookupUnsafe(key2, &foundKey)
+        let foundStr = try #require(foundKey, "Key should be found via custom compare")
+        #expect(String(cString: foundStr) == "hello")
+    }
+
+    @Test("Custom table: insert duplicate replaces value")
+    @available(iOS 16.4, *)
+    func customTableInsertDuplicate() {
+        util.CustomTableTestHelper.reset_counters()
+        let table = util.CustomTableTestHelper.create()
+        defer {
+            util.CustomTableTestHelper.destroy(table)
+        }
+
+        var value1 = 100
+        var value2 = 200
+
+        let key1 = strdup("key1")!
+        let inserted1 = withUnsafePointer(to: &value1) { v in
+            table.insert(key1, v)
+        }
+        #expect(inserted1 == true)
+        #expect(table.count() == 1)
+
+        let keyCountBefore = util.CustomTableTestHelper.remove_key_count()
+        let valCountBefore = util.CustomTableTestHelper.remove_value_count()
+
+        // Insert same key content again - should replace, not add
+        let key1dup = strdup("key1")!
+        let inserted2 = withUnsafePointer(to: &value2) { v in
+            table.insert(key1dup, v)
+        }
+        #expect(inserted2 == false, "Duplicate insert should return false")
+        #expect(table.count() == 1, "Count should not change on replace")
+
+        // Callbacks should have been called for the old key/value
+        #expect(util.CustomTableTestHelper.remove_key_count() == keyCountBefore + 1)
+        #expect(util.CustomTableTestHelper.remove_value_count() == valCountBefore + 1)
+        free(key1) // old key was replaced, free it
+    }
+
+    @Test("Custom table: remove calls callbacks")
+    @available(iOS 16.4, *)
+    func customTableRemoveCallbacks() {
+        util.CustomTableTestHelper.reset_counters()
+        let table = util.CustomTableTestHelper.create()
+        defer {
+            util.CustomTableTestHelper.destroy(table)
+        }
+
+        var value1 = 42
+        let key1 = strdup("removeMe")!
+        withUnsafePointer(to: &value1) { v in
+            _ = table.insert(key1, v)
+        }
+        #expect(table.count() == 1)
+
+        let keyCountBefore = util.CustomTableTestHelper.remove_key_count()
+        let valCountBefore = util.CustomTableTestHelper.remove_value_count()
+
+        // Remove using custom compare (non-pointer path) with different pointer
+        let key2 = strdup("removeMe")!
+        defer { free(key2) }
+        let removed = table.remove(key2)
+        #expect(removed == true)
+        #expect(table.count() == 0)
+
+        #expect(util.CustomTableTestHelper.remove_key_count() == keyCountBefore + 1)
+        #expect(util.CustomTableTestHelper.remove_value_count() == valCountBefore + 1)
+        free(key1) // removed key, free it
+    }
+
+    @Test("Custom table: remove non-existent key returns false")
+    @available(iOS 16.4, *)
+    func customTableRemoveNonExistent() {
+        util.CustomTableTestHelper.reset_counters()
+        let table = util.CustomTableTestHelper.create()
+        defer {
+            util.CustomTableTestHelper.destroy(table)
+        }
+
+        var value = 1
+        let key1 = strdup("existing")!
+        withUnsafePointer(to: &value) { v in
+            _ = table.insert(key1, v)
+        }
+
+        let key2 = strdup("nonexistent")!
+        defer { free(key2) }
+        let removed = table.remove(key2)
+        #expect(removed == false)
+        #expect(table.count() == 1)
+    }
+
+    @Test("Custom table: destructor calls callbacks for remaining entries")
+    @available(iOS 16.4, *)
+    func customTableDestructorCallbacks() {
+        util.CustomTableTestHelper.reset_counters()
+
+        let table = util.CustomTableTestHelper.create()
+        var v1 = 10
+        var v2 = 20
+        let keyA = strdup("a")!
+        let keyB = strdup("b")!
+        withUnsafePointer(to: &v1) { v in
+            _ = table.insert(keyA, v)
+        }
+        withUnsafePointer(to: &v2) { v in
+            _ = table.insert(keyB, v)
+        }
+        #expect(table.count() == 2)
+
+        // Destroy the table - should trigger callbacks for remaining 2 entries
+        util.CustomTableTestHelper.destroy(table)
+
+        #expect(util.CustomTableTestHelper.remove_key_count() == 2)
+        #expect(util.CustomTableTestHelper.remove_value_count() == 2)
+    }
+
+    @Test("Custom table: lookup with found_key_out")
+    @available(iOS 16.4, *)
+    func customTableLookupFoundKeyOut() {
+        util.CustomTableTestHelper.reset_counters()
+        let table = util.CustomTableTestHelper.create()
+        defer {
+            util.CustomTableTestHelper.destroy(table)
+        }
+
+        var value = 99
+        let key1 = strdup("theKey")!
+        withUnsafePointer(to: &value) { v in
+            _ = table.insert(key1, v)
+        }
+
+        // Lookup with found_key_out using different pointer with same content
+        let key2 = strdup("theKey")!
+        defer { free(key2) }
+        var foundKey: UnsafePointer<CChar>? = nil
+        let result = table.__lookupUnsafe(key2, &foundKey)
+        #expect(foundKey != nil, "found_key_out should be set on match")
+        #expect(result != nil, "Value should be returned")
+
+        // Lookup non-existent with found_key_out
+        let key3 = strdup("missing")!
+        defer { free(key3) }
+        var foundKey2: UnsafePointer<CChar>? = nil
+        let result2 = table.__lookupUnsafe(key3, &foundKey2)
+        #expect(foundKey2 == nil, "found_key_out should be nil for miss")
+        #expect(result2 == nil)
+    }
+
+    @Test("string_hash produces non-zero results")
+    @available(iOS 16.4, *)
+    func stringHashFunction() {
+        let hash1 = "hello".withCString { util.test_string_hash($0) }
+        let hash2 = "world".withCString { util.test_string_hash($0) }
+        let hash3 = "hello".withCString { util.test_string_hash($0) }
+
+        #expect(hash1 != 0, "Hash should be non-zero for non-empty string")
+        #expect(hash2 != 0, "Hash should be non-zero for non-empty string")
+        #expect(hash1 == hash3, "Same string should produce same hash")
+        #expect(hash1 != hash2, "Different strings should likely produce different hashes")
+    }
+
+    // MARK: - Pointer table tests (additional coverage)
+
+    @Test("Pointer table: lookup with found_key_out")
+    @available(iOS 16.4, *)
+    func pointerTableLookupFoundKeyOut() {
+        let table = util.UntypedTable.create()
+        defer {
+            util.UntypedTable.destroy(table)
+        }
+
+        var key = 42
+        var value = 100
+        withUnsafePointer(to: &key) { k in
+            withUnsafePointer(to: &value) { v in
+                _ = table.insert(k, v)
+            }
+        }
+
+        // Lookup with found_key_out on pointer table
+        var foundKey: UnsafeRawPointer? = nil
+        withUnsafePointer(to: &key) { k in
+            _ = table.__lookupUnsafe(k, &foundKey)
+        }
+        #expect(foundKey != nil, "found_key_out should be set on match")
+
+        // Miss case
+        var otherKey = 999
+        var foundKey2: UnsafeRawPointer? = nil
+        withUnsafePointer(to: &otherKey) { k in
+            _ = table.__lookupUnsafe(k, &foundKey2)
+        }
+        #expect(foundKey2 == nil, "found_key_out should be nil for miss")
+    }
+
     @Test("Grow buckets should not lose nodes")
     @available(iOS 16.4, *)
     func growBucketsPreservesAllNodes() {
diff --git a/Tests/UtilitiesTests/ObjCPtrTests.swift b/Tests/UtilitiesTests/ObjCPtrTests.swift
