From 582c2e6987a90cd8a9e65f2753126285d51e3397 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Thu, 7 May 2026 03:11:51 +0530 Subject: [PATCH 1/4] Add unit tests for resource help and tool delete functionality - Implemented TestResource_Help to validate help command behavior for various subcommands. - Added tests for the delete tool to ensure it handles readonly mode and validation errors correctly when both path and name are provided, or neither is provided. - Included TestTool_Delete_Readonly, TestTool_Delete_BothPathAndName, and TestTool_Delete_NeitherPathNorName to cover these scenarios. These tests enhance the coverage and reliability of the MCP resource and tool functionalities. --- pkg/mcp/tools_delete_test.go | 64 ++++++++++++++++++++++++++++++++++++ pkg/mcp/tools_deploy_test.go | 19 +++++++++++ 2 files changed, 83 insertions(+) diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index 58f5769821..2f9a59fbc8 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -8,6 +8,70 @@ import ( "knative.dev/func/pkg/mcp/mock" ) +// TestTool_Delete_Readonly ensures the delete tool returns an error when the server is in readonly mode. +func TestTool_Delete_Readonly(t *testing.T) { + client, _, err := newTestPairWithReadonly(t, true) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{"name": "my-function"}, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result for readonly server, got success") + } +} + +// TestTool_Delete_BothPathAndName ensures the delete tool returns a validation error +// when both path and name are provided simultaneously. +func TestTool_Delete_BothPathAndName(t *testing.T) { + client, server, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + server.readonly = false + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{ + "path": "/some/path", + "name": "my-function", + }, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when both path and name are provided, got success") + } +} + +// TestTool_Delete_NeitherPathNorName ensures the delete tool returns a validation error +// when neither path nor name is provided. +func TestTool_Delete_NeitherPathNorName(t *testing.T) { + client, server, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + server.readonly = false + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "delete", + Arguments: map[string]any{}, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result when neither path nor name is provided, got success") + } +} + // TestTool_Delete_Args ensures the delete tool executes with all arguments passed correctly. func TestTool_Delete_Args(t *testing.T) { // Test data - defined once and used for both input and validation diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index 5ec697622d..609b98d063 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -8,6 +8,25 @@ import ( "knative.dev/func/pkg/mcp/mock" ) +// TestTool_Deploy_Readonly ensures the deploy tool returns an error when the server is in readonly mode. +func TestTool_Deploy_Readonly(t *testing.T) { + client, _, err := newTestPairWithReadonly(t, true) + if err != nil { + t.Fatal(err) + } + + result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ + Name: "deploy", + Arguments: map[string]any{"path": "."}, + }) + if err != nil { + t.Fatal(err) + } + if !result.IsError { + t.Fatal("expected error result for readonly server, got success") + } +} + // TestTool_Deploy_Args ensures the deploy tool executes with all arguments passed correctly. func TestTool_Deploy_Args(t *testing.T) { // Test data - defined once and used for both input and validation From fff70e6b3ce21ef695a3902b89658595b985012a Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Thu, 7 May 2026 03:22:04 +0530 Subject: [PATCH 2/4] Refactor test setup in tool delete tests - Simplified the test setup in TestTool_Delete_BothPathAndName and TestTool_Delete_NeitherPathNorName by removing the unused server variable from newTestPair. - This change enhances code clarity and maintains the focus on testing the delete tool's functionality. --- pkg/mcp/tools_delete_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index 2f9a59fbc8..b9e036d15f 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -30,11 +30,10 @@ func TestTool_Delete_Readonly(t *testing.T) { // TestTool_Delete_BothPathAndName ensures the delete tool returns a validation error // when both path and name are provided simultaneously. func TestTool_Delete_BothPathAndName(t *testing.T) { - client, server, err := newTestPair(t) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } - server.readonly = false result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "delete", @@ -54,11 +53,10 @@ func TestTool_Delete_BothPathAndName(t *testing.T) { // TestTool_Delete_NeitherPathNorName ensures the delete tool returns a validation error // when neither path nor name is provided. func TestTool_Delete_NeitherPathNorName(t *testing.T) { - client, server, err := newTestPair(t) + client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } - server.readonly = false result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ Name: "delete", From c10245f90079a4e7889472d5dca77e9619e919ad Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 22 May 2026 10:57:13 +0530 Subject: [PATCH 3/4] Enhance error message validation in delete and deploy tool tests - Updated TestTool_Delete_Readonly and TestTool_Deploy_Readonly to verify that the error messages returned when the server is in readonly mode contain the phrase "readonly mode". - Modified TestTool_Delete_BothPathAndName and TestTool_Delete_NeitherPathNorName to ensure that mutual-exclusion validation errors are correctly checked for the phrases "exactly one of 'path' or 'name'". - These changes improve the robustness of the tests by ensuring that the error messages are not only returned but also contain the expected content. --- pkg/mcp/tools_delete_test.go | 24 +++++++++++++++++++----- pkg/mcp/tools_deploy_test.go | 8 +++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index b9e036d15f..027c075ca1 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -2,13 +2,15 @@ package mcp import ( "context" + "strings" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" "knative.dev/func/pkg/mcp/mock" ) -// TestTool_Delete_Readonly ensures the delete tool returns an error when the server is in readonly mode. +// TestTool_Delete_Readonly ensures the delete tool returns the readonly-mode +// error when the server is in readonly mode. func TestTool_Delete_Readonly(t *testing.T) { client, _, err := newTestPairWithReadonly(t, true) if err != nil { @@ -25,10 +27,14 @@ func TestTool_Delete_Readonly(t *testing.T) { if !result.IsError { t.Fatal("expected error result for readonly server, got success") } + got := result.Content[0].(*mcp.TextContent).Text + if !strings.Contains(got, "readonly mode") { + t.Fatalf("expected readonly-mode error, got: %q", got) + } } -// TestTool_Delete_BothPathAndName ensures the delete tool returns a validation error -// when both path and name are provided simultaneously. +// TestTool_Delete_BothPathAndName ensures the delete tool returns a mutual- +// exclusion validation error when both path and name are provided. func TestTool_Delete_BothPathAndName(t *testing.T) { client, _, err := newTestPair(t) if err != nil { @@ -48,10 +54,14 @@ func TestTool_Delete_BothPathAndName(t *testing.T) { if !result.IsError { t.Fatal("expected error result when both path and name are provided, got success") } + got := result.Content[0].(*mcp.TextContent).Text + if !strings.Contains(got, "exactly one of 'path' or 'name'") { + t.Fatalf("expected mutual-exclusion validation error, got: %q", got) + } } -// TestTool_Delete_NeitherPathNorName ensures the delete tool returns a validation error -// when neither path nor name is provided. +// TestTool_Delete_NeitherPathNorName ensures the delete tool returns a mutual- +// exclusion validation error when neither path nor name is provided. func TestTool_Delete_NeitherPathNorName(t *testing.T) { client, _, err := newTestPair(t) if err != nil { @@ -68,6 +78,10 @@ func TestTool_Delete_NeitherPathNorName(t *testing.T) { if !result.IsError { t.Fatal("expected error result when neither path nor name is provided, got success") } + got := result.Content[0].(*mcp.TextContent).Text + if !strings.Contains(got, "exactly one of 'path' or 'name'") { + t.Fatalf("expected mutual-exclusion validation error, got: %q", got) + } } // TestTool_Delete_Args ensures the delete tool executes with all arguments passed correctly. diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index 609b98d063..eb03636dd1 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -2,13 +2,15 @@ package mcp import ( "context" + "strings" "testing" "github.com/modelcontextprotocol/go-sdk/mcp" "knative.dev/func/pkg/mcp/mock" ) -// TestTool_Deploy_Readonly ensures the deploy tool returns an error when the server is in readonly mode. +// TestTool_Deploy_Readonly ensures the deploy tool returns the readonly-mode +// error when the server is in readonly mode. func TestTool_Deploy_Readonly(t *testing.T) { client, _, err := newTestPairWithReadonly(t, true) if err != nil { @@ -25,6 +27,10 @@ func TestTool_Deploy_Readonly(t *testing.T) { if !result.IsError { t.Fatal("expected error result for readonly server, got success") } + got := result.Content[0].(*mcp.TextContent).Text + if !strings.Contains(got, "readonly mode") { + t.Fatalf("expected readonly-mode error, got: %q", got) + } } // TestTool_Deploy_Args ensures the deploy tool executes with all arguments passed correctly. From a07cc5ee8c98dedd225bc8cf65219ae189d3eaab Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Fri, 22 May 2026 11:06:29 +0530 Subject: [PATCH 4/4] Remove unused readonly mode tests for delete and deploy tools - Deleted TestTool_Delete_Readonly and TestTool_Deploy_Readonly as they were redundant following recent enhancements to error message validation. - This cleanup improves test maintainability and focuses on essential test cases for the delete and deploy functionalities. --- pkg/mcp/tools_delete_test.go | 19 ------------------- pkg/mcp/tools_deploy_test.go | 19 ------------------- 2 files changed, 38 deletions(-) diff --git a/pkg/mcp/tools_delete_test.go b/pkg/mcp/tools_delete_test.go index 027c075ca1..99d6cd7646 100644 --- a/pkg/mcp/tools_delete_test.go +++ b/pkg/mcp/tools_delete_test.go @@ -152,22 +152,3 @@ func TestTool_Delete_Args(t *testing.T) { t.Fatal("executor was not invoked") } } - -// TestTool_Delete_Readonly ensures the delete tool rejects requests in readonly mode. -func TestTool_Delete_Readonly(t *testing.T) { - client, _, err := newTestPairWithReadonly(t, true) // readonly = true - if err != nil { - t.Fatal(err) - } - - result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ - Name: "delete", - Arguments: map[string]any{"name": "my-function"}, - }) - if err != nil { - t.Fatal(err) - } - if !result.IsError { - t.Fatal("expected delete to be rejected in readonly mode") - } -} diff --git a/pkg/mcp/tools_deploy_test.go b/pkg/mcp/tools_deploy_test.go index eb03636dd1..940ded5fe5 100644 --- a/pkg/mcp/tools_deploy_test.go +++ b/pkg/mcp/tools_deploy_test.go @@ -104,22 +104,3 @@ func TestTool_Deploy_Args(t *testing.T) { t.Fatal("executor was not invoked") } } - -// TestTool_Deploy_Readonly ensures the deploy tool rejects requests in readonly mode. -func TestTool_Deploy_Readonly(t *testing.T) { - client, _, err := newTestPairWithReadonly(t, true) // readonly = true - if err != nil { - t.Fatal(err) - } - - result, err := client.CallTool(t.Context(), &mcp.CallToolParams{ - Name: "deploy", - Arguments: map[string]any{"path": "."}, - }) - if err != nil { - t.Fatal(err) - } - if !result.IsError { - t.Fatal("expected deploy to be rejected in readonly mode") - } -}