Add additional tests for handler mappings

Author: PaulSonOfLars

ext/handler_mapping.go

@@ -40,14 +40,18 @@ func (m *handlerMappings) remove(name string, group int) bool {
 		return false
 	}
 
-	for i, handler := range currHandlers {
+	for idx, handler := range currHandlers {
 		if handler.Name() != name {
 			continue
 		}
 
 		// Only one item left, so just delete the group entirely.
 		if len(currHandlers) == 1 {
-			m.handlerGroups = append(m.handlerGroups[:group], m.handlerGroups[group+1:]...)
+			// get index of the current group to remove it from the list of handlergroups
+			gIdx := getIndex(group, m.handlerGroups)
+			if gIdx != -1 {
+				m.handlerGroups = append(m.handlerGroups[:gIdx], m.handlerGroups[gIdx+1:]...)
+			}
 			delete(m.handlers, group)
 			return true
 		}
@@ -57,13 +61,22 @@ func (m *handlerMappings) remove(name string, group int) bool {
 		newHandlers := make([]Handler, len(m.handlers[group]))
 		copy(newHandlers, m.handlers[group])
 
-		m.handlers[group] = append(newHandlers[:i], newHandlers[i+1:]...)
+		m.handlers[group] = append(newHandlers[:idx], newHandlers[idx+1:]...)
 		return true
 	}
 	// handler not found - removal failed.
 	return false
 }
 
+func getIndex(find int, is []int) int {
+	for i, v := range is {
+		if v == find {
+			return i
+		}
+	}
+	return -1
+}
+
 func (m *handlerMappings) removeGroup(group int) bool {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
@@ -73,12 +86,12 @@ func (m *handlerMappings) removeGroup(group int) bool {
 		return false
 	}
 
-	for j, handlerGroup := range m.handlerGroups {
+	for idx, handlerGroup := range m.handlerGroups {
 		if handlerGroup != group {
 			continue
 		}
 
-		m.handlerGroups = append(m.handlerGroups[:j], m.handlerGroups[j+1:]...)
+		m.handlerGroups = append(m.handlerGroups[:idx], m.handlerGroups[idx+1:]...)
 		delete(m.handlers, group)
 		// Group found, and deleted. Success!
 		return true

ext/handler_mapping_test.go

@@ -90,3 +90,47 @@ func checkList(t *testing.T, name string, got []Handler, expected ...Handler) {
 		}
 	}
 }
+
+func Test_handlerMappings_remove(t *testing.T) {
+	m := &handlerMappings{}
+	handler := dummy{name: "test"}
+
+	t.Run("nonExistent", func(t *testing.T) {
+		// removing an item that doesnt exist returns "false"
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalSuccess", func(t *testing.T) {
+		m.add(handler, 0)
+		// removing an item that DOES exist, returns true
+		if got := m.remove(handler.Name(), 0); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+		// And so the second time, it returns false
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalSuccess", func(t *testing.T) {
+		m.add(handler, 0)
+		// removing an item that DOES exist, returns true
+		if got := m.remove(handler.Name(), 0); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+		// And so the second time, it returns false
+		if got := m.remove(handler.Name(), 0); got {
+			t.Errorf("remove() = %v, want false", got)
+		}
+	})
+
+	t.Run("removalDifferentIndexes", func(t *testing.T) {
+		m.add(handler, 1)
+		m.add(handler, 2)
+		if got := m.remove(handler.Name(), 2); !got {
+			t.Errorf("remove() = %v, want true", got)
+		}
+	})
+}