fix: emit render errors on failure

cmd/diff/diffprocessor/diff_processor.go

@@ -160,6 +160,12 @@ func (p *DefaultDiffProcessor) PerformDiff(ctx context.Context, stdout io.Writer
 		if err != nil {
 			p.config.Logger.Debug("Failed to process resource", "resource", resourceID, "error", err)
 			errs = append(errs, errors.Wrapf(err, "unable to process resource %s", resourceID))
+
+			// Write error message to stdout so user can see it
+			errMsg := fmt.Sprintf("ERROR: Failed to process %s: %v\n\n", resourceID, err)
+			if _, writeErr := fmt.Fprint(stdout, errMsg); writeErr != nil {
+				p.config.Logger.Debug("Failed to write error message", "error", writeErr)
+			}
 		} else {
 			// Merge the diffs into our combined map
 			for k, v := range diffs {

cmd/diff/diffprocessor/diff_processor_test.go

@@ -149,7 +149,18 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) {
 			},
 			resources:     []*un.Unstructured{resource1},
 			processorOpts: testProcessorOptions(t),
-			want:          errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"),
+			verifyOutput: func(t *testing.T, output string) {
+				t.Helper()
+				// Verify that the error message was written to stdout
+				if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-1") {
+					t.Errorf("Expected stdout to contain error message, got: %s", output)
+				}
+				// Also verify it contains the composition not found error
+				if !strings.Contains(output, "composition not found") {
+					t.Errorf("Expected stdout to contain 'composition not found' error detail, got: %s", output)
+				}
+			},
+			want: errors.New("unable to process resource XR1/my-xr-1: cannot get composition: composition not found"),
 		},
 		"MultipleResourceErrors": {
 			setupMocks: func() (k8.Clients, xp.Clients) {
@@ -178,6 +189,22 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) {
 			},
 			resources:     []*un.Unstructured{resource1, resource2},
 			processorOpts: testProcessorOptions(t),
+			verifyOutput: func(t *testing.T, output string) {
+				t.Helper()
+				// Verify that error messages for both resources were written to stdout
+				if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-1") {
+					t.Errorf("Expected stdout to contain error message for my-xr-1, got: %s", output)
+				}
+
+				if !strings.Contains(output, "ERROR: Failed to process XR1/my-xr-2") {
+					t.Errorf("Expected stdout to contain error message for my-xr-2, got: %s", output)
+				}
+				// Both should contain the composition not found error
+				expectedCount := strings.Count(output, "composition not found")
+				if expectedCount < 2 {
+					t.Errorf("Expected stdout to contain 'composition not found' at least twice, found %d times in: %s", expectedCount, output)
+				}
+			},
 			want: errors.New("[unable to process resource XR1/my-xr-1: cannot get composition: composition not found, " +
 				"unable to process resource XR1/my-xr-2: cannot get composition: composition not found]"),
 		},
@@ -565,6 +592,11 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) {
 			}
 			err := processor.PerformDiff(ctx, &stdout, tt.resources, compositionProvider)
 
+			// Check output if verification function is provided (do this first, before error checks)
+			if tt.verifyOutput != nil {
+				tt.verifyOutput(t, stdout.String())
+			}
+
 			if tt.want != nil {
 				if err == nil {
 					t.Errorf("PerformDiff(...): expected error but got none")
@@ -581,11 +613,6 @@ func TestDefaultDiffProcessor_PerformDiff(t *testing.T) {
 			if err != nil {
 				t.Errorf("PerformDiff(...): unexpected error: %v", err)
 			}
-
-			// Check output if verification function is provided
-			if tt.verifyOutput != nil {
-				tt.verifyOutput(t, stdout.String())
-			}
 		})
 	}
 }

test/e2e/diff_test.go

