From 9964cdddde1d5fcb99998881413ddd85e96cd1b2 Mon Sep 17 00:00:00 2001 From: Jinghan Fu Date: Thu, 5 Mar 2026 16:00:10 +1100 Subject: [PATCH 1/3] refactor(kclshim): decouple Synthesize from stdin/stdout I/O Accept structured input parameter and return []client.Object instead of reading from stdin and writing to stdout. Add comprehensive tests with fixture-based KCL synthesizer covering deployment and service account assertions, and error handling for bad synthesizer input. --- pkg/kclshim/kcl.go | 67 ++++------ pkg/kclshim/kcl_test.go | 281 ++++++++++++++++++++++------------------ 2 files changed, 181 insertions(+), 167 deletions(-) diff --git a/pkg/kclshim/kcl.go b/pkg/kclshim/kcl.go index 99fc3338..bfee97ac 100644 --- a/pkg/kclshim/kcl.go +++ b/pkg/kclshim/kcl.go @@ -4,57 +4,33 @@ // // 1. Create a folder and write your KCLs. Take "./fixtures/example_synthesizer/main.k" as an example. // -// 2. Write a main.go and call "Synthesize()", defined below. +// 2. Write a main.go and call "Synthesize(workingDir, input)", defined below. package kclshim import ( "encoding/json" "fmt" - "io" - "os" krmv1 "github.com/Azure/eno/pkg/krm/functions/api/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" kcl "kcl-lang.io/kcl-go" "kcl-lang.io/kcl-go/pkg/spec/gpyrpc" + "sigs.k8s.io/controller-runtime/pkg/client" ) -func printErr(err error) { - rl := krmv1.ResourceList{ - APIVersion: krmv1.SchemeGroupVersion.String(), - Kind: krmv1.ResourceListKind, - Items: []*unstructured.Unstructured{}, - Results: []*krmv1.Result{ - { - Message: err.Error(), - Severity: krmv1.ResultSeverityError, - }, - }, - } - bytes, err := json.Marshal(rl) - if err != nil { - rl.Results = append(rl.Results, &krmv1.Result{ - Message: fmt.Sprintf("error marshaling error response: %v", err), - Severity: krmv1.ResultSeverityError, - }) - } - fmt.Print(string(bytes)) -} - -func Synthesize(workingDir string) { - buffer, err := io.ReadAll(os.Stdin) +// Synthesize runs a KCL program in the given directory with JSON-serializable structured input. +// It returns the synthesized Kubernetes objects or an error. +func Synthesize(workingDir string, input any) ([]client.Object, error) { + inputJSON, err := json.Marshal(input) if err != nil { - printErr(fmt.Errorf("error reading from stdin: %w", err)) - return + return nil, fmt.Errorf("error marshaling input to JSON: %w", err) } - input := string(buffer) + inputStr := string(inputJSON) depResult, err := kcl.UpdateDependencies(&gpyrpc.UpdateDependencies_Args{ ManifestPath: workingDir, }) if err != nil { - printErr(fmt.Errorf("error updating dependencies: %w", err)) - return + return nil, fmt.Errorf("error updating dependencies: %w", err) } depOpt := kcl.NewOption() @@ -64,24 +40,31 @@ func Synthesize(workingDir string) { "main.k", kcl.WithWorkDir(workingDir), *depOpt, - kcl.WithOptions(fmt.Sprintf("input=%s", input)), + kcl.WithOptions(fmt.Sprintf("input=%s", inputStr)), ) if err != nil { - printErr(fmt.Errorf("error running KCL: %w", err)) - return + return nil, fmt.Errorf("error running KCL: %w", err) } result, err := results.First().ToMap() + if err != nil { + return nil, fmt.Errorf("error converting KCL result to map: %w", err) + } + output := result["output"] outputJSON, err := json.Marshal(output) if err != nil { - printErr(fmt.Errorf("error marshaling output to JSON: %w", err)) - return + return nil, fmt.Errorf("error marshaling output to JSON: %w", err) } - _, err = fmt.Println(string(outputJSON)) - if err != nil { - printErr(fmt.Errorf("error printing output: %w", err)) - return + var rl krmv1.ResourceList + if err := json.Unmarshal(outputJSON, &rl); err != nil { + return nil, fmt.Errorf("error unmarshaling output to ResourceList: %w", err) + } + + var objects []client.Object + for _, item := range rl.Items { + objects = append(objects, item) } + return objects, nil } diff --git a/pkg/kclshim/kcl_test.go b/pkg/kclshim/kcl_test.go index 3ad5e1b4..d85cafd1 100644 --- a/pkg/kclshim/kcl_test.go +++ b/pkg/kclshim/kcl_test.go @@ -1,138 +1,169 @@ package kclshim import ( - "io" + "encoding/json" "os" "strings" "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func TestSynthesize(t *testing.T) { - tests := []struct { - name string - workingDir string - expectedOutput string - }{ - { - name: "successful synthesis", - workingDir: "fixtures/example_synthesizer", - expectedOutput: `{ - "apiVersion":"config.kubernetes.io/v1", - "items":[ - { - "apiVersion": "apps/v1", - "kind": "Deployment", - "metadata": { - "name": "my-deployment", - "namespace": "default" - }, - "spec": { - "replicas": 3, - "selector": { - "matchLabels": { - "app": "my-app" - } - }, - "template": { - "metadata": { - "labels": { - "app": "my-app" - } - }, - "spec": { - "containers": [ - { - "image": "mcr.microsoft.com/a/b/my-image:latest", - "name": "my-container" - } - ] - } - } - } - }, - { - "apiVersion": "v1", - "kind": "ServiceAccount", - "metadata": { - "name": "my-service-account", - "namespace": "default" - } - } - ], - "kind": "ResourceList" - }`, - }, - { - name: "failed synthesis", - workingDir: "fixtures/bad_example_synthesizer", - expectedOutput: `{ - "apiVersion":"config.kubernetes.io/v1", - "kind":"ResourceList", - "items":[], - "results":[ - { - "message":"error updating dependencies: No such file or directory (os error 2)", - "severity":"error" - } - ] - }`, - }, + // Load test input from fixture file + inputBytes, err := os.ReadFile("fixtures/example_input.json") + if err != nil { + t.Fatalf("Failed to read input file: %v", err) } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - originalStdin := os.Stdin - originalStdout := os.Stdout - defer func() { - os.Stdin = originalStdin - os.Stdout = originalStdout - }() - - input, err := os.Open("fixtures/example_input.json") - if err != nil { - t.Fatalf("Failed to open input file: %v", err) - } - defer input.Close() - - stdin, w, err := os.Pipe() - if err != nil { - t.Fatalf("Failed to create stdin pipe: %v", err) - } - os.Stdin = stdin - - go func() { - defer w.Close() - io.Copy(w, input) - }() - - r, stdout, err := os.Pipe() - if err != nil { - t.Fatalf("Failed to create stdout pipe: %v", err) - } - os.Stdout = stdout - - Synthesize(tt.workingDir) - stdout.Close() - - buf, err := io.ReadAll(r) - if err != nil { - t.Fatalf("Failed to read output: %v", err) - } - output := string(buf) - - normalizedExpected := normalizeWhitespace(tt.expectedOutput) - normalizedOutput := normalizeWhitespace(output) - - if normalizedOutput != normalizedExpected { - t.Errorf("Output mismatch\nExpected:\n%s\nGot:\n%s", normalizedExpected, normalizedOutput) - } - }) + var input map[string]interface{} + if err := json.Unmarshal(inputBytes, &input); err != nil { + t.Fatalf("Failed to unmarshal input JSON: %v", err) } -} -func normalizeWhitespace(s string) string { - for _, whitespace := range []string{"\n", "\t", " "} { - s = strings.ReplaceAll(s, whitespace, "") - } - return s + t.Run("successful synthesis", func(t *testing.T) { + output, err := Synthesize("fixtures/example_synthesizer", input) + if err != nil { + t.Fatalf("Synthesize returned unexpected error: %v", err) + } + if output == nil { + t.Fatalf("Expected non-nil output, got nil") + } + + // Verify the output []client.Object has the expected items + if len(output) != 2 { + t.Fatalf("Expected 2 items, got %d", len(output)) + } + + // Item[0]: Deployment + deployment, ok := output[0].(*unstructured.Unstructured) + if !ok { + t.Fatalf("Expected output[0] to be *unstructured.Unstructured, got %T", output[0]) + } + + // apiVersion + if deployment.GetAPIVersion() != "apps/v1" { + t.Errorf("Deployment: expected apiVersion 'apps/v1', got %q", deployment.GetAPIVersion()) + } + + // kind + if deployment.GetKind() != "Deployment" { + t.Errorf("Deployment: expected kind 'Deployment', got %q", deployment.GetKind()) + } + + // metadata.name + if deployment.GetName() != "my-deployment" { + t.Errorf("Deployment: expected name 'my-deployment', got %q", deployment.GetName()) + } + + // metadata.namespace + if deployment.GetNamespace() != "default" { + t.Errorf("Deployment: expected namespace 'default', got %q", deployment.GetNamespace()) + } + + // spec.replicas + replicas, found, err := unstructured.NestedInt64(deployment.Object, "spec", "replicas") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.replicas: found=%v, err=%v", found, err) + } + if replicas != 3 { + t.Errorf("Deployment: expected spec.replicas 3, got %d", replicas) + } + + // spec.selector.matchLabels.app + selectorApp, found, err := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.selector.matchLabels.app: found=%v, err=%v", found, err) + } + if selectorApp != "my-app" { + t.Errorf("Deployment: expected spec.selector.matchLabels.app 'my-app', got %q", selectorApp) + } + + // spec.template.metadata.labels.app + templateLabelApp, found, err := unstructured.NestedString(deployment.Object, "spec", "template", "metadata", "labels", "app") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.template.metadata.labels.app: found=%v, err=%v", found, err) + } + if templateLabelApp != "my-app" { + t.Errorf("Deployment: expected spec.template.metadata.labels.app 'my-app', got %q", templateLabelApp) + } + + // spec.template.spec.containers + containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.template.spec.containers: found=%v, err=%v", found, err) + } + if len(containers) != 1 { + t.Fatalf("Deployment: expected 1 container, got %d", len(containers)) + } + + // spec.template.spec.containers[0].image + container, ok := containers[0].(map[string]interface{}) + if !ok { + t.Fatalf("Expected containers[0] to be map[string]interface{}, got %T", containers[0]) + } + image, ok := container["image"].(string) + if !ok { + t.Fatal("Deployment: containers[0].image is not a string") + } + if image != "mcr.microsoft.com/a/b/my-image:latest" { + t.Errorf("Deployment: expected containers[0].image 'mcr.microsoft.com/a/b/my-image:latest', got %q", image) + } + + // spec.template.spec.containers[0].name + containerName, ok := container["name"].(string) + if !ok { + t.Fatal("Deployment: containers[0].name is not a string") + } + if containerName != "my-container" { + t.Errorf("Deployment: expected containers[0].name 'my-container', got %q", containerName) + } + + // Item[1]: ServiceAccount + sa, ok := output[1].(*unstructured.Unstructured) + if !ok { + t.Fatalf("Expected output[1] to be *unstructured.Unstructured, got %T", output[1]) + } + + // apiVersion + if sa.GetAPIVersion() != "v1" { + t.Errorf("ServiceAccount: expected apiVersion 'v1', got %q", sa.GetAPIVersion()) + } + + // kind + if sa.GetKind() != "ServiceAccount" { + t.Errorf("ServiceAccount: expected kind 'ServiceAccount', got %q", sa.GetKind()) + } + + // metadata.name + if sa.GetName() != "my-service-account" { + t.Errorf("ServiceAccount: expected name 'my-service-account', got %q", sa.GetName()) + } + + // metadata.namespace + if sa.GetNamespace() != "default" { + t.Errorf("ServiceAccount: expected namespace 'default', got %q", sa.GetNamespace()) + } + }) + + t.Run("failed synthesis", func(t *testing.T) { + output, err := Synthesize("fixtures/bad_example_synthesizer", input) + + // Verify error is returned + if err == nil { + t.Fatal("Expected error from bad synthesizer, got nil") + } + + // Verify output is nil (no items produced) + if output != nil { + t.Errorf("Expected nil output on error, got %d items", len(output)) + } + + // Verify error message + if !strings.Contains(err.Error(), "error updating dependencies") { + t.Errorf("Expected error containing 'error updating dependencies', got: %v", err) + } + if !strings.Contains(err.Error(), "No such file or directory") { + t.Errorf("Expected error containing 'No such file or directory', got: %v", err) + } + }) } From 13d2eb84782bd9aac0bf04e957053719c2d437ab Mon Sep 17 00:00:00 2001 From: Jinghan Fu Date: Fri, 6 Mar 2026 12:05:40 +1100 Subject: [PATCH 2/3] refactor(kclshim): use table-driven tests and simplify KCL output schema - Refactor kcl_test.go to use table-driven tests with simple if/else assertions - Remove unused Result and ResultResourceRef schemas from resource_list.k - Change KCL output type from ResourceList to object list ([any]) --- .../example_synthesizer/eno/resource_list.k | 13 - .../fixtures/example_synthesizer/main.k | 2 +- .../example_synthesizer/pkg/synthesizer.k | 14 +- pkg/kclshim/kcl.go | 10 +- pkg/kclshim/kcl_test.go | 311 ++++++++++-------- 5 files changed, 177 insertions(+), 173 deletions(-) diff --git a/pkg/kclshim/fixtures/example_synthesizer/eno/resource_list.k b/pkg/kclshim/fixtures/example_synthesizer/eno/resource_list.k index e2515939..5095fa41 100644 --- a/pkg/kclshim/fixtures/example_synthesizer/eno/resource_list.k +++ b/pkg/kclshim/fixtures/example_synthesizer/eno/resource_list.k @@ -1,20 +1,7 @@ # TODO: Move this file to a KCL module. -schema ResultResourceRef: - apiVersion: str - kind: str - name: str - namespace?: str - -schema Result: - message: str - resourceRef?: ResultResourceRef - severity?: str - tag?: [str]str - schema ResourceList: apiVersion: str = "config.kubernetes.io/v1" kind: str = "ResourceList" items: [any] functionConfig?: any - results?: [Result] \ No newline at end of file diff --git a/pkg/kclshim/fixtures/example_synthesizer/main.k b/pkg/kclshim/fixtures/example_synthesizer/main.k index cd56d6c8..8477d7c9 100644 --- a/pkg/kclshim/fixtures/example_synthesizer/main.k +++ b/pkg/kclshim/fixtures/example_synthesizer/main.k @@ -2,4 +2,4 @@ import eno import pkg input: eno.ResourceList = option("input") -output: eno.ResourceList = pkg.Synthesize(input) +output: [any] = pkg.Synthesize(input) diff --git a/pkg/kclshim/fixtures/example_synthesizer/pkg/synthesizer.k b/pkg/kclshim/fixtures/example_synthesizer/pkg/synthesizer.k index 88641460..b05b87bd 100644 --- a/pkg/kclshim/fixtures/example_synthesizer/pkg/synthesizer.k +++ b/pkg/kclshim/fixtures/example_synthesizer/pkg/synthesizer.k @@ -1,12 +1,10 @@ import eno -Synthesize: (eno.ResourceList) -> eno.ResourceList = lambda input: eno.ResourceList -> eno.ResourceList { +Synthesize: (eno.ResourceList) -> [any] = lambda input: eno.ResourceList -> [any] { image_base = [item.data.image_base for item in input.items if item.metadata.name == "some-config"][0] - output = eno.ResourceList { - items: [ - GetDeployment(image_base), - my_service_account, - ] - } -} \ No newline at end of file + output = [ + GetDeployment(image_base), + my_service_account, + ] +} diff --git a/pkg/kclshim/kcl.go b/pkg/kclshim/kcl.go index bfee97ac..d2544574 100644 --- a/pkg/kclshim/kcl.go +++ b/pkg/kclshim/kcl.go @@ -11,7 +11,7 @@ import ( "encoding/json" "fmt" - krmv1 "github.com/Azure/eno/pkg/krm/functions/api/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" kcl "kcl-lang.io/kcl-go" "kcl-lang.io/kcl-go/pkg/spec/gpyrpc" "sigs.k8s.io/controller-runtime/pkg/client" @@ -57,13 +57,13 @@ func Synthesize(workingDir string, input any) ([]client.Object, error) { return nil, fmt.Errorf("error marshaling output to JSON: %w", err) } - var rl krmv1.ResourceList - if err := json.Unmarshal(outputJSON, &rl); err != nil { - return nil, fmt.Errorf("error unmarshaling output to ResourceList: %w", err) + var items []*unstructured.Unstructured + if err := json.Unmarshal(outputJSON, &items); err != nil { + return nil, fmt.Errorf("error unmarshaling output to object list: %w", err) } var objects []client.Object - for _, item := range rl.Items { + for _, item := range items { objects = append(objects, item) } return objects, nil diff --git a/pkg/kclshim/kcl_test.go b/pkg/kclshim/kcl_test.go index d85cafd1..631ef473 100644 --- a/pkg/kclshim/kcl_test.go +++ b/pkg/kclshim/kcl_test.go @@ -20,150 +20,169 @@ func TestSynthesize(t *testing.T) { t.Fatalf("Failed to unmarshal input JSON: %v", err) } - t.Run("successful synthesis", func(t *testing.T) { - output, err := Synthesize("fixtures/example_synthesizer", input) - if err != nil { - t.Fatalf("Synthesize returned unexpected error: %v", err) - } - if output == nil { - t.Fatalf("Expected non-nil output, got nil") - } - - // Verify the output []client.Object has the expected items - if len(output) != 2 { - t.Fatalf("Expected 2 items, got %d", len(output)) - } - - // Item[0]: Deployment - deployment, ok := output[0].(*unstructured.Unstructured) - if !ok { - t.Fatalf("Expected output[0] to be *unstructured.Unstructured, got %T", output[0]) - } - - // apiVersion - if deployment.GetAPIVersion() != "apps/v1" { - t.Errorf("Deployment: expected apiVersion 'apps/v1', got %q", deployment.GetAPIVersion()) - } - - // kind - if deployment.GetKind() != "Deployment" { - t.Errorf("Deployment: expected kind 'Deployment', got %q", deployment.GetKind()) - } - - // metadata.name - if deployment.GetName() != "my-deployment" { - t.Errorf("Deployment: expected name 'my-deployment', got %q", deployment.GetName()) - } - - // metadata.namespace - if deployment.GetNamespace() != "default" { - t.Errorf("Deployment: expected namespace 'default', got %q", deployment.GetNamespace()) - } - - // spec.replicas - replicas, found, err := unstructured.NestedInt64(deployment.Object, "spec", "replicas") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.replicas: found=%v, err=%v", found, err) - } - if replicas != 3 { - t.Errorf("Deployment: expected spec.replicas 3, got %d", replicas) - } - - // spec.selector.matchLabels.app - selectorApp, found, err := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.selector.matchLabels.app: found=%v, err=%v", found, err) - } - if selectorApp != "my-app" { - t.Errorf("Deployment: expected spec.selector.matchLabels.app 'my-app', got %q", selectorApp) - } - - // spec.template.metadata.labels.app - templateLabelApp, found, err := unstructured.NestedString(deployment.Object, "spec", "template", "metadata", "labels", "app") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.template.metadata.labels.app: found=%v, err=%v", found, err) - } - if templateLabelApp != "my-app" { - t.Errorf("Deployment: expected spec.template.metadata.labels.app 'my-app', got %q", templateLabelApp) - } - - // spec.template.spec.containers - containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.template.spec.containers: found=%v, err=%v", found, err) - } - if len(containers) != 1 { - t.Fatalf("Deployment: expected 1 container, got %d", len(containers)) - } - - // spec.template.spec.containers[0].image - container, ok := containers[0].(map[string]interface{}) - if !ok { - t.Fatalf("Expected containers[0] to be map[string]interface{}, got %T", containers[0]) - } - image, ok := container["image"].(string) - if !ok { - t.Fatal("Deployment: containers[0].image is not a string") - } - if image != "mcr.microsoft.com/a/b/my-image:latest" { - t.Errorf("Deployment: expected containers[0].image 'mcr.microsoft.com/a/b/my-image:latest', got %q", image) - } - - // spec.template.spec.containers[0].name - containerName, ok := container["name"].(string) - if !ok { - t.Fatal("Deployment: containers[0].name is not a string") - } - if containerName != "my-container" { - t.Errorf("Deployment: expected containers[0].name 'my-container', got %q", containerName) - } - - // Item[1]: ServiceAccount - sa, ok := output[1].(*unstructured.Unstructured) - if !ok { - t.Fatalf("Expected output[1] to be *unstructured.Unstructured, got %T", output[1]) - } - - // apiVersion - if sa.GetAPIVersion() != "v1" { - t.Errorf("ServiceAccount: expected apiVersion 'v1', got %q", sa.GetAPIVersion()) - } - - // kind - if sa.GetKind() != "ServiceAccount" { - t.Errorf("ServiceAccount: expected kind 'ServiceAccount', got %q", sa.GetKind()) - } - - // metadata.name - if sa.GetName() != "my-service-account" { - t.Errorf("ServiceAccount: expected name 'my-service-account', got %q", sa.GetName()) - } - - // metadata.namespace - if sa.GetNamespace() != "default" { - t.Errorf("ServiceAccount: expected namespace 'default', got %q", sa.GetNamespace()) - } - }) - - t.Run("failed synthesis", func(t *testing.T) { - output, err := Synthesize("fixtures/bad_example_synthesizer", input) - - // Verify error is returned - if err == nil { - t.Fatal("Expected error from bad synthesizer, got nil") - } - - // Verify output is nil (no items produced) - if output != nil { - t.Errorf("Expected nil output on error, got %d items", len(output)) - } - - // Verify error message - if !strings.Contains(err.Error(), "error updating dependencies") { - t.Errorf("Expected error containing 'error updating dependencies', got: %v", err) - } - if !strings.Contains(err.Error(), "No such file or directory") { - t.Errorf("Expected error containing 'No such file or directory', got: %v", err) - } - }) + tests := []struct { + name string + workingDir string + expectErr bool + errContains []string + expectCount int + }{ + { + name: "successful synthesis", + workingDir: "fixtures/example_synthesizer", + expectErr: false, + expectCount: 2, + }, + { + name: "failed synthesis", + workingDir: "fixtures/bad_example_synthesizer", + expectErr: true, + errContains: []string{"error updating dependencies", "No such file or directory"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + output, err := Synthesize(tt.workingDir, input) + + // Failure path + if tt.expectErr { + if err == nil { + t.Fatal("Expected error, got nil") + } + for _, substr := range tt.errContains { + if !strings.Contains(err.Error(), substr) { + t.Errorf("Expected error containing %q, got: %v", substr, err) + } + } + if output != nil { + t.Errorf("Expected nil output on error, got %d items", len(output)) + } + return + } + + // Success path + if err != nil { + t.Fatalf("Synthesize returned unexpected error: %v", err) + } + if output == nil { + t.Fatalf("Expected non-nil output, got nil") + } + + // Verify output item count + if len(output) != tt.expectCount { + t.Fatalf("Expected %d items, got %d", tt.expectCount, len(output)) + } + + // Item[0]: Deployment + deployment, ok := output[0].(*unstructured.Unstructured) + if !ok { + t.Fatalf("Expected output[0] to be *unstructured.Unstructured, got %T", output[0]) + } + + // apiVersion + if deployment.GetAPIVersion() != "apps/v1" { + t.Errorf("Deployment: expected apiVersion 'apps/v1', got %q", deployment.GetAPIVersion()) + } + + // kind + if deployment.GetKind() != "Deployment" { + t.Errorf("Deployment: expected kind 'Deployment', got %q", deployment.GetKind()) + } + + // metadata.name + if deployment.GetName() != "my-deployment" { + t.Errorf("Deployment: expected name 'my-deployment', got %q", deployment.GetName()) + } + + // metadata.namespace + if deployment.GetNamespace() != "default" { + t.Errorf("Deployment: expected namespace 'default', got %q", deployment.GetNamespace()) + } + + // spec.replicas + replicas, found, err := unstructured.NestedInt64(deployment.Object, "spec", "replicas") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.replicas: found=%v, err=%v", found, err) + } + if replicas != 3 { + t.Errorf("Deployment: expected spec.replicas 3, got %d", replicas) + } + + // spec.selector.matchLabels.app + selectorApp, found, err := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.selector.matchLabels.app: found=%v, err=%v", found, err) + } + if selectorApp != "my-app" { + t.Errorf("Deployment: expected spec.selector.matchLabels.app 'my-app', got %q", selectorApp) + } + + // spec.template.metadata.labels.app + templateLabelApp, found, err := unstructured.NestedString(deployment.Object, "spec", "template", "metadata", "labels", "app") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.template.metadata.labels.app: found=%v, err=%v", found, err) + } + if templateLabelApp != "my-app" { + t.Errorf("Deployment: expected spec.template.metadata.labels.app 'my-app', got %q", templateLabelApp) + } + + // spec.template.spec.containers + containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") + if err != nil || !found { + t.Fatalf("Deployment: failed to find spec.template.spec.containers: found=%v, err=%v", found, err) + } + if len(containers) != 1 { + t.Fatalf("Deployment: expected 1 container, got %d", len(containers)) + } + + // spec.template.spec.containers[0].image + container, ok := containers[0].(map[string]interface{}) + if !ok { + t.Fatalf("Deployment: expected containers[0] to be map[string]interface{}, got %T", containers[0]) + } + image, ok := container["image"].(string) + if !ok { + t.Fatal("Deployment: containers[0].image is not a string") + } + if image != "mcr.microsoft.com/a/b/my-image:latest" { + t.Errorf("Deployment: expected containers[0].image 'mcr.microsoft.com/a/b/my-image:latest', got %q", image) + } + + // spec.template.spec.containers[0].name + containerName, ok := container["name"].(string) + if !ok { + t.Fatal("Deployment: containers[0].name is not a string") + } + if containerName != "my-container" { + t.Errorf("Deployment: expected containers[0].name 'my-container', got %q", containerName) + } + + // Item[1]: ServiceAccount + sa, ok := output[1].(*unstructured.Unstructured) + if !ok { + t.Fatalf("Expected output[1] to be *unstructured.Unstructured, got %T", output[1]) + } + + // apiVersion + if sa.GetAPIVersion() != "v1" { + t.Errorf("ServiceAccount: expected apiVersion 'v1', got %q", sa.GetAPIVersion()) + } + + // kind + if sa.GetKind() != "ServiceAccount" { + t.Errorf("ServiceAccount: expected kind 'ServiceAccount', got %q", sa.GetKind()) + } + + // metadata.name + if sa.GetName() != "my-service-account" { + t.Errorf("ServiceAccount: expected name 'my-service-account', got %q", sa.GetName()) + } + + // metadata.namespace + if sa.GetNamespace() != "default" { + t.Errorf("ServiceAccount: expected namespace 'default', got %q", sa.GetNamespace()) + } + }) + } } From a52896ede1c21bd7a1b1efa24f7b298c4a812060 Mon Sep 17 00:00:00 2001 From: Jinghan Fu Date: Fri, 6 Mar 2026 16:22:58 +1100 Subject: [PATCH 3/3] refactor(kclshim): simplify test to use table-driven JSON comparison Replace field-by-field assertions with JSON string comparison using normalizeWhitespace helper. Consolidate expectErr/errContains into a single expectedErrs field and add inline expectedOutput for success cases. Remove unused unstructured import. --- pkg/kclshim/kcl_test.go | 195 +++++++++++++--------------------------- 1 file changed, 64 insertions(+), 131 deletions(-) diff --git a/pkg/kclshim/kcl_test.go b/pkg/kclshim/kcl_test.go index 631ef473..7b155397 100644 --- a/pkg/kclshim/kcl_test.go +++ b/pkg/kclshim/kcl_test.go @@ -5,8 +5,6 @@ import ( "os" "strings" "testing" - - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) func TestSynthesize(t *testing.T) { @@ -21,23 +19,60 @@ func TestSynthesize(t *testing.T) { } tests := []struct { - name string - workingDir string - expectErr bool - errContains []string - expectCount int + name string + workingDir string + expectedErrs []string + expectedOutput string }{ { - name: "successful synthesis", - workingDir: "fixtures/example_synthesizer", - expectErr: false, - expectCount: 2, + name: "successful synthesis", + workingDir: "fixtures/example_synthesizer", + expectedOutput: `[ + { + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": "my-deployment", + "namespace": "default" + }, + "spec": { + "replicas": 3, + "selector": { + "matchLabels": { + "app": "my-app" + } + }, + "template": { + "metadata": { + "labels": { + "app": "my-app" + } + }, + "spec": { + "containers": [ + { + "image": "mcr.microsoft.com/a/b/my-image:latest", + "name": "my-container" + } + ] + } + } + } + }, + { + "apiVersion": "v1", + "kind": "ServiceAccount", + "metadata": { + "name": "my-service-account", + "namespace": "default" + } + } + ]`, }, { - name: "failed synthesis", - workingDir: "fixtures/bad_example_synthesizer", - expectErr: true, - errContains: []string{"error updating dependencies", "No such file or directory"}, + name: "failed synthesis", + workingDir: "fixtures/bad_example_synthesizer", + expectedErrs: []string{"error updating dependencies", "No such file or directory"}, }, } @@ -46,11 +81,11 @@ func TestSynthesize(t *testing.T) { output, err := Synthesize(tt.workingDir, input) // Failure path - if tt.expectErr { + if len(tt.expectedErrs) > 0 { if err == nil { t.Fatal("Expected error, got nil") } - for _, substr := range tt.errContains { + for _, substr := range tt.expectedErrs { if !strings.Contains(err.Error(), substr) { t.Errorf("Expected error containing %q, got: %v", substr, err) } @@ -65,124 +100,22 @@ func TestSynthesize(t *testing.T) { if err != nil { t.Fatalf("Synthesize returned unexpected error: %v", err) } - if output == nil { - t.Fatalf("Expected non-nil output, got nil") - } - - // Verify output item count - if len(output) != tt.expectCount { - t.Fatalf("Expected %d items, got %d", tt.expectCount, len(output)) - } - - // Item[0]: Deployment - deployment, ok := output[0].(*unstructured.Unstructured) - if !ok { - t.Fatalf("Expected output[0] to be *unstructured.Unstructured, got %T", output[0]) - } - - // apiVersion - if deployment.GetAPIVersion() != "apps/v1" { - t.Errorf("Deployment: expected apiVersion 'apps/v1', got %q", deployment.GetAPIVersion()) - } - - // kind - if deployment.GetKind() != "Deployment" { - t.Errorf("Deployment: expected kind 'Deployment', got %q", deployment.GetKind()) - } - - // metadata.name - if deployment.GetName() != "my-deployment" { - t.Errorf("Deployment: expected name 'my-deployment', got %q", deployment.GetName()) - } - - // metadata.namespace - if deployment.GetNamespace() != "default" { - t.Errorf("Deployment: expected namespace 'default', got %q", deployment.GetNamespace()) - } - - // spec.replicas - replicas, found, err := unstructured.NestedInt64(deployment.Object, "spec", "replicas") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.replicas: found=%v, err=%v", found, err) - } - if replicas != 3 { - t.Errorf("Deployment: expected spec.replicas 3, got %d", replicas) - } - - // spec.selector.matchLabels.app - selectorApp, found, err := unstructured.NestedString(deployment.Object, "spec", "selector", "matchLabels", "app") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.selector.matchLabels.app: found=%v, err=%v", found, err) - } - if selectorApp != "my-app" { - t.Errorf("Deployment: expected spec.selector.matchLabels.app 'my-app', got %q", selectorApp) - } - // spec.template.metadata.labels.app - templateLabelApp, found, err := unstructured.NestedString(deployment.Object, "spec", "template", "metadata", "labels", "app") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.template.metadata.labels.app: found=%v, err=%v", found, err) - } - if templateLabelApp != "my-app" { - t.Errorf("Deployment: expected spec.template.metadata.labels.app 'my-app', got %q", templateLabelApp) - } - - // spec.template.spec.containers - containers, found, err := unstructured.NestedSlice(deployment.Object, "spec", "template", "spec", "containers") - if err != nil || !found { - t.Fatalf("Deployment: failed to find spec.template.spec.containers: found=%v, err=%v", found, err) - } - if len(containers) != 1 { - t.Fatalf("Deployment: expected 1 container, got %d", len(containers)) - } - - // spec.template.spec.containers[0].image - container, ok := containers[0].(map[string]interface{}) - if !ok { - t.Fatalf("Deployment: expected containers[0] to be map[string]interface{}, got %T", containers[0]) - } - image, ok := container["image"].(string) - if !ok { - t.Fatal("Deployment: containers[0].image is not a string") - } - if image != "mcr.microsoft.com/a/b/my-image:latest" { - t.Errorf("Deployment: expected containers[0].image 'mcr.microsoft.com/a/b/my-image:latest', got %q", image) - } - - // spec.template.spec.containers[0].name - containerName, ok := container["name"].(string) - if !ok { - t.Fatal("Deployment: containers[0].name is not a string") - } - if containerName != "my-container" { - t.Errorf("Deployment: expected containers[0].name 'my-container', got %q", containerName) - } - - // Item[1]: ServiceAccount - sa, ok := output[1].(*unstructured.Unstructured) - if !ok { - t.Fatalf("Expected output[1] to be *unstructured.Unstructured, got %T", output[1]) - } - - // apiVersion - if sa.GetAPIVersion() != "v1" { - t.Errorf("ServiceAccount: expected apiVersion 'v1', got %q", sa.GetAPIVersion()) - } - - // kind - if sa.GetKind() != "ServiceAccount" { - t.Errorf("ServiceAccount: expected kind 'ServiceAccount', got %q", sa.GetKind()) - } - - // metadata.name - if sa.GetName() != "my-service-account" { - t.Errorf("ServiceAccount: expected name 'my-service-account', got %q", sa.GetName()) + outputJSON, err := json.Marshal(output) + if err != nil { + t.Fatalf("Failed to marshal output to JSON: %v", err) } - // metadata.namespace - if sa.GetNamespace() != "default" { - t.Errorf("ServiceAccount: expected namespace 'default', got %q", sa.GetNamespace()) + if normalizeWhitespace(string(outputJSON)) != normalizeWhitespace(tt.expectedOutput) { + t.Errorf("Output mismatch.\nExpected:\n%s\nGot:\n%s", tt.expectedOutput, string(outputJSON)) } }) } } + +func normalizeWhitespace(s string) string { + for _, whitespace := range []string{"\n", "\t", " "} { + s = strings.ReplaceAll(s, whitespace, "") + } + return s +}