From a2609e26a07831bf1c1783caf6d23fd4de824346 Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Wed, 18 Feb 2026 15:59:36 +0100 Subject: [PATCH] fix: resolve 4 pre-existing test failures in channelserver - Guard nil listener/acceptConns in Server.Shutdown() to prevent panic in test servers that don't bind a network listener - Remove redundant userBinaryPartsLock in TestHandleMsgMhfLoaddata that caused a deadlock with handleMsgMhfLoaddata's own lock acquisition - Increase test save blob size from 200 to 150000 bytes to accommodate ZZ save pointer offsets (up to 146728) - Initialize MHFEquipment.Sigils[].Effects slices in test data to prevent index-out-of-range panic in SerializeWarehouseEquipment - Insert warehouse row before updating it (UPDATE on 0 rows is not an error, so the INSERT fallback never triggered) - Use COALESCE for nullable kouryou_point column in kill counter test - Fix duplicate-add test expectation (CSV helper correctly deduplicates) --- server/channelserver/handlers_clients_test.go | 2 +- server/channelserver/handlers_data_test.go | 7 +- .../handlers_savedata_integration_test.go | 68 ++++++++++--------- .../session_lifecycle_integration_test.go | 13 ++-- server/channelserver/sys_channel_server.go | 8 ++- 5 files changed, 53 insertions(+), 45 deletions(-) diff --git a/server/channelserver/handlers_clients_test.go b/server/channelserver/handlers_clients_test.go index 15708cb51..e358066cc 100644 --- a/server/channelserver/handlers_clients_test.go +++ b/server/channelserver/handlers_clients_test.go @@ -494,7 +494,7 @@ func TestOprMember_EdgeCases_Integration(t *testing.T) { initialList: "1,2,3", operation: false, // add targetCharIDs: []uint32{2}, - wantList: "1,2,3,2", // CSV helper adds duplicates + wantList: "1,2,3", // CSV helper deduplicates }, { name: "remove_nonexistent_from_list", diff --git a/server/channelserver/handlers_data_test.go b/server/channelserver/handlers_data_test.go index 772246f6b..8dca38dd0 100644 --- a/server/channelserver/handlers_data_test.go +++ b/server/channelserver/handlers_data_test.go @@ -444,8 +444,6 @@ func TestHandleMsgMhfLoaddata_Integration(t *testing.T) { s.charID = charID s.server.db = db s.server.userBinaryParts = make(map[userBinaryPartID][]byte) - s.server.userBinaryPartsLock.Lock() - defer s.server.userBinaryPartsLock.Unlock() pkt := &mhfpacket.MsgMhfLoaddata{ AckHandle: 5678, @@ -570,7 +568,8 @@ func TestSaveDataCorruptionDetection_Integration(t *testing.T) { s.server.erupeConfig.DeleteOnSaveCorruption = false // Create save data with a DIFFERENT name (corruption) - corruptedData := make([]byte, 200) + // Must be large enough for ZZ save pointer offsets (highest: pKQF at 146728) + corruptedData := make([]byte, 150000) copy(corruptedData[88:], []byte("HackedName\x00")) compressed, _ := nullcomp.Compress(corruptedData) @@ -618,7 +617,7 @@ func TestConcurrentSaveData_Integration(t *testing.T) { s.Name = fmt.Sprintf("Char%d", index) s.server.db = db - saveData := make([]byte, 200) + saveData := make([]byte, 150000) copy(saveData[88:], []byte(fmt.Sprintf("Char%d\x00", index))) compressed, _ := nullcomp.Compress(saveData) diff --git a/server/channelserver/handlers_savedata_integration_test.go b/server/channelserver/handlers_savedata_integration_test.go index 378a6cbb2..b44c5f4a6 100644 --- a/server/channelserver/handlers_savedata_integration_test.go +++ b/server/channelserver/handlers_savedata_integration_test.go @@ -120,7 +120,7 @@ func TestSaveLoad_MonsterKillCounter(t *testing.T) { // Initial Koryo points initialPoints := uint32(0) - err := db.QueryRow("SELECT kouryou_point FROM characters WHERE id = $1", charID).Scan(&initialPoints) + err := db.QueryRow("SELECT COALESCE(kouryou_point, 0) FROM characters WHERE id = $1", charID).Scan(&initialPoints) if err != nil { t.Fatalf("Failed to query initial koryo points: %v", err) } @@ -197,28 +197,30 @@ func TestSaveLoad_Warehouse(t *testing.T) { userID := CreateTestUser(t, db, "testuser") charID := CreateTestCharacter(t, db, userID, "TestChar") - // Create test equipment for warehouse + // Create test equipment for warehouse (Decorations and Sigils must be initialized) + newEquip := func(id uint16, wid uint32) mhfitem.MHFEquipment { + e := mhfitem.MHFEquipment{ItemID: id, WarehouseID: wid} + e.Decorations = make([]mhfitem.MHFItem, 3) + e.Sigils = make([]mhfitem.MHFSigil, 3) + for i := range e.Sigils { + e.Sigils[i].Effects = make([]mhfitem.MHFSigilEffect, 3) + } + return e + } equipment := []mhfitem.MHFEquipment{ - {ItemID: 100, WarehouseID: 1}, - {ItemID: 101, WarehouseID: 2}, - {ItemID: 102, WarehouseID: 3}, + newEquip(100, 1), + newEquip(101, 2), + newEquip(102, 3), } // Serialize and save to warehouse serializedEquip := mhfitem.SerializeWarehouseEquipment(equipment) - // Update warehouse equip0 + // Initialize warehouse row then update + _, _ = db.Exec("INSERT INTO warehouse (character_id) VALUES ($1) ON CONFLICT DO NOTHING", charID) _, err := db.Exec("UPDATE warehouse SET equip0 = $1 WHERE character_id = $2", serializedEquip, charID) if err != nil { - // Warehouse entry might not exist, try insert - _, err = db.Exec(` - INSERT INTO warehouse (character_id, equip0) - VALUES ($1, $2) - ON CONFLICT (character_id) DO UPDATE SET equip0 = $2 - `, charID, serializedEquip) - if err != nil { - t.Fatalf("Failed to save warehouse: %v", err) - } + t.Fatalf("Failed to save warehouse: %v", err) } // Reload warehouse @@ -368,11 +370,16 @@ func TestSaveLoad_Transmog(t *testing.T) { s.charID = charID s.server.db = db - // Create transmog/decoration set data - transmogData := make([]byte, 100) - for i := range transmogData { - transmogData[i] = byte((i * 3) % 256) - } + // Create valid transmog/decoration set data + // Format: [version byte][count byte][count * (uint16 index + setSize bytes)] + // setSize is 76 for G10+, 68 otherwise + setSize := 76 // G10+ + numSets := 1 + transmogData := make([]byte, 2+numSets*(2+setSize)) + transmogData[0] = 1 // version + transmogData[1] = byte(numSets) // count + transmogData[2] = 0 // index high byte + transmogData[3] = 1 // index low byte (set #1) // Save transmog data pkt := &mhfpacket.MsgMhfSaveDecoMyset{ @@ -409,23 +416,20 @@ func TestSaveLoad_CraftedEquipment(t *testing.T) { // Crafted equipment would be stored in savedata or warehouse // Let's test warehouse equipment with upgrade levels - // Create crafted equipment with upgrade level - equipment := []mhfitem.MHFEquipment{ - { - ItemID: 5000, // Crafted weapon - WarehouseID: 12345, - // Upgrade level would be in equipment metadata - }, + // Create crafted equipment with upgrade level (Decorations and Sigils must be initialized) + equip := mhfitem.MHFEquipment{ItemID: 5000, WarehouseID: 12345} + equip.Decorations = make([]mhfitem.MHFItem, 3) + equip.Sigils = make([]mhfitem.MHFSigil, 3) + for i := range equip.Sigils { + equip.Sigils[i].Effects = make([]mhfitem.MHFSigilEffect, 3) } + equipment := []mhfitem.MHFEquipment{equip} serialized := mhfitem.SerializeWarehouseEquipment(equipment) // Save to warehouse - _, err := db.Exec(` - INSERT INTO warehouse (character_id, equip0) - VALUES ($1, $2) - ON CONFLICT (character_id) DO UPDATE SET equip0 = $2 - `, charID, serialized) + _, _ = db.Exec("INSERT INTO warehouse (character_id) VALUES ($1) ON CONFLICT DO NOTHING", charID) + _, err := db.Exec("UPDATE warehouse SET equip0 = $1 WHERE character_id = $2", serialized, charID) if err != nil { t.Fatalf("Failed to save crafted equipment: %v", err) } diff --git a/server/channelserver/session_lifecycle_integration_test.go b/server/channelserver/session_lifecycle_integration_test.go index 06afc1a40..7cba56146 100644 --- a/server/channelserver/session_lifecycle_integration_test.go +++ b/server/channelserver/session_lifecycle_integration_test.go @@ -179,11 +179,8 @@ func TestSessionLifecycle_WarehouseDataPersistence(t *testing.T) { serializedEquip := mhfitem.SerializeWarehouseEquipment(equipment) // Save to warehouse directly (simulating a save handler) - _, err := db.Exec(` - INSERT INTO warehouse (character_id, equip0) - VALUES ($1, $2) - ON CONFLICT (character_id) DO UPDATE SET equip0 = $2 - `, charID, serializedEquip) + _, _ = db.Exec("INSERT INTO warehouse (character_id) VALUES ($1) ON CONFLICT DO NOTHING", charID) + _, err := db.Exec("UPDATE warehouse SET equip0 = $1 WHERE character_id = $2", serializedEquip, charID) if err != nil { t.Fatalf("Failed to save warehouse: %v", err) } @@ -562,11 +559,15 @@ func TestSessionLifecycle_RapidReconnect(t *testing.T) { // Helper function to create test equipment item with proper initialization func createTestEquipmentItem(itemID uint16, warehouseID uint32) mhfitem.MHFEquipment { + sigils := make([]mhfitem.MHFSigil, 3) + for i := range sigils { + sigils[i].Effects = make([]mhfitem.MHFSigilEffect, 3) + } return mhfitem.MHFEquipment{ ItemID: itemID, WarehouseID: warehouseID, Decorations: make([]mhfitem.MHFItem, 3), - Sigils: make([]mhfitem.MHFSigil, 3), + Sigils: sigils, } } diff --git a/server/channelserver/sys_channel_server.go b/server/channelserver/sys_channel_server.go index d4309cf82..abdc1093e 100644 --- a/server/channelserver/sys_channel_server.go +++ b/server/channelserver/sys_channel_server.go @@ -223,9 +223,13 @@ func (s *Server) Shutdown() { s.isShuttingDown = true s.Unlock() - _ = s.listener.Close() + if s.listener != nil { + _ = s.listener.Close() + } - close(s.acceptConns) + if s.acceptConns != nil { + close(s.acceptConns) + } } func (s *Server) acceptClients() {