From 8ad125d0cf74e6499cb08f8d9def20294e7a032d Mon Sep 17 00:00:00 2001 From: Molecule AI Fullstack Engineer Date: Wed, 13 May 2026 12:19:28 +0000 Subject: [PATCH] test(handlers): add 14 additional pure-function cases to org_helpers_pure_test.go Extends the staging org_helpers_pure_test.go with coverage from feat/709 that was missing due to add/add conflict when the base branch diverged. New test cases: - expandWithEnv: BracedVar, DollarVar, Mixed, MissingVar, EmptyMap, LiteralDollar, PartiallyPresent - mergeCategoryRouting: WorkspaceAddsCategory, EmptyListDropsCategory, EmptyDefaultKeySkipped, EmptyWorkspaceKeySkipped, DoesNotMutateInputs - renderCategoryRoutingYAML: SingleCategory, MultipleCategoriesSorted, EmptyListCategory (join existing coverage) - appendYAMLBlock: BothEmpty, ExistingHasNewline, ExistingNoNewline, ExistingEmpty, NilExisting - mergePlugins: DefaultsOnly, WorkspaceAdds, DeduplicationOrder, ExclusionThenAddSameName - isSafeRoleName: SpecialCharsRejected Closes #709 Co-Authored-By: Claude Opus 4.7 --- .../handlers/org_helpers_pure_test.go | 291 ++++++++++++++++++ 1 file changed, 291 insertions(+) diff --git a/workspace-server/internal/handlers/org_helpers_pure_test.go b/workspace-server/internal/handlers/org_helpers_pure_test.go index 2324314a..6e3a8a50 100644 --- a/workspace-server/internal/handlers/org_helpers_pure_test.go +++ b/workspace-server/internal/handlers/org_helpers_pure_test.go @@ -2,6 +2,8 @@ package handlers import ( "testing" + + "github.com/stretchr/testify/assert" ) // ── isSafeRoleName ──────────────────────────────────────────────────────────── @@ -419,3 +421,292 @@ func TestMergePlugins_EmptyPlugin(t *testing.T) { t.Errorf("got %v, want 2 items", r) } } + +// ── Additional coverage: expandWithEnv ────────────────────────────── +func TestExpandWithEnv_BracedVar(t *testing.T) { + env := map[string]string{"FOO": "bar", "BAZ": "qux"} + result := expandWithEnv("value is ${FOO}", env) + assert.Equal(t, "value is bar", result) +} + +func TestExpandWithEnv_DollarVar(t *testing.T) { + env := map[string]string{"X": "1", "Y": "2"} + result := expandWithEnv("$X + $Y = 3", env) + assert.Equal(t, "1 + 2 = 3", result) +} + +func TestExpandWithEnv_Mixed(t *testing.T) { + env := map[string]string{"A": "alpha", "B": "beta"} + result := expandWithEnv("${A}_${B}", env) + assert.Equal(t, "alpha_beta", result) +} + +func TestExpandWithEnv_MissingVar(t *testing.T) { + // Missing vars stay as-is (os.Getenv fallback returns "" for unset vars). + env := map[string]string{} + result := expandWithEnv("${UNSET}", env) + assert.Equal(t, "", result) +} + +func TestExpandWithEnv_EmptyMap(t *testing.T) { + result := expandWithEnv("no vars here", map[string]string{}) + assert.Equal(t, "no vars here", result) +} + +func TestExpandWithEnv_LiteralDollar(t *testing.T) { + // A bare $ not followed by a valid identifier char stays as-is. + result := expandWithEnv("cost $100", map[string]string{}) + assert.Equal(t, "cost $100", result) +} + +func TestExpandWithEnv_PartiallyPresent(t *testing.T) { + env := map[string]string{"SET": "yes"} + result := expandWithEnv("${SET} and ${NOT_SET}", env) + // ${SET} resolved; ${NOT_SET} -> "" via empty fallback. + assert.Equal(t, "yes and ", result) +} + +// mergeCategoryRouting tests — unions defaults with per-workspace routing. + +// ── Additional coverage: mergeCategoryRouting ────────────────────── +func TestMergeCategoryRouting_WorkspaceAddsCategory(t *testing.T) { + defaults := map[string][]string{ + "security": {"Backend Engineer"}, + } + wsRouting := map[string][]string{ + "ui": {"Frontend Engineer"}, + } + result := mergeCategoryRouting(defaults, wsRouting) + assert.Equal(t, []string{"Backend Engineer"}, result["security"]) + assert.Equal(t, []string{"Frontend Engineer"}, result["ui"]) +} + +func TestMergeCategoryRouting_EmptyListDropsCategory(t *testing.T) { + defaults := map[string][]string{ + "security": {"Backend Engineer"}, + "infra": {"SRE"}, + } + wsRouting := map[string][]string{ + "security": {}, // empty list = explicit drop + } + result := mergeCategoryRouting(defaults, wsRouting) + _, hasSecurity := result["security"] + assert.False(t, hasSecurity) + assert.Equal(t, []string{"SRE"}, result["infra"]) +} + +func TestMergeCategoryRouting_EmptyDefaultKeySkipped(t *testing.T) { + defaults := map[string][]string{ + "": {"Backend Engineer"}, // empty key should be skipped + } + result := mergeCategoryRouting(defaults, nil) + _, has := result[""] + assert.False(t, has) +} + +func TestMergeCategoryRouting_EmptyWorkspaceKeySkipped(t *testing.T) { + defaults := map[string][]string{ + "security": {"Backend Engineer"}, + } + wsRouting := map[string][]string{ + "": {"Some Role"}, + } + result := mergeCategoryRouting(defaults, wsRouting) + _, has := result[""] + assert.False(t, has) + assert.Equal(t, []string{"Backend Engineer"}, result["security"]) +} + +func TestMergeCategoryRouting_DoesNotMutateInputs(t *testing.T) { + defaults := map[string][]string{ + "security": {"Backend Engineer"}, + } + wsRouting := map[string][]string{ + "security": {"DevOps"}, + } + orig := defaults["security"][0] + _ = mergeCategoryRouting(defaults, wsRouting) + assert.Equal(t, orig, defaults["security"][0]) +} + +// renderCategoryRoutingYAML tests — deterministic YAML emission. + +// ── Additional coverage: renderCategoryRoutingYAML ──────────────── +func TestRenderCategoryRoutingYAML_SingleCategory(t *testing.T) { + routing := map[string][]string{ + "security": {"Backend Engineer", "DevOps"}, + } + result, err := renderCategoryRoutingYAML(routing) + assert.NoError(t, err) + assert.Contains(t, result, "security:") + assert.Contains(t, result, "Backend Engineer") + assert.Contains(t, result, "DevOps") +} + +func TestRenderCategoryRoutingYAML_MultipleCategoriesSorted(t *testing.T) { + routing := map[string][]string{ + "zebra": {"RoleZ"}, + "alpha": {"RoleA"}, + "middleware": {"RoleM"}, + } + result, err := renderCategoryRoutingYAML(routing) + assert.NoError(t, err) + // Keys are sorted alphabetically. + idxAlpha := assertFind(t, result, "alpha:") + idxZebra := assertFind(t, result, "zebra:") + idxMid := assertFind(t, result, "middleware:") + if idxAlpha > -1 && idxZebra > -1 { + assert.True(t, idxAlpha < idxZebra, "alpha should appear before zebra") + } + if idxMid > -1 && idxZebra > -1 { + assert.True(t, idxMid < idxZebra, "middleware should appear before zebra") + } +} + +func TestRenderCategoryRoutingYAML_EmptyListCategory(t *testing.T) { + // Empty-list category should still render (mergeCategoryRouting drops + // them before they reach this function, but we test the render in isolation). + routing := map[string][]string{ + "security": {}, + } + result, err := renderCategoryRoutingYAML(routing) + assert.NoError(t, err) + assert.Contains(t, result, "security:") +} + +func TestRenderCategoryRoutingYAML_SpecialCharactersEscaped(t *testing.T) { + routing := map[string][]string{ + "notes": {`has: colon`, `and "quotes"`, "emoji: 🚀"}, + } + result, err := renderCategoryRoutingYAML(routing) + assert.NoError(t, err) + // Should not panic and should produce valid YAML. + assert.Contains(t, result, "notes:") +} + +// appendYAMLBlock tests — safe concatenation with newline boundary. + +// ── Additional coverage: appendYAMLBlock ─────────────────────────── +func TestAppendYAMLBlock_BothEmpty(t *testing.T) { + result := appendYAMLBlock(nil, "") + assert.Equal(t, "", result) +} + +func TestAppendYAMLBlock_ExistingHasNewline(t *testing.T) { + existing := []byte("existing:\n") + block := "key: value\n" + result := appendYAMLBlock(existing, block) + assert.Equal(t, "existing:\nkey: value\n", string(result)) +} + +func TestAppendYAMLBlock_ExistingNoNewline(t *testing.T) { + existing := []byte("existing:") + block := "key: value\n" + result := appendYAMLBlock(existing, block) + assert.Equal(t, "existing:\nkey: value\n", string(result)) +} + +func TestAppendYAMLBlock_ExistingEmpty(t *testing.T) { + existing := []byte("") + block := "key: value\n" + result := appendYAMLBlock(existing, block) + assert.Equal(t, "key: value\n", string(result)) +} + +func TestAppendYAMLBlock_NilExisting(t *testing.T) { + block := "key: value\n" + result := appendYAMLBlock(nil, block) + assert.Equal(t, "key: value\n", string(result)) +} + +// mergePlugins tests — union with exclusion prefix (!/-). + +// ── Additional coverage: mergePlugins (additional cases) ─────────── +func TestMergePlugins_DefaultsOnly(t *testing.T) { + defaults := []string{"plugin-a", "plugin-b"} + result := mergePlugins(defaults, nil) + assert.Equal(t, []string{"plugin-a", "plugin-b"}, result) +} + +func TestMergePlugins_WorkspaceAdds(t *testing.T) { + defaults := []string{"plugin-a"} + wsPlugins := []string{"plugin-b", "plugin-a"} // duplicate of default + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-b"}, result) +} + +func TestMergePlugins_ExclusionWithBang(t *testing.T) { + defaults := []string{"plugin-a", "plugin-b", "plugin-c"} + wsPlugins := []string{"!plugin-b"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-c"}, result) +} + +func TestMergePlugins_ExclusionWithDash(t *testing.T) { + defaults := []string{"plugin-a", "plugin-b", "plugin-c"} +func TestMergePlugins_ExclusionEmptyTarget(t *testing.T) { + defaults := []string{"plugin-a", "plugin-b"} + wsPlugins := []string{"!", "-"} // no-op exclusions + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-b"}, result) +} + +func TestMergePlugins_ExclusionNotInDefaults(t *testing.T) { + // Excluding something not in defaults is a no-op. + defaults := []string{"plugin-a"} + wsPlugins := []string{"!plugin-b"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a"}, result) +} + +func TestMergePlugins_WorkspaceAddsNew(t *testing.T) { + defaults := []string{"plugin-a"} +func TestMergePlugins_DeduplicationOrder(t *testing.T) { + // Defaults first; workspace entries deduplicated. + defaults := []string{"plugin-a", "plugin-a", "plugin-b"} + wsPlugins := []string{"plugin-b", "plugin-c", "plugin-c"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-b", "plugin-c"}, result) +} + +func TestMergePlugins_ExclusionThenAddSameName(t *testing.T) { + // Remove then re-add: order matters. + defaults := []string{"plugin-a", "plugin-b"} + wsPlugins := []string{"!plugin-a", "plugin-a"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-b", "plugin-a"}, result) +} + +// isSafeRoleName tests — alphanumeric + hyphen/underscore, no path separators. + +// ── Additional coverage: isSafeRoleName ─────────────────────────── +func TestIsSafeRoleName_SpecialCharsRejected(t *testing.T) { + bad := []string{ + "role@name", + "role#name", + "role$name", + "role%name", + "role&name", + "role*name", + "role?name", + "role=name", + } + for _, r := range bad { + if isSafeRoleName(r) { + t.Errorf("isSafeRoleName(%q) expected false, got true", r) + } + } +} + +// assertFind is a helper: returns index of first occurrence of substr in s, or -1. +func assertFind(t *testing.T, s, substr string) int { + t.Helper() + idx := -1 + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + idx = i + break + } + } + return idx +}