From 41eba80bebfb9a47db39b969aba17df0550f3662 Mon Sep 17 00:00:00 2001 From: Shreyansh Sancheti <007ssancheti@gmail.com> Date: Thu, 22 Oct 2026 14:54:35 +0530 Subject: [PATCH] test: trim redundant cases and align triage Remove 15 duplicate or tautological tests. Preserve keeper assertions and boundary coverage. Keep native issue labels valid and document scope-based triage. Test implementation and review: Claude Opus 5.5. Signed-off-by: Shreyansh Sancheti <007ssancheti@gmail.com> --- .github/ISSUE_TEMPLATE/oss_feature.yml | 1 - .github/PULL_REQUEST_TEMPLATE.md | 2 +- CONTRIBUTING.md | 10 +- agents/tests/test_state.py | 108 ------------------ agents/tests/test_workflow.py | 47 -------- .../agentworkload_compatibility_test.go | 14 --- api/v1alpha1/agentworkload_webhook_test.go | 80 ------------- .../agentworkload_controller_argo_test.go | 108 +----------------- .../agentworkload_controller_quota_test.go | 55 --------- pkg/runtime/adapter_test.go | 12 -- 10 files changed, 12 insertions(+), 425 deletions(-) delete mode 100644 agents/tests/test_state.py diff --git a/.github/ISSUE_TEMPLATE/oss_feature.yml b/.github/ISSUE_TEMPLATE/oss_feature.yml index c61d634..94e29e2 100644 --- a/.github/ISSUE_TEMPLATE/oss_feature.yml +++ b/.github/ISSUE_TEMPLATE/oss_feature.yml @@ -3,7 +3,6 @@ description: Request an open-source feature for agentic-operator-core. title: "[OSS] " labels: - enhancement - - oss body: - type: textarea id: problem diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 3a5de14..42959d0 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -27,4 +27,4 @@ ## Related issues - + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ed8fe47..0744c8d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -66,7 +66,8 @@ If your change touches Python runtime code under `agents/`, run `make test-pytho ## Pull Request Guidelines - Use a clear title and summary of behavior changes. -- Link the related issue (`Fixes #123`). +- Use closing keywords (`Fixes`, `Closes`, `Resolves`) only when the PR satisfies the issue's full acceptance criteria. +- Use `Refs #123` or `Relates to #123` for partial progress. - Include test evidence (commands and outcomes). - Avoid unrelated refactors in the same PR. - Keep reconciliation behavior idempotent. @@ -81,6 +82,9 @@ If your change touches Python runtime code under `agents/`, run `make test-pytho - Use the bug template for defects. - Use the feature template for enhancement requests. +- Templates apply `bug` or `enhancement` automatically. +- Priorities reflect current phase blockers. Age alone does not determine priority. +- Deferred work closes as not planned, with a scope explanation and a condition for revisiting it. - Provide reproduction steps and expected behavior. ## Security @@ -92,5 +96,7 @@ If your change touches Python runtime code under `agents/`, run `make test-pytho This repository is the open-source core. -- Include: CRD lifecycle, orchestration, isolation, generic runtime patterns. +- Include: runtime-agnostic governance, workload contracts, runtime adapters, pod isolation, declared egress, and offline-verifiable evidence. +- Supported adapters include Argo, BYO pods, and kagent. +- Generic registries, routing, sidecar buses, and multi-cluster features need a concrete regulated deployment requirement. - Exclude: proprietary licensing, billing, and private enterprise overlays. diff --git a/agents/tests/test_state.py b/agents/tests/test_state.py deleted file mode 100644 index 81e28d0..0000000 --- a/agents/tests/test_state.py +++ /dev/null @@ -1,108 +0,0 @@ -""" -Tests for LangGraph state schema. -""" - -import pytest -from agents.graph.state import AgentWorkflowState - - -def test_state_schema_structure(): - """Test that state schema has all required fields""" - state: AgentWorkflowState = { - "job_id": "test-001", - "target_urls": ["https://example.com"], - "status": "running", - "raw_html": {"https://example.com": ""}, - "screenshots": {"https://example.com": b"data"}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": None, - "start_time": None, - "end_time": None, - "messages": [], - } - - assert state["job_id"] == "test-001" - assert len(state["target_urls"]) == 1 - assert state["status"] == "running" - print("✅ State schema valid") - - -def test_state_initialization(): - """Test state can be created with defaults""" - state: AgentWorkflowState = { - "job_id": "test", - "target_urls": [], - "status": "running", - "raw_html": {}, - "screenshots": {}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": None, - "start_time": None, - "end_time": None, - "messages": [], - } - - # All required fields present - assert "job_id" in state - assert "target_urls" in state - assert "status" in state - print("✅ State initialization works") - - -def test_state_mutations(): - """Test that state can be mutated""" - state: AgentWorkflowState = { - "job_id": "test", - "target_urls": ["https://example.com"], - "status": "running", - "raw_html": {}, - "screenshots": {}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": None, - "start_time": None, - "end_time": None, - "messages": [], - } - - # Mutate state - state["status"] = "complete" - state["report_content"] = "Generated report" - state["raw_html"]["https://example.com"] = "Test" - - assert state["status"] == "complete" - assert state["report_content"] == "Generated report" - assert len(state["raw_html"]) > 0 - print("✅ State mutations work") - - -def test_state_error_handling(): - """Test state error field""" - state: AgentWorkflowState = { - "job_id": "error-test", - "target_urls": [], - "status": "failed", - "raw_html": {}, - "screenshots": {}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": "Connection refused", - "start_time": None, - "end_time": None, - "messages": [], - } - - assert state["status"] == "failed" - assert state["error"] is not None - assert "Connection" in state["error"] - print("✅ Error handling in state works") diff --git a/agents/tests/test_workflow.py b/agents/tests/test_workflow.py index 4944e90..060185d 100644 --- a/agents/tests/test_workflow.py +++ b/agents/tests/test_workflow.py @@ -152,30 +152,6 @@ async def test_synthesis_agent_report(sample_state): print("✅ Report synthesis successful") -def test_workflow_state_structure(): - """Test that state structure matches expectations""" - state: AgentWorkflowState = { - "job_id": "test", - "target_urls": ["https://example.com"], - "status": "running", - "raw_html": {}, - "screenshots": {}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": None, - "start_time": None, - "end_time": None, - "messages": [], - } - - assert state["job_id"] == "test" - assert isinstance(state["target_urls"], list) - assert isinstance(state["raw_html"], dict) - print("✅ State structure valid") - - @pytest.mark.asyncio @pytest.mark.xfail( reason="LangGraph >=1.0 enforces single-value-per-step on non-Annotated keys; " @@ -220,26 +196,3 @@ async def test_full_workflow_execution(sample_state): assert result["report_content"] is not None print("✅ Full workflow execution successful") - - -def test_error_handling(): - """Test error handling in workflow""" - state: AgentWorkflowState = { - "job_id": "error-test", - "target_urls": [], # Empty list - "status": "running", - "raw_html": {}, - "screenshots": {}, - "dom_structures": {}, - "visual_insights": [], - "competitive_signals": [], - "report_content": None, - "error": None, - "start_time": None, - "end_time": None, - "messages": [], - } - - # Should handle empty URL list gracefully - assert len(state["target_urls"]) == 0 - print("✅ Error handling tested") diff --git a/api/v1alpha1/agentworkload_compatibility_test.go b/api/v1alpha1/agentworkload_compatibility_test.go index 779cc2f..ae15811 100644 --- a/api/v1alpha1/agentworkload_compatibility_test.go +++ b/api/v1alpha1/agentworkload_compatibility_test.go @@ -335,20 +335,6 @@ func TestAgentWorkloadCompatibility_CRDObjectiveLength(t *testing.T) { } } -func TestAgentWorkloadCompatibility_WebhookAcceptsBoundedContextObjective(t *testing.T) { - objective := strings.Repeat("a", 4096) - workload := &AgentWorkload{ - Spec: AgentWorkloadSpec{ - Objective: &objective, - Agents: []string{"agent1"}, - }, - } - - if err := workload.ValidateCreate(); err != nil { - t.Fatalf("4 KiB objective allowed by the CRD must pass webhook validation: %v", err) - } -} - func TestAgentWorkloadCompatibility_CRDAndWebhookAgreeOnMultibyteObjective(t *testing.T) { objective := strings.Repeat("é", 32768) workload := &AgentWorkload{ diff --git a/api/v1alpha1/agentworkload_webhook_test.go b/api/v1alpha1/agentworkload_webhook_test.go index d6891a2..93433ee 100644 --- a/api/v1alpha1/agentworkload_webhook_test.go +++ b/api/v1alpha1/agentworkload_webhook_test.go @@ -132,42 +132,6 @@ func TestWebhook_RejectEmptyAgents(t *testing.T) { } } -func TestWebhook_RejectObjectiveTooLong(t *testing.T) { - longObjective := strings.Repeat("a", 32769) - - workload := &AgentWorkload{ - Spec: AgentWorkloadSpec{ - WorkloadType: stringPtr("generic"), - MCPServerEndpoint: stringPtr("https://localhost:8000"), - Objective: &longObjective, - Agents: []string{"agent1"}, - }, - } - - err := workload.ValidateCreate() - if err == nil { - t.Error("Expected validation error for objective > 32768 characters, got nil") - } else { - t.Logf("Correctly rejected: %v", err) - } -} - -func TestWebhook_AcceptsObjectiveAtLimit(t *testing.T) { - objective := strings.Repeat("a", 32768) - workload := &AgentWorkload{ - Spec: AgentWorkloadSpec{ - WorkloadType: stringPtr("generic"), - MCPServerEndpoint: stringPtr("https://localhost:8000"), - Objective: &objective, - Agents: []string{"agent1"}, - }, - } - - if err := workload.ValidateCreate(); err != nil { - t.Fatalf("objective at 32768-character limit was rejected: %v", err) - } -} - func TestWebhook_ObjectiveLimitCountsUnicodeCharacters(t *testing.T) { objective := strings.Repeat("é", 32768) workload := &AgentWorkload{ @@ -210,50 +174,6 @@ func TestWebhook_AcceptAllWorkloadTypes(t *testing.T) { t.Log("✅ All workload types accepted") } -func TestWebhook_DefaultAutoApproveThreshold(t *testing.T) { - workload := &AgentWorkload{ - Spec: AgentWorkloadSpec{ - WorkloadType: stringPtr("generic"), - MCPServerEndpoint: stringPtr("https://localhost:8000"), - Objective: stringPtr("test objective"), - Agents: []string{"agent1"}, - // No AutoApproveThreshold specified - }, - } - - workload.Default() - - if workload.Spec.AutoApproveThreshold == nil { - t.Error("Expected Default() to set AutoApproveThreshold") - } else if *workload.Spec.AutoApproveThreshold != "0.95" { - t.Errorf("Expected default threshold 0.95, got %q", *workload.Spec.AutoApproveThreshold) - } else { - t.Log("✅ Default AutoApproveThreshold set correctly") - } -} - -func TestWebhook_DefaultOPAPolicy(t *testing.T) { - workload := &AgentWorkload{ - Spec: AgentWorkloadSpec{ - WorkloadType: stringPtr("generic"), - MCPServerEndpoint: stringPtr("https://localhost:8000"), - Objective: stringPtr("test objective"), - Agents: []string{"agent1"}, - // No OPAPolicy specified - }, - } - - workload.Default() - - if workload.Spec.OPAPolicy == nil { - t.Error("Expected Default() to set OPAPolicy") - } else if *workload.Spec.OPAPolicy != "strict" { - t.Errorf("Expected default policy 'strict', got %q", *workload.Spec.OPAPolicy) - } else { - t.Log("✅ Default OPAPolicy set correctly") - } -} - func TestValidateThreshold(t *testing.T) { testCases := []struct { threshold string diff --git a/internal/controller/agentworkload_controller_argo_test.go b/internal/controller/agentworkload_controller_argo_test.go index 7a01e79..924ddea 100644 --- a/internal/controller/agentworkload_controller_argo_test.go +++ b/internal/controller/agentworkload_controller_argo_test.go @@ -17,111 +17,6 @@ import ( "github.com/Clawdlinux/agentic-operator-core/pkg/argo" ) -func TestReconcile_ArgoOrchestration(t *testing.T) { - t.Parallel() - - ctx := context.Background() - scheme := newControllerTestScheme(t) - orchestrationType := "argo" - jobID := "argo-direct-job" - workload := &agenticv1alpha1.AgentWorkload{ - ObjectMeta: metav1.ObjectMeta{Name: "argo-direct", Namespace: "default", UID: types.UID("argo-direct-uid")}, - Spec: agenticv1alpha1.AgentWorkloadSpec{ - JobID: &jobID, - Orchestration: &agenticv1alpha1.OrchestrationSpec{ - Type: &orchestrationType, - }, - }, - } - - k8sClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithStatusSubresource(&agenticv1alpha1.AgentWorkload{}). - WithObjects( - &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}, - &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: argo.DefaultWorkflowNamespace}}, - workload, - ). - Build() - - reconciler := &AgentWorkloadReconciler{Client: k8sClient, Scheme: scheme} - defer func() { - if recovered := recover(); recovered != nil { - t.Fatalf("reconcile panicked without an explicit Argo client: %v", recovered) - } - }() - - _, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}}) - if err != nil { - t.Fatalf("reconcile returned error: %v", err) - } - - updated := &agenticv1alpha1.AgentWorkload{} - if err := k8sClient.Get(ctx, types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}, updated); err != nil { - t.Fatalf("fetch updated workload: %v", err) - } - if updated.Status.Phase == "Failed" { - t.Fatalf("phase = Failed, want Argo path to avoid MCP failure") - } -} - -func TestReconcile_ArgoOrchestrationCreatesWorkflowAndStatusRef(t *testing.T) { - t.Parallel() - - ctx := context.Background() - scheme := newControllerTestScheme(t) - orchestrationType := "argo" - jobID := "argo-demo-job" - workload := &agenticv1alpha1.AgentWorkload{ - ObjectMeta: metav1.ObjectMeta{Name: "argo-demo", Namespace: "default", UID: types.UID("workload-uid")}, - Spec: agenticv1alpha1.AgentWorkloadSpec{ - JobID: &jobID, - Orchestration: &agenticv1alpha1.OrchestrationSpec{ - Type: &orchestrationType, - }, - }, - } - - k8sClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithStatusSubresource(&agenticv1alpha1.AgentWorkload{}). - WithObjects( - &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "default"}}, - &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: argo.DefaultWorkflowNamespace}}, - workload, - ). - Build() - - reconciler := &AgentWorkloadReconciler{Client: k8sClient, Scheme: scheme} - result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}}) - if err != nil { - t.Fatalf("reconcile returned error: %v", err) - } - if result.RequeueAfter == 0 { - t.Fatalf("requeueAfter = 0, want workflow polling requeue") - } - - workflow := &unstructured.Unstructured{} - workflow.SetGroupVersionKind(schema.GroupVersionKind{Group: "argoproj.io", Version: "v1alpha1", Kind: "Workflow"}) - if err := k8sClient.Get(ctx, types.NamespacedName{Name: workload.Name, Namespace: argo.DefaultWorkflowNamespace}, workflow); err != nil { - t.Fatalf("expected Argo Workflow to be created: %v", err) - } - if workflow.GetLabels()["agentic.io/job-id"] != workload.Name { - t.Fatalf("workflow job label = %q, want %q", workflow.GetLabels()["agentic.io/job-id"], workload.Name) - } - - updated := &agenticv1alpha1.AgentWorkload{} - if err := k8sClient.Get(ctx, types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}, updated); err != nil { - t.Fatalf("fetch updated workload: %v", err) - } - if updated.Status.Phase != "Running" { - t.Fatalf("phase = %q, want Running", updated.Status.Phase) - } - if updated.Status.ArgoWorkflow == nil || updated.Status.ArgoWorkflow.Name != workload.Name { - t.Fatalf("ArgoWorkflow ref = %#v, want workflow name %q", updated.Status.ArgoWorkflow, workload.Name) - } -} - func TestReconcile_ArgoCreatesWorkflow(t *testing.T) { t.Parallel() @@ -165,6 +60,9 @@ func TestReconcile_ArgoCreatesWorkflow(t *testing.T) { if err := k8sClient.Get(ctx, types.NamespacedName{Name: workload.Name, Namespace: argo.DefaultWorkflowNamespace}, workflow); err != nil { t.Fatalf("expected Argo Workflow to be created by fake client: %v", err) } + if workflow.GetLabels()["agentic.io/job-id"] != workload.Name { + t.Fatalf("workflow job label = %q, want %q", workflow.GetLabels()["agentic.io/job-id"], workload.Name) + } templateName, found, err := unstructured.NestedString(workflow.Object, "spec", "workflowTemplateRef", "name") if err != nil { diff --git a/internal/controller/agentworkload_controller_quota_test.go b/internal/controller/agentworkload_controller_quota_test.go index 9a15058..dcc416c 100644 --- a/internal/controller/agentworkload_controller_quota_test.go +++ b/internal/controller/agentworkload_controller_quota_test.go @@ -2,7 +2,6 @@ package controller import ( "context" - "errors" "testing" "time" @@ -16,60 +15,6 @@ import ( "github.com/Clawdlinux/agentic-operator-core/pkg/multitenancy" ) -type stubQuotaManager struct{} - -func (stubQuotaManager) CheckAndConsume(string, float64) error { - return errors.New("quota exceeded") -} - -type stubTenantResolver struct { - tenant *multitenancy.TenantContext -} - -func (r stubTenantResolver) ExtractFromNamespace(context.Context, string) (*multitenancy.TenantContext, error) { - return r.tenant, nil -} - -func TestReconcile_QuotaExceeded(t *testing.T) { - t.Parallel() - - ctx := context.Background() - scheme := newControllerTestScheme(t) - tenant := newControllerTenant("acme-quota", "agentic-customer-acme-quota") - workload := &agenticv1alpha1.AgentWorkload{ - ObjectMeta: metav1.ObjectMeta{Name: "quota-exceeded", Namespace: tenant.Namespace}, - Spec: agenticv1alpha1.AgentWorkloadSpec{}, - } - - k8sClient := fake.NewClientBuilder(). - WithScheme(scheme). - WithStatusSubresource(&agenticv1alpha1.AgentWorkload{}). - WithObjects(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: tenant.Namespace}}, workload). - Build() - - reconciler := &AgentWorkloadReconciler{ - Client: k8sClient, - Scheme: scheme, - QuotaMgr: stubQuotaManager{}, - TenantRes: stubTenantResolver{tenant: tenant}, - } - result, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}}) - if err != nil { - t.Fatalf("reconcile returned error: %v", err) - } - if result.RequeueAfter != time.Hour { - t.Fatalf("requeueAfter = %s, want 1h", result.RequeueAfter) - } - - updated := &agenticv1alpha1.AgentWorkload{} - if err := k8sClient.Get(ctx, types.NamespacedName{Name: workload.Name, Namespace: workload.Namespace}, updated); err != nil { - t.Fatalf("fetch updated workload: %v", err) - } - if updated.Status.Phase != "Failed" { - t.Fatalf("phase = %q, want Failed", updated.Status.Phase) - } -} - func TestReconcile_QuotaExceededFailsAndRequeuesForReset(t *testing.T) { t.Parallel() diff --git a/pkg/runtime/adapter_test.go b/pkg/runtime/adapter_test.go index e64b04f..1011a4c 100644 --- a/pkg/runtime/adapter_test.go +++ b/pkg/runtime/adapter_test.go @@ -46,18 +46,6 @@ func TestRuntimeAdapterInterface_Compliance(t *testing.T) { } } -// TestExecutionStatus_NormalizedPhases verifies the status model works -// for any adapter, not just Argo. -func TestExecutionStatus_NormalizedPhases(t *testing.T) { - phases := []string{"Pending", "Running", "Suspended", "Succeeded", "Failed", "Error"} - for _, phase := range phases { - s := ExecutionStatus{Phase: phase, Name: "test", Namespace: "default"} - if s.Phase == "" { - t.Errorf("phase %q should not be empty after assignment", phase) - } - } -} - // TestRuntimeAdapter_CleanupNilWorkflow verifies cleanup is safe when no // workflow reference exists. This is runtime-neutral behavior. func TestRuntimeAdapter_CleanupNilWorkflow(t *testing.T) {