From 6c7b7c4751a67cbde435e7bc74d455e7a18f53e8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 6 Feb 2026 07:32:08 +0000 Subject: [PATCH] Address code review feedback - Fixed analyzer name in README.md to use consistent 'CustomResource' name - Improved TestCRDAnalyzer_NilClientConfig to be more explicit about expected behavior - Added clear comments about what the test verifies Co-authored-by: AlexsJones <1235925+AlexsJones@users.noreply.github.com> --- README.md | 2 +- pkg/analyzer/crd_test.go | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 62a9337b..a27d8b1d 100644 --- a/README.md +++ b/README.md @@ -279,7 +279,7 @@ you will be able to write your own analyzers. - [x] OperatorGroup - [x] InstallPlan - [x] Subscription -- [x] **customResourceAnalyzer** - Generic analyzer for any CRD (cert-manager, ArgoCD, Kafka, etc.) [Documentation](docs/CRD_ANALYZER.md) +- [x] **CustomResource** - Generic analyzer for any CRD (cert-manager, ArgoCD, Kafka, etc.) [Documentation](docs/CRD_ANALYZER.md) ## Examples diff --git a/pkg/analyzer/crd_test.go b/pkg/analyzer/crd_test.go index 729955de..00d31c0a 100644 --- a/pkg/analyzer/crd_test.go +++ b/pkg/analyzer/crd_test.go @@ -383,24 +383,28 @@ func TestAnalyzeGenericHealth_NoStatusFields(t *testing.T) { } } -// Dummy test to satisfy the requirement +// TestCRDAnalyzer_NilClientConfig tests that the analyzer handles errors gracefully func TestCRDAnalyzer_NilClientConfig(t *testing.T) { viper.Reset() viper.Set("crd_analyzer", map[string]interface{}{ "enabled": true, }) - // Create a client with nil config - this should cause an error when trying to create apiextensions client + // Create a client with a config that will cause an error when trying to create apiextensions client a := common.Analyzer{ Context: context.TODO(), Client: &kubernetes.Client{Config: &rest.Config{}}, } - // This should fail gracefully - _, err := (CRDAnalyzer{}).Analyze(a) - if err == nil { - // Depending on the test setup, this may or may not error - // The important thing is that it doesn't panic - t.Log("Analyzer did not error with empty config - that's okay for this test") + // The analyzer should handle the error gracefully without panicking + results, err := (CRDAnalyzer{}).Analyze(a) + + // We expect either an error or no results, but no panic + if err != nil { + // Error is expected in this case - that's fine + if results != nil { + t.Errorf("Expected nil results when error occurs, got %v", results) + } } + // The important thing is that we didn't panic }