@@ -531,6 +531,7 @@ var clusterNopList = composed.NewList(composed.FromReferenceToList(corev1.Object
 // Regular expressions to match the dynamic parts.
 var (
 	resourceNameRegex              = regexp.MustCompile(`(existing-resource)-[a-z0-9]{5,}(?:-nop-resource)?`)
+	compResourceNameRegex          = regexp.MustCompile(`(test-comp-resource)-[a-z0-9]{5,}`)
 	claimNameRegex                 = regexp.MustCompile(`(test-claim)-[a-z0-9]{5,}(?:-[a-z0-9]{5,})?`)
 	claimCompositionRevisionRegex  = regexp.MustCompile(`(xnopclaims\.claim\.diff\.example\.org)-[a-z0-9]{7,}`)
 	compositionRevisionRegex       = regexp.MustCompile(`(xnopresources\.(cluster\.|legacy\.)?diff\.example\.org)-[a-z0-9]{7,}`)
@@ -545,6 +546,7 @@ func normalizeLine(line string) string {
 
 	// Replace resource names with random suffixes
 	line = resourceNameRegex.ReplaceAllString(line, "${1}-XXXXX")
+	line = compResourceNameRegex.ReplaceAllString(line, "${1}-XXXXX")
 	line = claimNameRegex.ReplaceAllString(line, "${1}-XXXXX")
 
 	// Replace composition revision refs with random hash
@@ -689,6 +691,7 @@ func TestDiffExistingComposition(t *testing.T) {
 	imageTag := strings.Split(environment.GetCrossplaneImage(), ":")[1]
 	manifests := filepath.Join("test/e2e/manifests/beta/diff", imageTag, "comp")
 	setupPath := filepath.Join(manifests, "setup")
+	expectPath := filepath.Join(manifests, "expect")
 
 	environment.Test(t,
 		features.New("DiffExistingComposition").
@@ -717,23 +720,7 @@ func TestDiffExistingComposition(t *testing.T) {
 					t.Fatalf("Error running comp diff command: %v\nLog output:\n%s", err, log)
 				}
 
-				// Basic validation - ensure we get meaningful output
-				if output == "" {
-					t.Fatalf("Expected non-empty output from comp diff command")
-				}
-
-				// Check that output contains references to our test resource
-				if !strings.Contains(output, "test-comp-resource") {
-					t.Errorf("Expected output to contain reference to test-comp-resource, got: %s", output)
-				}
-
-				// Check that output shows the expected changes
-				if !strings.Contains(output, "resource-tier") || !strings.Contains(output, "config-data") {
-					t.Logf("Output: %s", output)
-					t.Errorf("Expected output to show changes to resource-tier and config-data annotations")
-				}
-
-				t.Logf("Comp diff output:\n%s", output)
+				assertDiffMatchesFile(t, output, filepath.Join(expectPath, "existing-xr.ansi"), log)
 
 				return ctx
 			}).

test/e2e/manifests/beta/diff/main/comp/expect/existing-xr.ansi

@@ -0,0 +1,94 @@
+=== Composition Changes ===
+
+~~~ Composition/xcompdiffresources.compdiff.example.org
+  apiVersion: apiextensions.crossplane.io/v1
+  kind: Composition
+  metadata:
+    name: xcompdiffresources.compdiff.example.org
+  spec:
+    compositeTypeRef:
+      apiVersion: compdiff.example.org/v1alpha1
+      kind: XCompDiffResource
+    mode: Pipeline
+    pipeline:
+    - functionRef:
+        name: function-patch-and-transform
+      input:
+        apiVersion: pt.fn.crossplane.io/v1beta1
+        kind: Resources
+        resources:
+        - base:
+            apiVersion: nop.crossplane.io/v1alpha1
+            kind: ClusterNopResource
+            spec:
+              forProvider:
+                conditionAfter:
+                - conditionStatus: "True"
+                  conditionType: Ready
+                  time: 0s
+          name: nop-resource
+          patches:
+          - fromFieldPath: spec.coolField
+            toFieldPath: metadata.annotations[config-data]
++           transforms:
++           - string:
++               fmt: updated-%s
++               type: Format
++             type: string
+            type: FromCompositeFieldPath
+          - fromFieldPath: spec.coolField
+            toFieldPath: metadata.annotations[resource-tier]
+            transforms:
+            - string:
+-               fmt: basic-%s
++               fmt: premium-%s
+                type: Format
+              type: string
+            type: FromCompositeFieldPath
+      step: generate-resources
+    - functionRef:
+        name: function-auto-ready
+      step: detect-ready-resources
+
+---
+
+Summary: 1 modified
+
+=== Affected Composite Resources ===
+
+- XCompDiffResource/test-comp-resource (namespace: )
+
+=== Impact Analysis ===
+
+~~~ ClusterNopResource/test-comp-resource-XXXXX
+  apiVersion: nop.crossplane.io/v1alpha1
+  kind: ClusterNopResource
+  metadata:
+    annotations:
+-     config-data: existing-value
++     config-data: updated-existing-value
+      crossplane.io/composition-resource-name: nop-resource
+      crossplane.io/external-name: test-comp-resource-XXXXX
+-     resource-tier: basic-existing-value
++     resource-tier: premium-existing-value
+    finalizers:
+    - finalizer.managedresource.crossplane.io
+    generateName: test-comp-resource-
+    labels:
+      crossplane.io/composite: test-comp-resource
+    name: test-comp-resource-XXXXX
+  spec:
+    deletionPolicy: Delete
+    forProvider:
+      conditionAfter:
+      - conditionStatus: "True"
+        conditionType: Ready
+        time: 0s
+    managementPolicies:
+    - '*'
+    providerConfigRef:
+      name: default
+
+---
+
+Summary: 1 modified

test/e2e/manifests/beta/diff/main/comp/setup/composition.yaml

@@ -36,7 +36,7 @@ spec:
                   - type: string
                     string:
                       type: Format
-                      fmt: "basic"
+                      fmt: "basic-%s"
     - step: detect-ready-resources
       functionRef:
         name: function-auto-ready

test/e2e/manifests/beta/diff/main/comp/updated-composition.yaml

@@ -41,7 +41,7 @@ spec:
                   - type: string
                     string:
                       type: Format
-                      fmt: "premium"
+                      fmt: "premium-%s"
     - step: detect-ready-resources
       functionRef:
         name: function-auto-ready