Fixed Some test cases during a POC for Claude models - #682
Conversation
WalkthroughThis change updates and cleans up test fixtures for Kubernetes troubleshooting scenarios. It deletes numerous static output and log files, modifies test case YAMLs for clarity and specificity, comments out certain monitoring resources, adjusts container and service ports, introduces a new DNS/network policy test case with corresponding manifests and toolset configuration, and updates toolset enablement flags. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LLM Test Harness
participant Kubernetes Cluster
User->>LLM Test Harness: Submit troubleshooting prompt (e.g., DNS issue)
LLM Test Harness->>Kubernetes Cluster: Apply test manifest (namespace, pods, network policy)
LLM Test Harness->>LLM Test Harness: Wait/setup
LLM Test Harness->>LLM: Provide prompt and context (toolsets enabled, logs, etc.)
LLM->>LLM Test Harness: Generate diagnostic output
LLM Test Harness->>Kubernetes Cluster: Teardown (delete manifest)
LLM Test Harness->>User: Return evaluation result
Estimated code review effort3 (~45 minutes) Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/test_case.yaml (2)
1-7: Expected output is again too prescriptive.Prior learnings note that strict wording causes false negatives. Requiring the answer to “clearly state that example-ingress-class does not exist” may fail an otherwise correct LLM reply that phrases it differently (e.g., “no such ingress class”). Consider loosening to “must mention that the specified ingress class is missing”.
- - It should clearly state that example-ingress-class does not exist - saying it just might not exist is not good enough. + - The answer should indicate the specified ingress class (`example-ingress-class`) is missing or not present in the cluster.
11-11: Comment acknowledges test weakness—consider turning into TODO.Instead of an inline remark, add a formal
todo:entry so it surfaces during test maintenance.tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/toolsets.yaml (1)
10-11: Normalize boolean style for consistency
prometheus/metrics.enabledusesFalse(capital F) while the newaws/rds.enabledfollows the lower-case convention (true). Pick one style across the file to avoid noisy diffs and accidental mis-merges.- prometheus/metrics: - enabled: False + prometheus/metrics: + enabled: falsetests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/slow-rds-query-for-medium.yaml (1)
64-103: Large blocks commented out—delete or move to separate fileKeeping >40 commented lines clutters the manifest and invites drift. If these monitoring resources are truly irrelevant to this scenario, remove them or move to
slow-rds-query-for-medium.monitoring.yamlso they can be re-enabled viakustomize/helmoverlays when needed.tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/toolsets.yaml (1)
11-16: Mixed quoting style for scalar values
busyboxis unquoted, whereas the dnsutils image is quoted. Consistently quoting (or not) keeps diffs clean and avoids accidental parsing surprises.tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/manifest.yaml (1)
38-43: Containers run as root with privilege-escalation allowed (CKV_K8S_20 / CKV_K8S_23)Hardening the fixtures costs two lines:
securityContext: - # empty – runs as root, escalation allowed + runAsNonRoot: true + allowPrivilegeEscalation: falseNot critical for a POC, but worth tightening before these files become reference material.
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/test_case.yaml (1)
5-5: Assertion could be less brittleHard-coding
default-deny-egressrisks breakage if the policy name changes. Matching on “an egress-deny NetworkPolicy in test-ns” would keep the test flexible while still verifying the diagnosis.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_describe_deployment_customer-orders-for-medium_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_describe_pod_customer-orders-for-medium-7744d956fb-pv79v_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_find_resource_deployment_customer-orders-for-medium.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_get_yaml_pod_customer-orders-for-medium-7744d956fb-pv79v_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_lineage_children_deployment_customer-orders-for-medium_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_logs_all_containers_customer-orders-for-medium-7744d956fb-pv79v_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_previous_logs_all_containers_customer-orders-for-medium-7744d956fb-pv79v_default.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/slow-rds-query-for-medium.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/toolsets.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/ingress_with_class.yaml(2 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_ingress.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_pod.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_service.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_find_resource.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_cluster.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_cluster_deployment.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace_deployment.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace_service.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_name.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_lineage_children.txt(0 hunks)tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/manifest.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/toolsets.yaml(1 hunks)
🧠 Learnings (4)
tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/ingress_with_class.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
🪛 Checkov (3.2.334)
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/manifest.yaml
[MEDIUM] 17-29: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 17-29: Minimize the admission of root containers
(CKV_K8S_23)
[MEDIUM] 30-45: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 30-45: Minimize the admission of root containers
(CKV_K8S_23)
💤 Files with no reviewable changes (18)
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_cluster_deployment.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_ingress.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_lineage_children.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_service.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_name.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_cluster.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_find_resource.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace_service.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_describe_pod.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_describe_pod_customer-orders-for-medium-7744d956fb-pv79v_default.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_logs_all_containers_customer-orders-for-medium-7744d956fb-pv79v_default.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_lineage_children_deployment_customer-orders-for-medium_default.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_describe_deployment_customer-orders-for-medium_default.txt
- tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/kubectl_get_by_kind_in_namespace_deployment.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_previous_logs_all_containers_customer-orders-for-medium-7744d956fb-pv79v_default.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_find_resource_deployment_customer-orders-for-medium.txt
- tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/kubectl_get_yaml_pod_customer-orders-for-medium-7744d956fb-pv79v_default.txt
🧰 Additional context used
🧠 Learnings (4)
tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/ingress_with_class.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/test_case.yaml (1)
Learnt from: Sheeproid
PR: #586
File: tests/llm/fixtures/test_ask_holmes/03_what_is_the_command_to_port_forward/test_case.yaml:4-4
Timestamp: 2025-07-02T10:27:17.231Z
Learning: In LLM-as-judge test cases for HolmesGPT, expected outputs should be descriptive rather than prescriptive when testing for flexible responses like port numbers. Using specific values in expected outputs can cause unnecessary test failures when the AI generates different but equally valid responses.
🪛 Checkov (3.2.334)
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/manifest.yaml
[MEDIUM] 17-29: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 17-29: Minimize the admission of root containers
(CKV_K8S_23)
[MEDIUM] 30-45: Containers should not run with allowPrivilegeEscalation
(CKV_K8S_20)
[MEDIUM] 30-45: Minimize the admission of root containers
(CKV_K8S_23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (4)
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class/ingress_with_class.yaml (2)
48-49: Service targetPort updated correctly.The service now forwards to
targetPort: 80, keeping the Deployment/Service/Ingress stack consistent.
19-19: No stale 8080 references found – approving changeI ran a recursive search for “8080” in
tests/llm/fixtures/test_ask_holmes/25_misconfigured_ingress_class
and found no matches. The update tocontainerPort: 80now correctly aligns with the service definition. No further changes needed here.tests/llm/fixtures/test_ask_holmes/22_high_latency_dbi_down/test_case.yaml (1)
6-6: Re-evaluate need for the explicit “logs” tagThe new tag is fine, but confirm that any grading logic actually keys off this tag; otherwise it’s dead weight and may confuse future readers.
tests/llm/fixtures/test_ask_holmes/42_dns_issues_result_new_tools_no_runbook/manifest.yaml (1)
25-29: Client image may lack DNS tools and is not in the toolset whitelist
mainred/client:v2is neither guaranteed to containnslookup/dignor present intoolsets.yaml.
If the LLM tries tokubectl execinto this pod, the call will be rejected or the command will be missing—both break the test. Consider switching tobusybox/dnsutilsor add this image to the whitelist.
No description provided.