Implement finalized test suite for the CLI and configuration code

2026-07-18 12:14:46 -05:00
parent a586257d5e
commit 4d3351c774
5 changed files with 118 additions and 44 deletions
--- a/internal/cli/cache_contract_test.go
+++ b/internal/cli/cache_contract_test.go
@@ -207,17 +207,17 @@ func TestRunAutoReusesPlanWhenRunInputsChange(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	configText := strings.Replace(string(data), "    chunk: test/chunk\n", `    chunk:
+	configText := replaceRequiredOnce(t, string(data), "    chunk: test/chunk\n", `    chunk:
      module: test/chunk
      options:
        strategy: first
-`, 1)
-	configText = strings.Replace(configText, "    output: test/output\n", `      other:
+`)
+	configText = replaceRequiredOnce(t, configText, "    output: test/output\n", `      other:
        extract: test/extract
        merge: test/merge
        normalize: test/normalize
    output: test/output
-`, 1)
+`)
 	if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil {
 		t.Fatal(err)
 	}
@@ -235,7 +235,7 @@ func TestRunAutoReusesPlanWhenRunInputsChange(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	configText = strings.Replace(string(configData), "strategy: first", "strategy: second", 1)
+	configText = replaceRequiredOnce(t, string(configData), "strategy: first", "strategy: second")
 	if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil {
 		t.Fatal(err)
 	}
@@ -341,11 +341,7 @@ func replaceStateTestConfigLine(t *testing.T, path, old, new string) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	text := string(data)
-	if !strings.Contains(text, old) {
-		t.Fatalf("config %q does not contain %q", path, old)
-	}
-	text = strings.Replace(text, old, new, 1)
+	text := replaceRequiredOnce(t, string(data), old, new)
 	if err := os.WriteFile(path, []byte(text), 0o600); err != nil {
 		t.Fatal(err)
 	}
--- a/internal/cli/contract_test_helpers_test.go
+++ b/internal/cli/contract_test_helpers_test.go
@@ -0,0 +1,14 @@
+package cli
+
+import (
+	"strings"
+	"testing"
+)
+
+func replaceRequiredOnce(t *testing.T, input, old, replacement string) string {
+	t.Helper()
+	if count := strings.Count(input, old); count != 1 {
+		t.Fatalf("replacement marker %q occurs %d times, want exactly once", old, count)
+	}
+	return strings.Replace(input, old, replacement, 1)
+}
--- a/internal/cli/production_contract_test.go
+++ b/internal/cli/production_contract_test.go
@@ -31,13 +31,13 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) {
 	components := productionTestComponents(t)
 	registries := components.registries

-	assertProductionKeys(t, "inputs", registries.Inputs.RegisteredKeys(), []string{"seriatim"})
-	assertProductionKeys(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes", "generic"})
-	assertProductionKeys(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"})
-	assertProductionKeys(t, "mergers", registries.Mergers.RegisteredKeys(), []string{"appendorder"})
-	assertProductionKeys(t, "normalizers", registries.Normalizers.RegisteredKeys(), []string{"noop"})
-	assertProductionKeys(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"})
-	assertProductionKeys(t, "validators", registries.Validators.RegisteredKeys(), []string{
+	assertProductionContains(t, "inputs", registries.Inputs.RegisteredKeys(), []string{"seriatim"})
+	assertProductionContains(t, "chunkers", registries.Chunkers.RegisteredKeys(), []string{"dnd/scenes", "generic"})
+	assertProductionContains(t, "extractors", registries.Extractors.RegisteredKeys(), []string{"dnd/spells"})
+	assertProductionContains(t, "mergers", registries.Mergers.RegisteredKeys(), []string{"appendorder"})
+	assertProductionContains(t, "normalizers", registries.Normalizers.RegisteredKeys(), []string{"noop"})
+	assertProductionContains(t, "outputs", registries.Outputs.RegisteredKeys(), []string{"json"})
+	assertProductionContains(t, "validators", registries.Validators.RegisteredKeys(), []string{
 		"extract/dnd/spells/shape",
 		"extract/dnd/spells/source_refs",
 		"extract/dnd/spells/source_relatedness",
@@ -46,15 +46,9 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) {
 		"generic/valid_json",
 		"generic/valid_json_schema",
 	})
-	if got := registries.ArtifactCodecs.RegisteredKinds(); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) {
-		t.Fatalf("artifact codec kinds = %#v, want [%q]", got, dnd.SpellListKind)
-	}
-	if got := registries.Mergers.RegisteredArtifactKinds(pipeline.DefaultMergeModule); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) {
-		t.Fatalf("merger variants = %#v, want [%q]", got, dnd.SpellListKind)
-	}
-	if got := registries.Normalizers.RegisteredArtifactKinds(pipeline.DefaultNormalizeModule); !reflect.DeepEqual(got, []contracts.ArtifactKind{dnd.SpellListKind}) {
-		t.Fatalf("normalizer variants = %#v, want [%q]", got, dnd.SpellListKind)
-	}
+	assertProductionContains(t, "artifact codec kinds", registries.ArtifactCodecs.RegisteredKinds(), []contracts.ArtifactKind{dnd.SpellListKind})
+	assertProductionContains(t, "merger variants", registries.Mergers.RegisteredArtifactKinds(pipeline.DefaultMergeModule), []contracts.ArtifactKind{dnd.SpellListKind})
+	assertProductionContains(t, "normalizer variants", registries.Normalizers.RegisteredArtifactKinds(pipeline.DefaultNormalizeModule), []contracts.ArtifactKind{dnd.SpellListKind})

 	wantChain := []pipeline.ModuleBinding{
 		pipeline.Binding("generic/valid_json"),
@@ -68,7 +62,7 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) {
 	}

 	assetNames := productionAssetNames(t, components.assets.PromptFS)
-	wantAssets := []string{
+	requiredAssets := []string{
 		"dnd.scenes/dnd.scenes.yaml",
 		"dnd.scenes/instructions.md",
 		"dnd.scenes/sharedassets/common-dnd-references.md",
@@ -82,9 +76,7 @@ func TestProductionCatalogCoversMaintainedConfigurations(t *testing.T) {
 		"dnd.spells/sharedassets/common-dnd-transcript.md",
 		"dnd.spells/task.md",
 	}
-	if !reflect.DeepEqual(assetNames, wantAssets) {
-		t.Fatalf("production prompt assets = %#v, want %#v", assetNames, wantAssets)
-	}
+	assertProductionContains(t, "production prompt assets", assetNames, requiredAssets)

 	catalog := catalogFromRegistries(registries)
 	converted := registriesFromCatalog(catalog)
@@ -156,6 +148,62 @@ func TestProductionPromptAssetsPrepareWithoutProviderCredentials(t *testing.T) {
 	}
 }

+func TestProductionLLMClientFactoriesBuildOfflineRuntime(t *testing.T) {
+	components := productionTestComponents(t)
+	factories := []struct {
+		name    string
+		factory LLMClientFactory
+	}{
+		{name: "default production assets", factory: productionLLMClientFactory},
+		{name: "provided production assets", factory: productionLLMClientFactoryWithAssets(components.assets)},
+	}
+	for _, tt := range factories {
+		t.Run(tt.name, func(t *testing.T) {
+			client, manifests, err := tt.factory(context.Background(), config.Default(), "test-profile")
+			if err != nil {
+				t.Fatalf("build production LLM runtime: %v", err)
+			}
+			if client == nil {
+				t.Fatal("production LLM runtime returned a nil client")
+			}
+			if len(manifests) != 0 {
+				t.Fatalf("eager profile manifests = %#v, want none", manifests)
+			}
+			if _, ok := client.(contracts.LLMProfileManifestProvider); !ok {
+				t.Fatalf("production LLM client %T does not provide profile manifests", client)
+			}
+		})
+	}
+}
+
+func TestProductionLLMClientFactoriesRejectInvalidConstruction(t *testing.T) {
+	t.Run("canceled context", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel()
+		client, manifests, err := productionLLMClientFactory(ctx, config.Default(), "test-profile")
+		if !errors.Is(err, context.Canceled) || client != nil || len(manifests) != 0 {
+			t.Fatalf("client=%T manifests=%#v error=%v, want canceled construction", client, manifests, err)
+		}
+	})
+
+	t.Run("nil assets", func(t *testing.T) {
+		client, manifests, err := productionLLMClientFactoryWithAssets(nil)(context.Background(), config.Default(), "test-profile")
+		if err == nil || !strings.Contains(err.Error(), "asset registry must not be nil") || client != nil || len(manifests) != 0 {
+			t.Fatalf("client=%T manifests=%#v error=%v, want nil-assets failure", client, manifests, err)
+		}
+	})
+
+	t.Run("invalid scheduler concurrency", func(t *testing.T) {
+		components := productionTestComponents(t)
+		cfg := config.Default()
+		cfg.Concurrency.TotalLLM = 0
+		client, manifests, err := productionLLMClientFactoryWithAssets(components.assets)(context.Background(), cfg, "test-profile")
+		if err == nil || !strings.Contains(err.Error(), "create LLM scheduler") || !strings.Contains(err.Error(), "greater than zero") || client != nil || len(manifests) != 0 {
+			t.Fatalf("client=%T manifests=%#v error=%v, want scheduler-construction failure", client, manifests, err)
+		}
+	})
+}
+
 func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T) {
 	base := string(readRepositoryFile(t, "examples", "dnd-spells.config.yml"))
 	validPath := writeProductionContractConfig(t, base)
@@ -180,13 +228,13 @@ func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T)
 	}{
 		{
 			name:      "unknown module",
-			content:   strings.Replace(base, "    input: seriatim\n", "    input: missing/input\n", 1),
+			content:   replaceRequiredOnce(t, base, "    input: seriatim\n", "    input: missing/input\n"),
 			options:   productionCLIOptions(t),
 			fragments: []string{"pipeline \"dnd-session\"", "input", "missing/input"},
 		},
 		{
 			name:      "unknown validator",
-			content:   strings.Replace(base, "        extract: dnd/spells\n", "        extract:\n          module: dnd/spells\n          validators:\n            - module: missing/validator\n", 1),
+			content:   replaceRequiredOnce(t, base, "        extract: dnd/spells\n", "        extract:\n          module: dnd/spells\n          validators:\n            - module: missing/validator\n"),
 			options:   productionCLIOptions(t),
 			fragments: []string{"validator", "missing/validator"},
 		},
@@ -198,7 +246,7 @@ func TestProductionConfigValidationCoversModuleAndVariantFailures(t *testing.T)
 		},
 		{
 			name:      "deterministic validator with profile",
-			content:   strings.Replace(base, "        extract: dnd/spells\n", "        extract:\n          module: dnd/spells\n          validators:\n            - module: generic/valid_json\n              llm_profile: forbidden-profile\n", 1),
+			content:   replaceRequiredOnce(t, base, "        extract: dnd/spells\n", "        extract:\n          module: dnd/spells\n          validators:\n            - module: generic/valid_json\n              llm_profile: forbidden-profile\n"),
 			options:   productionCLIOptions(t),
 			fragments: []string{"deterministic validator", "llm_profile"},
 		},
@@ -383,10 +431,20 @@ func productionAssetNames(t *testing.T, getFS func() (fs.FS, error)) []string {
 	return names
 }

-func assertProductionKeys(t *testing.T, name string, got, want []string) {
+func assertProductionContains[T comparable](t *testing.T, name string, got, required []T) {
 	t.Helper()
-	if !reflect.DeepEqual(got, want) {
-		t.Fatalf("%s = %#v, want %#v", name, got, want)
+	available := make(map[T]struct{}, len(got))
+	for _, entry := range got {
+		available[entry] = struct{}{}
+	}
+	var missing []T
+	for _, entry := range required {
+		if _, ok := available[entry]; !ok {
+			missing = append(missing, entry)
+		}
+	}
+	if len(missing) > 0 {
+		t.Fatalf("%s missing required entries %#v; registered entries are %#v", name, missing, got)
 	}
 }

--- a/internal/cli/run_contract_test.go
+++ b/internal/cli/run_contract_test.go
@@ -104,7 +104,7 @@ func TestRunOnlyExecutesSelectedLanes(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	data = bytes.Replace(data, []byte("    output: test/output\n"), []byte("      other:\n        extract: test/extract\n    output: test/output\n"), 1)
+	data = []byte(replaceRequiredOnce(t, string(data), "    output: test/output\n", "      other:\n        extract: test/extract\n    output: test/output\n"))
 	if err := os.WriteFile(roots.config, data, 0o600); err != nil {
 		t.Fatal(err)
 	}
@@ -170,8 +170,8 @@ func TestRunStateRootsHonorEnvironmentFlagsAndDefaults(t *testing.T) {
 			t.Fatal(err)
 		}
 		text := string(data)
-		text = strings.Replace(text, fmt.Sprintf("  directory: %q\n", roots.output), "", 1)
-		text = strings.Replace(text, fmt.Sprintf("  directory: %q\n", roots.debug), "", 1)
+		text = replaceRequiredOnce(t, text, fmt.Sprintf("  directory: %q\n", roots.output), "")
+		text = replaceRequiredOnce(t, text, fmt.Sprintf("  directory: %q\n", roots.debug), "")
 		if err := os.WriteFile(roots.config, []byte(text), 0o600); err != nil {
 			t.Fatal(err)
 		}
@@ -266,17 +266,23 @@ func TestRunLLMProfileOverrideAndValidationUseInjectedBoundaries(t *testing.T) {

 func TestEffectiveLLMProfileIDsAreSortedDeduplicatedAndLLMOnly(t *testing.T) {
 	resolved := pipeline.ResolvedPipeline{
+		Input: pipeline.ModuleBinding{LLMProfile: "input-profile"},
 		Chunk: pipeline.ModuleBinding{LLMProfile: " zeta "},
 		ArtifactLanes: []pipeline.ResolvedArtifactLane{
-			{Extract: pipeline.ModuleBinding{LLMProfile: "alpha"}, Merge: pipeline.ModuleBinding{LLMProfile: "zeta"}},
+			{
+				Extract:   pipeline.ModuleBinding{LLMProfile: "alpha"},
+				Merge:     pipeline.ModuleBinding{LLMProfile: "zeta"},
+				Normalize: pipeline.ModuleBinding{LLMProfile: " gamma "},
+			},
 		},
 		ValidatorChains: []pipeline.ResolvedValidatorChain{{Validators: []pipeline.ResolvedValidator{
 			{Binding: pipeline.ModuleBinding{LLMProfile: "deterministic-profile"}, ExecutionClass: contracts.ExecutionClassDeterministic},
 			{Binding: pipeline.ModuleBinding{LLMProfile: "beta"}, ExecutionClass: contracts.ExecutionClassLLMBacked},
 		}}},
+		Output: pipeline.ModuleBinding{LLMProfile: "output-profile"},
 	}
 	got := effectiveLLMProfileIDs(resolved)
-	want := []string{"alpha", "beta", "zeta"}
+	want := []string{"alpha", "beta", "gamma", "zeta"}
 	if strings.Join(got, ",") != strings.Join(want, ",") {
 		t.Fatalf("effective profiles = %#v, want %#v", got, want)
 	}
@@ -335,7 +341,7 @@ func TestRunFactoryAndPreparationFailuresAreProcessFailures(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		data = bytes.Replace(data, []byte("extract: test/extract"), []byte("extract: test/failing-extract"), 1)
+		data = []byte(replaceRequiredOnce(t, string(data), "extract: test/extract", "extract: test/failing-extract"))
 		if err := os.WriteFile(roots.config, data, 0o600); err != nil {
 			t.Fatal(err)
 		}
--- a/internal/cli/state_hardening_test.go
+++ b/internal/cli/state_hardening_test.go
@@ -256,7 +256,7 @@ func TestRunRedactsSensitiveModuleOptionsFromConfigAndPipelineSummaries(t *testi
 	if err != nil {
 		t.Fatal(err)
 	}
-	configText := strings.Replace(string(data), "    input: test/input\n", `    input:
+	configText := replaceRequiredOnce(t, string(data), "    input: test/input\n", `    input:
      module: test/input
      options:
        api_key: CONFIG_SUMMARY_SECRET_SENTINEL
@@ -264,7 +264,7 @@ func TestRunRedactsSensitiveModuleOptionsFromConfigAndPipelineSummaries(t *testi
        nested:
          - - password: PIPELINE_SUMMARY_SECRET_SENTINEL
              neighbor: SAFE_NESTED_OPTION_SENTINEL
-`, 1)
+`)
 	if err := os.WriteFile(roots.config, []byte(configText), 0o600); err != nil {
 		t.Fatal(err)
 	}