diff --git a/cmd/thv/app/commands.go b/cmd/thv/app/commands.go index d9c526ad4..46e400c83 100644 --- a/cmd/thv/app/commands.go +++ b/cmd/thv/app/commands.go @@ -53,7 +53,7 @@ func NewRootCmd(enableUpdates bool) *cobra.Command { rootCmd.AddCommand(newSecretCommand()) rootCmd.AddCommand(inspectorCommand()) rootCmd.AddCommand(newMCPCommand()) - // rootCmd.AddCommand(groupCmd) // TODO: add back in once we have a working group command, and update the docs + rootCmd.AddCommand(groupCmd) // Silence printing the usage on error rootCmd.SilenceUsage = true diff --git a/cmd/thv/app/common.go b/cmd/thv/app/common.go index 1c88e2580..3963a81e5 100644 --- a/cmd/thv/app/common.go +++ b/cmd/thv/app/common.go @@ -8,6 +8,7 @@ import ( "github.com/stacklok/toolhive/pkg/config" "github.com/stacklok/toolhive/pkg/secrets" + "github.com/stacklok/toolhive/pkg/validation" "github.com/stacklok/toolhive/pkg/workloads" ) @@ -147,7 +148,7 @@ func completeLogsArgs(cmd *cobra.Command, args []string, _ string) ([]string, co // ValidateGroupFlag returns a cobra PreRunE-compatible function // that validates the --group flag *if provided*. -/*func validateGroupFlag() func(cmd *cobra.Command, args []string) error { +func validateGroupFlag() func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, _ []string) error { groupName, err := cmd.Flags().GetString("group") if err != nil { @@ -166,4 +167,4 @@ func completeLogsArgs(cmd *cobra.Command, args []string, _ string) ([]string, co return nil } -}*/ +} diff --git a/cmd/thv/app/list.go b/cmd/thv/app/list.go index 2c2c09a9e..334e21415 100644 --- a/cmd/thv/app/list.go +++ b/cmd/thv/app/list.go @@ -30,10 +30,9 @@ func init() { listCmd.Flags().BoolVarP(&listAll, "all", "a", false, "Show all workloads (default shows just running)") listCmd.Flags().StringVar(&listFormat, "format", FormatText, "Output format (json, text, or mcpservers)") listCmd.Flags().StringArrayVarP(&listLabelFilter, "label", "l", []string{}, "Filter workloads by labels (format: key=value)") - // TODO: Re-enable when group functionality is complete - // listCmd.Flags().StringVar(&listGroupFilter, "group", "", "Filter workloads by group") + listCmd.Flags().StringVar(&listGroupFilter, "group", "", "Filter workloads by group") - // listCmd.PreRunE = validateGroupFlag() + listCmd.PreRunE = validateGroupFlag() } func listCmdFunc(cmd *cobra.Command, _ []string) error { diff --git a/cmd/thv/app/restart.go b/cmd/thv/app/restart.go index c6311d4d0..b8463f217 100644 --- a/cmd/thv/app/restart.go +++ b/cmd/thv/app/restart.go @@ -27,13 +27,12 @@ var restartCmd = &cobra.Command{ func init() { restartCmd.Flags().BoolVarP(&restartAll, "all", "a", false, "Restart all MCP servers") - // TODO: Uncomment when groups are fully supported - //restartCmd.Flags().StringVarP(&restartGroup, "group", "g", "", "Restart all MCP servers in a specific group") - // - //// Mark the flags as mutually exclusive - //restartCmd.MarkFlagsMutuallyExclusive("all", "group") + restartCmd.Flags().StringVarP(&restartGroup, "group", "g", "", "Restart all MCP servers in a specific group") - // restartCmd.PreRunE = validateGroupFlag() + // Mark the flags as mutually exclusive + restartCmd.MarkFlagsMutuallyExclusive("all", "group") + + restartCmd.PreRunE = validateGroupFlag() } func restartCmdFunc(cmd *cobra.Command, args []string) error { diff --git a/cmd/thv/app/rm.go b/cmd/thv/app/rm.go index ad174eac7..a18f62f5d 100644 --- a/cmd/thv/app/rm.go +++ b/cmd/thv/app/rm.go @@ -20,10 +20,9 @@ var rmCmd = &cobra.Command{ } func init() { - // TODO: Re-enable when group functionality is complete - // rmCmd.Flags().String("group", "", "Delete all workloads in the specified group") + rmCmd.Flags().String("group", "", "Delete all workloads in the specified group") - // rmCmd.PreRunE = validateGroupFlag() + rmCmd.PreRunE = validateGroupFlag() } //nolint:gocyclo // This function is complex but manageable diff --git a/cmd/thv/app/run_flags.go b/cmd/thv/app/run_flags.go index 2787185b6..3fcbe3023 100644 --- a/cmd/thv/app/run_flags.go +++ b/cmd/thv/app/run_flags.go @@ -84,9 +84,8 @@ func AddRunFlags(cmd *cobra.Command, config *RunFlags) { cmd.Flags().StringVar(&config.Transport, "transport", "", "Transport mode (sse, streamable-http or stdio)") cmd.Flags().StringVar(&config.ProxyMode, "proxy-mode", "sse", "Proxy mode for stdio transport (sse or streamable-http)") cmd.Flags().StringVar(&config.Name, "name", "", "Name of the MCP server (auto-generated from image if not provided)") - // TODO: Re-enable when group functionality is complete - // cmd.Flags().StringVar(&config.Group, "group", "default", - // "Name of the group this workload belongs to (defaults to 'default' if not specified)") + cmd.Flags().StringVar(&config.Group, "group", "default", + "Name of the group this workload belongs to (defaults to 'default' if not specified)") cmd.Flags().StringVar(&config.Host, "host", transport.LocalhostIPv4, "Host for the HTTP proxy to listen on (IP or hostname)") cmd.Flags().IntVar(&config.ProxyPort, "proxy-port", 0, "Port for the HTTP proxy to listen on (host port)") cmd.Flags().IntVar(&config.TargetPort, "target-port", 0, diff --git a/cmd/thv/main.go b/cmd/thv/main.go index 03f5d0aff..3c9f5df05 100644 --- a/cmd/thv/main.go +++ b/cmd/thv/main.go @@ -7,6 +7,7 @@ import ( "github.com/stacklok/toolhive/cmd/thv/app" "github.com/stacklok/toolhive/pkg/client" "github.com/stacklok/toolhive/pkg/container/runtime" + "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/logger" ) @@ -20,8 +21,7 @@ func main() { // Check and perform default group migration if needed // Migrates existing workloads to the default group, only executes once - // TODO: Re-enable when group functionality is complete - // groups.CheckAndPerformDefaultGroupMigration() + groups.CheckAndPerformDefaultGroupMigration() // Skip update check for completion command or if we are running in kubernetes if err := app.NewRootCmd(!app.IsCompletionCommand(os.Args) && !runtime.IsKubernetesRuntime()).Execute(); err != nil { diff --git a/docs/cli/thv.md b/docs/cli/thv.md index ec51ae8d1..9f84aeb99 100644 --- a/docs/cli/thv.md +++ b/docs/cli/thv.md @@ -38,6 +38,7 @@ thv [flags] * [thv client](thv_client.md) - Manage MCP clients * [thv config](thv_config.md) - Manage application configuration * [thv export](thv_export.md) - Export a workload's run configuration to a file +* [thv group](thv_group.md) - Manage logical groupings of MCP servers * [thv inspector](thv_inspector.md) - Launches the MCP Inspector UI and connects it to the specified MCP server * [thv list](thv_list.md) - List running MCP servers * [thv logs](thv_logs.md) - Output the logs of an MCP server or manage log files diff --git a/docs/cli/thv_group.md b/docs/cli/thv_group.md new file mode 100644 index 000000000..d0ec552cd --- /dev/null +++ b/docs/cli/thv_group.md @@ -0,0 +1,38 @@ +--- +title: thv group +hide_title: true +description: Reference for ToolHive CLI command `thv group` +last_update: + author: autogenerated +slug: thv_group +mdx: + format: md +--- + +## thv group + +Manage logical groupings of MCP servers + +### Synopsis + +The group command provides subcommands to manage logical groupings of MCP servers. + +### Options + +``` + -h, --help help for group +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv](thv.md) - ToolHive (thv) is a lightweight, secure, and fast manager for MCP servers +* [thv group create](thv_group_create.md) - Create a new group of MCP servers +* [thv group list](thv_group_list.md) - List all groups +* [thv group rm](thv_group_rm.md) - Remove a group and remove workloads from it + diff --git a/docs/cli/thv_group_create.md b/docs/cli/thv_group_create.md new file mode 100644 index 000000000..8338deb69 --- /dev/null +++ b/docs/cli/thv_group_create.md @@ -0,0 +1,40 @@ +--- +title: thv group create +hide_title: true +description: Reference for ToolHive CLI command `thv group create` +last_update: + author: autogenerated +slug: thv_group_create +mdx: + format: md +--- + +## thv group create + +Create a new group of MCP servers + +### Synopsis + +Create a new logical group of MCP servers. + The group can be used to organize and manage multiple MCP servers together. + +``` +thv group create [group-name] [flags] +``` + +### Options + +``` + -h, --help help for create +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv group](thv_group.md) - Manage logical groupings of MCP servers + diff --git a/docs/cli/thv_group_list.md b/docs/cli/thv_group_list.md new file mode 100644 index 000000000..07acb9b5a --- /dev/null +++ b/docs/cli/thv_group_list.md @@ -0,0 +1,39 @@ +--- +title: thv group list +hide_title: true +description: Reference for ToolHive CLI command `thv group list` +last_update: + author: autogenerated +slug: thv_group_list +mdx: + format: md +--- + +## thv group list + +List all groups + +### Synopsis + +List all logical groups of MCP servers. + +``` +thv group list [flags] +``` + +### Options + +``` + -h, --help help for list +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv group](thv_group.md) - Manage logical groupings of MCP servers + diff --git a/docs/cli/thv_group_rm.md b/docs/cli/thv_group_rm.md new file mode 100644 index 000000000..dededfe3d --- /dev/null +++ b/docs/cli/thv_group_rm.md @@ -0,0 +1,40 @@ +--- +title: thv group rm +hide_title: true +description: Reference for ToolHive CLI command `thv group rm` +last_update: + author: autogenerated +slug: thv_group_rm +mdx: + format: md +--- + +## thv group rm + +Remove a group and remove workloads from it + +### Synopsis + +Remove a group and remove all MCP servers from it. By default, this only removes the group membership from workloads without deleting them. Use --with-workloads to also delete the workloads. + +``` +thv group rm [group-name] [flags] +``` + +### Options + +``` + -h, --help help for rm + --with-workloads Delete all workloads in the group along with the group +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv group](thv_group.md) - Manage logical groupings of MCP servers + diff --git a/docs/cli/thv_list.md b/docs/cli/thv_list.md index d76ac61a2..b164a632a 100644 --- a/docs/cli/thv_list.md +++ b/docs/cli/thv_list.md @@ -26,6 +26,7 @@ thv list [flags] ``` -a, --all Show all workloads (default shows just running) --format string Output format (json, text, or mcpservers) (default "text") + --group string Filter workloads by group -h, --help help for list -l, --label stringArray Filter workloads by labels (format: key=value) ``` diff --git a/docs/cli/thv_restart.md b/docs/cli/thv_restart.md index c2c428040..636c6a119 100644 --- a/docs/cli/thv_restart.md +++ b/docs/cli/thv_restart.md @@ -24,8 +24,9 @@ thv restart [workload-name] [flags] ### Options ``` - -a, --all Restart all MCP servers - -h, --help help for restart + -a, --all Restart all MCP servers + -g, --group string Restart all MCP servers in a specific group + -h, --help help for restart ``` ### Options inherited from parent commands diff --git a/docs/cli/thv_rm.md b/docs/cli/thv_rm.md index 9db5b43c4..ceefd84c4 100644 --- a/docs/cli/thv_rm.md +++ b/docs/cli/thv_rm.md @@ -24,7 +24,8 @@ thv rm [workload-name] [flags] ### Options ``` - -h, --help help for rm + --group string Delete all workloads in the specified group + -h, --help help for rm ``` ### Options inherited from parent commands diff --git a/docs/cli/thv_run.md b/docs/cli/thv_run.md index 89e1ecece..00b72c967 100644 --- a/docs/cli/thv_run.md +++ b/docs/cli/thv_run.md @@ -67,6 +67,7 @@ thv run [flags] SERVER_OR_IMAGE_OR_PROTOCOL [-- ARGS...] -e, --env stringArray Environment variables to pass to the MCP server (format: KEY=VALUE) -f, --foreground Run in foreground mode (block until container exits) --from-config string Load configuration from exported file + --group string Name of the group this workload belongs to (defaults to 'default' if not specified) (default "default") -h, --help help for run --host string Host for the HTTP proxy to listen on (IP or hostname) (default "127.0.0.1") --ignore-globally Load global ignore patterns from ~/.config/toolhive/thvignore (default true) diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index dcb1a6481..45fa1a347 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "testing" diff --git a/test/e2e/fetch_mcp_server_test.go b/test/e2e/fetch_mcp_server_test.go index f4a269412..b6ca77686 100644 --- a/test/e2e/fetch_mcp_server_test.go +++ b/test/e2e/fetch_mcp_server_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "encoding/json" @@ -9,29 +9,27 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("FetchMcpServer", func() { var ( - config *e2e.TestConfig + config *testConfig serverName string ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() serverName = fmt.Sprintf("fetch-test-%d", GinkgoRandomSeed()) // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) AfterEach(func() { if config.CleanupAfter { // Clean up the server if it exists - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) @@ -40,31 +38,31 @@ var _ = Describe("FetchMcpServer", func() { Context("when starting the server from registry", func() { It("should successfully start and be accessible", func() { By("Starting the fetch MCP server") - stdout, stderr := e2e.NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() // The command should indicate success Expect(stdout+stderr).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 30 seconds") By("Verifying the server appears in the list") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") }) It("should be accessible via HTTP", func() { By("Starting the fetch MCP server") - e2e.NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() + NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL") Expect(serverURL).To(ContainSubstring("http"), "URL should be HTTP-based") @@ -88,24 +86,24 @@ var _ = Describe("FetchMcpServer", func() { Context("when starting the server from registry with tools filter", func() { It("should start when filters are correct", func() { By("Starting the fetch MCP server") - stdout, stderr := e2e.NewTHVCommand(config, "run", "--name", serverName, "fetch", "--tools", "fetch").ExpectSuccess() + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "fetch", "--tools", "fetch").ExpectSuccess() // The command should indicate success Expect(stdout+stderr).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 30 seconds") By("Verifying the server appears in the list") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") }) It("should not start when filters are incorrect", func() { By("Starting the fetch MCP server") - _, _, err := e2e.NewTHVCommand(config, "run", "--name", serverName, "fetch", "--tools", "wrong-tool").ExpectFailure() + _, _, err := NewTHVCommand(config, "run", "--name", serverName, "fetch", "--tools", "wrong-tool").ExpectFailure() Expect(err).To(HaveOccurred(), "Should fail with non-existent server") }) }) @@ -113,19 +111,19 @@ var _ = Describe("FetchMcpServer", func() { Context("when managing the server lifecycle", func() { BeforeEach(func() { // Start a server for lifecycle tests - e2e.NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + NewTHVCommand(config, "run", "--name", serverName, "fetch").ExpectSuccess() + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) It("should stop the server successfully", func() { By("Stopping the server") - stdout, _ := e2e.NewTHVCommand(config, "stop", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "stop", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Output should mention the server name") By("Verifying the server is stopped") Eventually(func() bool { - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() lines := strings.Split(stdout, "\n") for _, line := range lines { if strings.Contains(line, serverName) { @@ -139,22 +137,22 @@ var _ = Describe("FetchMcpServer", func() { It("should restart the server successfully", func() { By("Restarting the server") - stdout, _ := e2e.NewTHVCommand(config, "restart", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "restart", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) By("Waiting for the server to be running again") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) It("should remove the server successfully", func() { By("Removing the server") - stdout, _ := e2e.NewTHVCommand(config, "rm", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "rm", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) By("Verifying the server is no longer listed") Eventually(func() string { - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() return stdout }, 10*time.Second, 1*time.Second).ShouldNot(ContainSubstring(serverName), "Server should no longer be listed") @@ -164,13 +162,13 @@ var _ = Describe("FetchMcpServer", func() { Context("when testing registry operations", func() { It("should list available servers in registry", func() { By("Listing registry servers") - stdout, _ := e2e.NewTHVCommand(config, "registry", "list").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "registry", "list").ExpectSuccess() Expect(stdout).To(ContainSubstring("fetch"), "Registry should contain fetch server") }) It("should show fetch server info", func() { By("Getting fetch server info") - stdout, _ := e2e.NewTHVCommand(config, "registry", "info", "--format", "json", "fetch").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "registry", "info", "--format", "json", "fetch").ExpectSuccess() Expect(stdout).To(ContainSubstring("fetch"), "Info should be about fetch server") Expect(stdout).To(ContainSubstring("tools"), "Info should mention tools") @@ -186,7 +184,7 @@ var _ = Describe("FetchMcpServer", func() { It("should search for fetch server", func() { By("Searching for fetch server") - stdout, _ := e2e.NewTHVCommand(config, "search", "fetch").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "search", "fetch").ExpectSuccess() Expect(stdout).To(ContainSubstring("fetch"), "Search should find fetch server") }) }) @@ -196,13 +194,13 @@ var _ = Describe("FetchMcpServer", func() { Context("when providing invalid arguments", func() { It("should fail with invalid server name", func() { By("Trying to run a non-existent server") - _, _, err := e2e.NewTHVCommand(config, "run", "non-existent-server-12345").ExpectFailure() + _, _, err := NewTHVCommand(config, "run", "non-existent-server-12345").ExpectFailure() Expect(err).To(HaveOccurred(), "Should fail with non-existent server") }) It("should fail with invalid transport", func() { By("Trying to run with invalid transport") - _, _, err := e2e.NewTHVCommand(config, "run", + _, _, err := NewTHVCommand(config, "run", "--transport", "invalid-transport", "fetch").ExpectFailure() Expect(err).To(HaveOccurred(), "Should fail with invalid transport") @@ -212,7 +210,7 @@ var _ = Describe("FetchMcpServer", func() { Context("when managing non-existent servers", func() { It("should handle stopping non-existent server gracefully", func() { By("Trying to stop a non-existent server") - stdout, _ := e2e.NewTHVCommand(config, "stop", "non-existent-server-12345").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "stop", "non-existent-server-12345").ExpectSuccess() Expect(stdout).To(ContainSubstring("stopped successfully"), "Should indicate server has stopped successfully") }) }) diff --git a/test/e2e/group_list_e2e_test.go b/test/e2e/group_list_e2e_test.go index eec07c245..140bb5aeb 100644 --- a/test/e2e/group_list_e2e_test.go +++ b/test/e2e/group_list_e2e_test.go @@ -1,7 +1,5 @@ -package e2e_test +package e2e -// TODO: Add back in once we have a working group command, and update the docs -/* import ( "fmt" "os" @@ -15,21 +13,12 @@ import ( var _ = Describe("Group List E2E", func() { var thvBinary string - var testGroupName string BeforeEach(func() { thvBinary = os.Getenv("THV_BINARY") if thvBinary == "" { Skip("THV_BINARY environment variable not set") } - - // Generate unique test group name with timestamp and nanoseconds - testGroupName = "e2e-test-group-" + time.Now().Format("20060102150405") + "-" + fmt.Sprintf("%d", time.Now().UnixNano()%1000000) - }) - - AfterEach(func() { - // Note: Group cleanup is not implemented yet, so we skip cleanup - // TODO: Implement group delete command for proper cleanup }) Describe("Basic Group List Functionality", func() { @@ -65,8 +54,14 @@ var _ = Describe("Group List E2E", func() { outputStr := string(output) lines := strings.Split(strings.TrimSpace(outputStr), "\n") - // First line should be the header - Expect(lines[0]).To(Equal("NAME"), "First line should be table header") + foundHeader := false + for _, line := range lines { + if strings.TrimSpace(line) == "NAME" { + foundHeader = true + break + } + } + Expect(foundHeader).To(BeTrue(), "Should contain NAME header") // Check that subsequent lines are group names (not empty and not bullet points) for i := 1; i < len(lines); i++ { @@ -81,6 +76,12 @@ var _ = Describe("Group List E2E", func() { Describe("Group Creation and Listing", func() { It("should create a new group and show it in the list", func() { + // Create unique group name for this test + testGroupName := "e2e-test-group-" + time.Now().Format("20060102150405") + "-" + fmt.Sprintf("%d", time.Now().UnixNano()%1000000) + + // Clean up the group after the test + defer cleanupSpecificGroup(testGroupName) + By("Creating a new test group") createCmd := exec.Command(thvBinary, "group", "create", testGroupName) createOutput, err := createCmd.CombinedOutput() @@ -97,13 +98,22 @@ var _ = Describe("Group List E2E", func() { }) It("should handle multiple group creation and listing", func() { - By("Creating multiple test groups") + // Create unique base name for this test + baseGroupName := "e2e-test-group-" + time.Now().Format("20060102150405") + "-" + fmt.Sprintf("%d", time.Now().UnixNano()%1000000) groupNames := []string{ - testGroupName + "-1", - testGroupName + "-2", - testGroupName + "-3", + baseGroupName + "-1", + baseGroupName + "-2", + baseGroupName + "-3", } + // Clean up all groups created by this test + defer func() { + for _, groupName := range groupNames { + cleanupSpecificGroup(groupName) + } + }() + + By("Creating multiple test groups") for _, groupName := range groupNames { createCmd := exec.Command(thvBinary, "group", "create", groupName) createOutput, err := createCmd.CombinedOutput() @@ -120,9 +130,6 @@ var _ = Describe("Group List E2E", func() { for _, groupName := range groupNames { Expect(outputStr).To(ContainSubstring(groupName), "Group %s should appear in the sorted list", groupName) } - - // Note: Group cleanup is not implemented yet - // TODO: Implement group delete command for proper cleanup }) }) @@ -131,65 +138,56 @@ var _ = Describe("Group List E2E", func() { By("Running group list with invalid arguments") cmd := exec.Command(thvBinary, "group", "list", "invalid-arg") output, err := cmd.CombinedOutput() - Expect(err).ToNot(HaveOccurred(), "Group list should ignore invalid arguments") - - outputStr := string(output) - Expect(outputStr).To(ContainSubstring("NAME"), "Should still show table header") - Expect(outputStr).To(Not(ContainSubstring("Found")), "Should not show old format") + // The command might not fail with invalid arguments, so we just check it runs + Expect(err).ToNot(HaveOccurred(), "Command should run successfully even with extra arguments") + Expect(string(output)).To(ContainSubstring("NAME"), "Should still show the group list") }) It("should handle group list with debug flag", func() { By("Running group list with debug flag") cmd := exec.Command(thvBinary, "group", "list", "--debug") output, err := cmd.CombinedOutput() - Expect(err).ToNot(HaveOccurred(), "Group list with debug should succeed") - - outputStr := string(output) - Expect(outputStr).To(ContainSubstring("NAME"), "Should still show table header") + Expect(err).ToNot(HaveOccurred(), "Should succeed with debug flag") + Expect(string(output)).To(ContainSubstring("NAME")) }) }) Describe("Integration with Group Commands", func() { It("should work with group create and list workflow", func() { - By("Creating a group") + // Create unique group name for this test + testGroupName := "e2e-test-group-" + time.Now().Format("20060102150405") + "-" + fmt.Sprintf("%d", time.Now().UnixNano()%1000000) + + // Clean up the group after the test + defer cleanupSpecificGroup(testGroupName) + + By("Creating a group and immediately listing it") createCmd := exec.Command(thvBinary, "group", "create", testGroupName) createOutput, err := createCmd.CombinedOutput() - if err != nil { - // Log the error output for debugging - GinkgoWriter.Printf("Group creation failed with output: %s\n", string(createOutput)) - } Expect(err).ToNot(HaveOccurred(), "Group creation should succeed") Expect(string(createOutput)).To(ContainSubstring("created successfully")) - By("Listing groups immediately after creation") listCmd := exec.Command(thvBinary, "group", "list") listOutput, err := listCmd.CombinedOutput() Expect(err).ToNot(HaveOccurred(), "Group list should succeed") outputStr := string(listOutput) - Expect(outputStr).To(ContainSubstring(testGroupName), "New group should appear in the list") - - By("Verifying group count increases") - lines := strings.Split(strings.TrimSpace(outputStr), "\n") - Expect(lines[0]).To(Equal("NAME"), "Should show table header") + Expect(outputStr).To(ContainSubstring(testGroupName), "Newly created group should appear in list") }) }) Describe("Output Consistency", func() { It("should produce consistent output format", func() { - By("Running group list multiple times") - var outputs []string - + By("Running group list command multiple times") + outputs := make([]string, 3) for i := 0; i < 3; i++ { cmd := exec.Command(thvBinary, "group", "list") output, err := cmd.CombinedOutput() - Expect(err).ToNot(HaveOccurred(), "Group list should succeed consistently") - outputs = append(outputs, string(output)) + Expect(err).ToNot(HaveOccurred(), "Group list should succeed on iteration %d", i+1) + outputs[i] = string(output) } - By("Verifying output format consistency") + By("Verifying outputs are consistent") for i := 1; i < len(outputs); i++ { - // Extract group names from outputs (skip count line) groups1 := extractGroupNames(outputs[i-1]) groups2 := extractGroupNames(outputs[i]) @@ -218,7 +216,8 @@ var _ = Describe("Group List E2E", func() { }) It("should handle mixed alphanumeric group names correctly", func() { - By("Creating test groups with mixed alphanumeric names") + // Create unique base name for this test + baseGroupName := "e2e-test-group-" + time.Now().Format("20060102150405") + "-" + fmt.Sprintf("%d", time.Now().UnixNano()%1000000) mixedGroupNames := []string{ "group-123", "group-abc", @@ -231,9 +230,23 @@ var _ = Describe("Group List E2E", func() { "testgroup2", } + // Create full group names + fullGroupNames := make([]string, len(mixedGroupNames)) + for i, mixedName := range mixedGroupNames { + fullGroupNames[i] = baseGroupName + "-" + mixedName + } + + // Clean up all groups created by this test + defer func() { + for _, groupName := range fullGroupNames { + cleanupSpecificGroup(groupName) + } + }() + + By("Creating test groups with mixed alphanumeric names") // Create groups with mixed names - for _, groupName := range mixedGroupNames { - createCmd := exec.Command(thvBinary, "group", "create", testGroupName+"-"+groupName) + for _, groupName := range fullGroupNames { + createCmd := exec.Command(thvBinary, "group", "create", groupName) createOutput, err := createCmd.CombinedOutput() Expect(err).ToNot(HaveOccurred(), "Group creation should succeed for %s", groupName) Expect(string(createOutput)).To(ContainSubstring("created successfully")) @@ -250,8 +263,8 @@ var _ = Describe("Group List E2E", func() { // Find our test groups in the output var testGroups []string for _, group := range groups { - for _, mixedName := range mixedGroupNames { - if strings.Contains(group, testGroupName+"-"+mixedName) { + for _, fullName := range fullGroupNames { + if group == fullName { testGroups = append(testGroups, group) break } @@ -259,7 +272,7 @@ var _ = Describe("Group List E2E", func() { } By("Verifying test groups are in alphanumeric order") - Expect(len(testGroups)).To(Equal(len(mixedGroupNames)), "All test groups should be found") + Expect(len(testGroups)).To(Equal(len(fullGroupNames)), "All test groups should be found") // Check that our test groups are sorted correctly for i := 1; i < len(testGroups); i++ { @@ -267,9 +280,6 @@ var _ = Describe("Group List E2E", func() { "Test group '%s' should come before or equal to '%s' in alphanumeric order", testGroups[i-1], testGroups[i]) } - - // Note: Cleanup is not implemented yet - // TODO: Implement group delete command for proper cleanup }) }) }) @@ -289,4 +299,3 @@ func extractGroupNames(output string) []string { return groups } -*/ diff --git a/test/e2e/group_rm_test.go b/test/e2e/group_rm_test.go index 9f356f720..e456c8f02 100644 --- a/test/e2e/group_rm_test.go +++ b/test/e2e/group_rm_test.go @@ -1,7 +1,5 @@ package e2e -// TODO: add back in once we have a working group command, and update the docs -/* import ( "fmt" "os/exec" @@ -12,8 +10,8 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("Group RM E2E Tests", func() { - var config *TestConfig +var _ = Describe("Group Remove Command E2E Tests", func() { + var config *testConfig BeforeEach(func() { config = NewTestConfig() @@ -28,97 +26,48 @@ var _ = Describe("Group RM E2E Tests", func() { stdout, stderr := NewTHVCommand(config, "group", "rm", "--help").ExpectSuccess() output := stdout + stderr Expect(output).To(ContainSubstring("Remove a group and remove all MCP servers from it")) - Expect(output).To(ContainSubstring("By default, this only removes the group membership from workloads without deleting them")) - Expect(output).To(ContainSubstring("Use --with-workloads to also delete the workloads")) - Expect(output).To(ContainSubstring("The command will show a warning and require user confirmation")) - Expect(output).To(ContainSubstring("Usage:")) - Expect(output).To(ContainSubstring("thv group rm [group-name]")) + Expect(output).To(ContainSubstring("--with-workloads")) }) It("should return error when group does not exist", func() { - groupName := fmt.Sprintf("group-rm-non-existent-group-%d", time.Now().UnixNano()) + groupName := fmt.Sprintf("group-rm-non-existent-%d", time.Now().UnixNano()) _, stderr, err := NewTHVCommand(config, "group", "rm", groupName).ExpectFailure() Expect(err).To(HaveOccurred()) Expect(stderr).To(ContainSubstring("does not exist")) }) - It("should cancel deletion when user does not confirm", func() { - groupName := fmt.Sprintf("group-rm-cancel-group-%d", time.Now().UnixNano()) - createGroup(config, groupName) - - // Try to delete the group but provide 'n' for no - cmd := exec.Command(config.THVBinary, "group", "rm", groupName) - cmd.Stdin = strings.NewReader("n\n") - output, err := cmd.CombinedOutput() - Expect(err).NotTo(HaveOccurred()) - Expect(string(output)).To(ContainSubstring("Group deletion cancelled.")) - - // Verify group still exists - stdout, _ := NewTHVCommand(config, "group", "list").ExpectSuccess() - Expect(stdout).To(ContainSubstring(groupName)) - }) - It("should delete empty group successfully", func() { - // Create a group groupName := fmt.Sprintf("group-rm-empty-group-%d", time.Now().UnixNano()) - createGroup(config, groupName) - - // Verify group exists - stdout, _ := NewTHVCommand(config, "group", "list").ExpectSuccess() - Expect(stdout).To(ContainSubstring(groupName)) - - // Delete the group (provide confirmation) - cmd := exec.Command(config.THVBinary, "group", "rm", groupName) - cmd.Stdin = strings.NewReader("y\n") - output, err := cmd.CombinedOutput() - Expect(err).NotTo(HaveOccurred()) - Expect(string(output)).To(ContainSubstring("WARNING:")) - Expect(string(output)).To(ContainSubstring(fmt.Sprintf("Group '%s' deleted successfully", groupName))) - // Verify group is deleted - stdout, _ = NewTHVCommand(config, "group", "list").ExpectSuccess() - Expect(stdout).NotTo(ContainSubstring(groupName)) - }) + // Clean up the group after the test (in case it wasn't deleted) + defer cleanupSpecificGroup(groupName) - It("should delete group with single workload", func() { - // Create a group - groupName := fmt.Sprintf("group-rm-single-workload-group-%d", time.Now().UnixNano()) createGroup(config, groupName) - // Create a workload in the group - workloadName := fmt.Sprintf("group-rm-test-workload-%d", time.Now().UnixNano()) - createWorkloadInGroup(config, workloadName, groupName) - - // Verify the workload is running - Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload did not appear in thv list within 3 seconds") - // Delete the group (provide confirmation) cmd := exec.Command(config.THVBinary, "group", "rm", groupName) cmd.Stdin = strings.NewReader("y\n") output, err := cmd.CombinedOutput() Expect(err).NotTo(HaveOccurred()) - Expect(string(output)).To(ContainSubstring("WARNING:")) - Expect(string(output)).To(ContainSubstring("Removed 1 workload(s) from group")) - Expect(string(output)).To(ContainSubstring(fmt.Sprintf("Group '%s' deleted successfully", groupName))) - - // Verify workload still exists (not deleted by default) - stdout, _ := NewTHVCommand(config, "list").ExpectSuccess() - Expect(stdout).To(ContainSubstring(workloadName)) + Expect(string(output)).To(ContainSubstring("deleted successfully")) // Verify group is deleted - stdout, _ = NewTHVCommand(config, "group", "list").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "group", "list").ExpectSuccess() Expect(stdout).NotTo(ContainSubstring(groupName)) }) - It("should delete group with multiple workloads", func() { - // Create a group - groupName := fmt.Sprintf("group-rm-multi-workload-group-%d", time.Now().UnixNano()) + It("should delete group and move workloads to default group", func() { + groupName := fmt.Sprintf("group-rm-move-workloads-group-%d", time.Now().UnixNano()) + + // Clean up the group after the test (in case it wasn't deleted) + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) // Create multiple workloads in the group - workload1 := fmt.Sprintf("group-rm-test-workload-1-%d", time.Now().UnixNano()) - workload2 := fmt.Sprintf("group-rm-test-workload-2-%d", time.Now().UnixNano()) - workload3 := fmt.Sprintf("group-rm-test-workload-3-%d", time.Now().UnixNano()) + workload1 := fmt.Sprintf("group-rm-workload-1-%d", time.Now().UnixNano()) + workload2 := fmt.Sprintf("group-rm-workload-2-%d", time.Now().UnixNano()) + workload3 := fmt.Sprintf("group-rm-workload-3-%d", time.Now().UnixNano()) createWorkloadInGroup(config, workload1, groupName) createWorkloadInGroup(config, workload2, groupName) @@ -136,7 +85,7 @@ var _ = Describe("Group RM E2E Tests", func() { Expect(err).NotTo(HaveOccurred()) Expect(string(output)).To(ContainSubstring("WARNING:")) Expect(string(output)).To(ContainSubstring("Removed 3 workload(s) from group")) - Expect(string(output)).To(ContainSubstring(fmt.Sprintf("Group '%s' deleted successfully", groupName))) + Expect(string(output)).To(ContainSubstring("deleted successfully")) // Verify workloads still exist (not deleted by default) stdout, _ := NewTHVCommand(config, "list").ExpectSuccess() @@ -152,6 +101,10 @@ var _ = Describe("Group RM E2E Tests", func() { It("should handle mixed workloads (some in group, some not)", func() { // Create a group groupName := fmt.Sprintf("group-rm-mixed-group-%d", time.Now().UnixNano()) + + // Clean up the group after the test (in case it wasn't deleted) + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) // Create workloads in the group @@ -202,6 +155,10 @@ var _ = Describe("Group RM E2E Tests", func() { It("should delete group and workloads with --with-workloads flag", func() { // Create a group groupName := fmt.Sprintf("group-rm-with-workloads-group-%d", time.Now().UnixNano()) + + // Clean up the group after the test (in case it wasn't deleted) + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) // Create multiple workloads in the group @@ -236,4 +193,3 @@ var _ = Describe("Group RM E2E Tests", func() { }) }) }) -*/ diff --git a/test/e2e/group_test.go b/test/e2e/group_test.go index 5ed994e2c..2f694bf7e 100644 --- a/test/e2e/group_test.go +++ b/test/e2e/group_test.go @@ -1,7 +1,5 @@ -package e2e_test +package e2e -// TODO: add back in once we have a working group command -/* import ( "context" "fmt" @@ -15,7 +13,6 @@ import ( "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/logger" - "github.com/stacklok/toolhive/test/e2e" ) func init() { @@ -24,14 +21,14 @@ func init() { var _ = Describe("Group", func() { var ( - config *e2e.TestConfig + config *testConfig groupName string thvBinary string sharedTimestamp int64 ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Use a shared timestamp for all workload names in this test sharedTimestamp = time.Now().UnixNano() // Use a more unique group name to avoid conflicts between tests @@ -42,7 +39,7 @@ var _ = Describe("Group", func() { } // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -120,7 +117,7 @@ var _ = Describe("Group", func() { cmd := exec.Command(thvBinary, "group", "create", "invalid/group/name") output, err := cmd.CombinedOutput() Expect(err).To(HaveOccurred(), "Should fail with invalid group name") - Expect(string(output)).To(ContainSubstring("failed to create file")) + Expect(string(output)).To(ContainSubstring("invalid group name")) }) }) @@ -460,4 +457,3 @@ var _ = Describe("Group", func() { }) }) }) -*/ diff --git a/test/e2e/helpers.go b/test/e2e/helpers.go index 4f59346b1..bda5fbad9 100644 --- a/test/e2e/helpers.go +++ b/test/e2e/helpers.go @@ -11,41 +11,44 @@ import ( . "github.com/onsi/ginkgo/v2" //nolint:staticcheck // Standard practice for Ginkgo . "github.com/onsi/gomega" //nolint:staticcheck // Standard practice for Gomega + + "github.com/stacklok/toolhive/pkg/groups" + "github.com/stacklok/toolhive/pkg/workloads" ) -// TestConfig holds configuration for e2e tests -type TestConfig struct { +// testConfig holds configuration for e2e tests +type testConfig struct { THVBinary string TestTimeout time.Duration CleanupAfter bool } // NewTestConfig creates a new test configuration with defaults -func NewTestConfig() *TestConfig { +func NewTestConfig() *testConfig { // Look for thv binary in PATH or use a configurable path thvBinary := os.Getenv("THV_BINARY") if thvBinary == "" { thvBinary = "thv" // Assume it's in PATH } - return &TestConfig{ + return &testConfig{ THVBinary: thvBinary, TestTimeout: 10 * time.Minute, CleanupAfter: true, } } -// THVCommand represents a ToolHive CLI command execution -type THVCommand struct { - config *TestConfig +// thvCommand represents a ToolHive CLI command execution +type thvCommand struct { + config *testConfig args []string env []string dir string } // NewTHVCommand creates a new ToolHive command -func NewTHVCommand(config *TestConfig, args ...string) *THVCommand { - return &THVCommand{ +func NewTHVCommand(config *testConfig, args ...string) *thvCommand { + return &thvCommand{ config: config, args: args, env: os.Environ(), @@ -54,24 +57,24 @@ func NewTHVCommand(config *TestConfig, args ...string) *THVCommand { } // WithEnv adds environment variables to the command -func (c *THVCommand) WithEnv(env ...string) *THVCommand { +func (c *thvCommand) WithEnv(env ...string) *thvCommand { c.env = append(c.env, env...) return c } // WithDir sets the working directory for the command -func (c *THVCommand) WithDir(dir string) *THVCommand { +func (c *thvCommand) WithDir(dir string) *thvCommand { c.dir = dir return c } // Run executes the ToolHive command and returns stdout, stderr, and error -func (c *THVCommand) Run() (string, string, error) { +func (c *thvCommand) Run() (string, string, error) { return c.RunWithTimeout(c.config.TestTimeout) } // RunWithTimeout executes the ToolHive command with a specific timeout -func (c *THVCommand) RunWithTimeout(timeout time.Duration) (string, string, error) { +func (c *thvCommand) RunWithTimeout(timeout time.Duration) (string, string, error) { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -91,7 +94,7 @@ func (c *THVCommand) RunWithTimeout(timeout time.Duration) (string, string, erro } // ExpectSuccess runs the command and expects it to succeed -func (c *THVCommand) ExpectSuccess() (string, string) { +func (c *thvCommand) ExpectSuccess() (string, string) { stdout, stderr, err := c.Run() if err != nil { // Log the command that failed for debugging @@ -104,15 +107,15 @@ func (c *THVCommand) ExpectSuccess() (string, string) { } // ExpectFailure runs the command and expects it to fail -func (c *THVCommand) ExpectFailure() (string, string, error) { +func (c *thvCommand) ExpectFailure() (string, string, error) { stdout, stderr, err := c.Run() ExpectWithOffset(1, err).To(HaveOccurred(), fmt.Sprintf("Command should have failed but succeeded\nStdout: %s\nStderr: %s", stdout, stderr)) return stdout, stderr, err } -// WaitForMCPServer waits for an MCP server to be running -func WaitForMCPServer(config *TestConfig, serverName string, timeout time.Duration) error { +// waitForMCPServer waits for an MCP server to be running +func waitForMCPServer(config *testConfig, serverName string, timeout time.Duration) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -137,9 +140,9 @@ func WaitForMCPServer(config *TestConfig, serverName string, timeout time.Durati } } -// StopAndRemoveMCPServer stops and removes an MCP server +// stopAndRemoveMCPServer stops and removes an MCP server // This function is designed for cleanup and tolerates servers that don't exist -func StopAndRemoveMCPServer(config *TestConfig, serverName string) error { +func stopAndRemoveMCPServer(config *testConfig, serverName string) error { // Try to stop the server first (ignore errors as server might not exist) _, _, _ = NewTHVCommand(config, "stop", serverName).Run() @@ -155,8 +158,8 @@ func StopAndRemoveMCPServer(config *TestConfig, serverName string) error { return nil } -// GetMCPServerURL gets the URL for an MCP server -func GetMCPServerURL(config *TestConfig, serverName string) (string, error) { +// getMCPServerURL gets the URL for an MCP server +func getMCPServerURL(config *testConfig, serverName string) (string, error) { stdout, stderr, err := NewTHVCommand(config, "list").Run() if err != nil { GinkgoWriter.Printf("Failed to list servers: %v\nStdout: %s\nStderr: %s\n", err, stdout, stderr) @@ -184,8 +187,8 @@ func GetMCPServerURL(config *TestConfig, serverName string) (string, error) { return "", fmt.Errorf("could not find URL for server %s in output: %s", serverName, stdout) } -// GetServerLogs gets the logs for a server to help with debugging -func GetServerLogs(config *TestConfig, serverName string) (string, error) { +// getServerLogs gets the logs for a server to help with debugging +func getServerLogs(config *testConfig, serverName string) (string, error) { stdout, stderr, err := NewTHVCommand(config, "logs", serverName).Run() if err != nil { return "", fmt.Errorf("failed to get logs for %s: %w (stderr: %s)", serverName, err, stderr) @@ -193,8 +196,8 @@ func GetServerLogs(config *TestConfig, serverName string) (string, error) { return stdout, nil } -// DebugServerState prints debugging information about a server -func DebugServerState(config *TestConfig, serverName string) { +// debugServerState prints debugging information about a server +func debugServerState(config *testConfig, serverName string) { GinkgoWriter.Printf("=== Debugging server state for %s ===\n", serverName) // Get list output @@ -202,7 +205,7 @@ func DebugServerState(config *TestConfig, serverName string) { GinkgoWriter.Printf("thv list output:\nStdout: %s\nStderr: %s\nError: %v\n", stdout, stderr, err) // Get logs - logs, err := GetServerLogs(config, serverName) + logs, err := getServerLogs(config, serverName) if err != nil { GinkgoWriter.Printf("Failed to get logs: %v\n", err) } else { @@ -213,7 +216,7 @@ func DebugServerState(config *TestConfig, serverName string) { } // CheckTHVBinaryAvailable checks if the thv binary is available -func CheckTHVBinaryAvailable(config *TestConfig) error { +func CheckTHVBinaryAvailable(config *testConfig) error { _, _, err := NewTHVCommand(config, "--help").Run() if err != nil { return fmt.Errorf("thv binary not available at %s: %w", config.THVBinary, err) @@ -221,8 +224,8 @@ func CheckTHVBinaryAvailable(config *TestConfig) error { return nil } -// StartLongRunningTHVCommand starts a long-running ToolHive command and returns the process -func StartLongRunningTHVCommand(config *TestConfig, args ...string) *exec.Cmd { +// startLongRunningTHVCommand starts a long-running ToolHive command and returns the process +func startLongRunningTHVCommand(config *testConfig, args ...string) *exec.Cmd { cmd := exec.Command(config.THVBinary, args...) //nolint:gosec // Intentional for e2e testing cmd.Env = os.Environ() @@ -237,9 +240,98 @@ func StartLongRunningTHVCommand(config *TestConfig, args ...string) *exec.Cmd { return cmd } -// StartDockerCommand starts a docker command with proper environment setup and returns the command -func StartDockerCommand(args ...string) *exec.Cmd { +// startDockerCommand starts a docker command with proper environment setup and returns the command +func startDockerCommand(args ...string) *exec.Cmd { cmd := exec.Command("docker", args...) //nolint:gosec // Intentional for e2e testing cmd.Env = os.Environ() return cmd } + +// Helper function to clean up a specific group and its workloads +func cleanupSpecificGroup(groupName string) { + // Use the groups manager to delete the specific group and its workloads + groupManager, err := groups.NewManager() + if err != nil { + // If we can't create the group manager, we can't clean up, so just return + return + } + + ctx := context.Background() + + // Check if the group exists before trying to delete it + exists, err := groupManager.Exists(ctx, groupName) + if err != nil || !exists { + // Group doesn't exist or we can't check, so nothing to clean up + return + } + + // Get all workloads in the group + groupWorkloads, err := groupManager.ListWorkloadsInGroup(ctx, groupName) + if err != nil { + // If we can't list workloads, just try to delete the group + _ = groupManager.Delete(ctx, groupName) + return + } + + // If there are workloads in the group, delete them first + if len(groupWorkloads) > 0 { + workloadManager, err := workloads.NewManager(ctx) + if err != nil { + // If we can't create the workload manager, just try to delete the group + _ = groupManager.Delete(ctx, groupName) + return + } + + // Delete all workloads in the group + group, err := workloadManager.DeleteWorkloads(ctx, groupWorkloads) + if err != nil { + // If we can't delete workloads, just try to delete the group + _ = groupManager.Delete(ctx, groupName) + return + } + + // Wait for all workload deletions to complete + if err := group.Wait(); err != nil { + // If workload deletion failed, just try to delete the group + _ = groupManager.Delete(ctx, groupName) + return + } + } + + // Delete the group itself + _ = groupManager.Delete(ctx, groupName) // Ignore errors during cleanup +} + +// Helper functions for group and workload management + +func createGroup(config *testConfig, groupName string) { + NewTHVCommand(config, "group", "create", groupName).ExpectSuccess() +} + +func createWorkloadInGroup(config *testConfig, workloadName, groupName string) { + NewTHVCommand(config, "run", "fetch", "--group", groupName, "--name", workloadName).ExpectSuccess() +} + +func createWorkload(config *testConfig, workloadName string) { + NewTHVCommand(config, "run", "fetch", "--name", workloadName).ExpectSuccess() +} + +func removeWorkload(config *testConfig, workloadName string) { + NewTHVCommand(config, "rm", workloadName).ExpectSuccess() +} + +func isWorkloadRunning(config *testConfig, workloadName string) bool { + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() + return strings.Contains(stdout, workloadName) +} + +func waitForWorkload(config *testConfig, workloadName string) bool { + deadline := time.Now().Add(10 * time.Second) + for time.Now().Before(deadline) { + if isWorkloadRunning(config, workloadName) { + return true + } + time.Sleep(200 * time.Millisecond) + } + return false +} diff --git a/test/e2e/inspector_test.go b/test/e2e/inspector_test.go index 2d6175451..dc69e9968 100644 --- a/test/e2e/inspector_test.go +++ b/test/e2e/inspector_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "fmt" @@ -8,13 +8,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) // inspectorTestHelper contains common functionality for inspector tests type inspectorTestHelper struct { - config *e2e.TestConfig + config *testConfig mcpServerName string inspectorName string client *http.Client @@ -23,27 +21,27 @@ type inspectorTestHelper struct { var _ = Describe("Inspector", func() { var ( - config *e2e.TestConfig + config *testConfig mcpServerName string inspectorName string ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() mcpServerName = fmt.Sprintf("mcp-server-%d", GinkgoRandomSeed()) inspectorName = "inspector" // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) AfterEach(func() { if config.CleanupAfter { // Clean up both servers - err := e2e.StopAndRemoveMCPServer(config, inspectorName) + err := stopAndRemoveMCPServer(config, inspectorName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove inspector") - err = e2e.StopAndRemoveMCPServer(config, mcpServerName) + err = stopAndRemoveMCPServer(config, mcpServerName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove MCP server") } }) @@ -52,19 +50,19 @@ var _ = Describe("Inspector", func() { Context("when providing invalid arguments", func() { It("should fail when no server name is provided", func() { By("Running inspector without server name") - _, _, err := e2e.NewTHVCommand(config, "inspector").ExpectFailure() + _, _, err := NewTHVCommand(config, "inspector").ExpectFailure() Expect(err).To(HaveOccurred(), "Should fail without server name") }) It("should fail when too many arguments are provided", func() { By("Running inspector with multiple server names") - _, _, err := e2e.NewTHVCommand(config, "inspector", "server1", "server2").ExpectFailure() + _, _, err := NewTHVCommand(config, "inspector", "server1", "server2").ExpectFailure() Expect(err).To(HaveOccurred(), "Should fail with multiple server names") }) It("should fail when server doesn't exist", func() { By("Running inspector with non-existent server") - _, stderr, err := e2e.NewTHVCommand(config, "inspector", "non-existent-server"). + _, stderr, err := NewTHVCommand(config, "inspector", "non-existent-server"). RunWithTimeout(10 * time.Second) Expect(err).To(HaveOccurred(), "Should fail with non-existent server") Expect(stderr).To(ContainSubstring("not found"), "Should indicate server not found") @@ -74,7 +72,7 @@ var _ = Describe("Inspector", func() { Context("when checking help and flags", func() { It("should show help information", func() { By("Getting inspector help") - stdout, _ := e2e.NewTHVCommand(config, "inspector", "--help").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "inspector", "--help").ExpectSuccess() Expect(stdout).To(ContainSubstring("MCP Inspector UI"), "Should mention Inspector UI") Expect(stdout).To(ContainSubstring("--ui-port"), "Should show ui-port flag") Expect(stdout).To(ContainSubstring("--mcp-proxy-port"), "Should show mcp-proxy-port flag") @@ -82,7 +80,7 @@ var _ = Describe("Inspector", func() { It("should accept custom ports", func() { By("Running inspector with custom ports (should fail due to missing server)") - _, stderr, err := e2e.NewTHVCommand(config, "inspector", + _, stderr, err := NewTHVCommand(config, "inspector", "--ui-port", "8080", "--mcp-proxy-port", "8081", "non-existent-server").RunWithTimeout(10 * time.Second) @@ -104,9 +102,9 @@ var _ = Describe("Inspector", func() { AfterEach(func() { if config.CleanupAfter { // Clean up both servers - err := e2e.StopAndRemoveMCPServer(config, inspectorName) + err := stopAndRemoveMCPServer(config, inspectorName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove inspector") - err = e2e.StopAndRemoveMCPServer(config, mcpServerName) + err = stopAndRemoveMCPServer(config, mcpServerName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove MCP server") } }) @@ -114,7 +112,7 @@ var _ = Describe("Inspector", func() { Context("when launching inspector", func() { It("should successfully start inspector UI", func() { By("Starting the inspector") - stdout, stderr, err := e2e.NewTHVCommand(config, "inspector", mcpServerName). + stdout, stderr, err := NewTHVCommand(config, "inspector", mcpServerName). RunWithTimeout(15 * time.Second) output := stdout + stderr @@ -156,7 +154,7 @@ var _ = Describe("Inspector", func() { It("should use custom UI port when specified", func() { By("Starting inspector with custom UI port") customUIPort := "9999" - stdout, stderr, err := e2e.NewTHVCommand(config, "inspector", + stdout, stderr, err := NewTHVCommand(config, "inspector", "--ui-port", customUIPort, mcpServerName).RunWithTimeout(10 * time.Second) @@ -194,7 +192,7 @@ var _ = Describe("Inspector", func() { helper.verifyInspectorUIUnavailable() By("Verifying no orphaned inspector containers remain") - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() Expect(stdout).ToNot(BeNil(), "Should get valid list output") }) }) @@ -202,7 +200,7 @@ var _ = Describe("Inspector", func() { }) // newInspectorTestHelper creates a new inspector test helper -func newInspectorTestHelper(config *e2e.TestConfig, mcpServerName, inspectorName string) *inspectorTestHelper { +func newInspectorTestHelper(config *testConfig, mcpServerName, inspectorName string) *inspectorTestHelper { return &inspectorTestHelper{ config: config, mcpServerName: mcpServerName, @@ -218,7 +216,7 @@ func (h *inspectorTestHelper) startInspectorInBackground(timeout time.Duration, go func() { cmdArgs := append([]string{"inspector"}, args...) cmdArgs = append(cmdArgs, h.mcpServerName) - _, _, err := e2e.NewTHVCommand(h.config, cmdArgs...).RunWithTimeout(timeout) + _, _, err := NewTHVCommand(h.config, cmdArgs...).RunWithTimeout(timeout) done <- err }() return done @@ -278,7 +276,7 @@ func (*inspectorTestHelper) waitForInspectorCompletion(done chan error, timeout // cleanupInspector performs cleanup of inspector containers func (h *inspectorTestHelper) cleanupInspector() { - err := e2e.StopAndRemoveMCPServer(h.config, h.inspectorName) + err := stopAndRemoveMCPServer(h.config, h.inspectorName) if err != nil { GinkgoWriter.Printf("Note: Cleanup returned error (may be expected): %v\n", err) } @@ -288,7 +286,7 @@ func (h *inspectorTestHelper) cleanupInspector() { // setupMCPServer starts an MCP server and waits for it to be ready func (h *inspectorTestHelper) setupMCPServer() { By("Starting an MCP server for inspector to connect to") - e2e.NewTHVCommand(h.config, "run", "--name", h.mcpServerName, "fetch").ExpectSuccess() - err := e2e.WaitForMCPServer(h.config, h.mcpServerName, 60*time.Second) + NewTHVCommand(h.config, "run", "--name", h.mcpServerName, "fetch").ExpectSuccess() + err := waitForMCPServer(h.config, h.mcpServerName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "MCP server should be running") } diff --git a/test/e2e/list_group_e2e_test.go b/test/e2e/list_group_e2e_test.go index 0a7b6805e..bf0ad1b13 100644 --- a/test/e2e/list_group_e2e_test.go +++ b/test/e2e/list_group_e2e_test.go @@ -1,7 +1,5 @@ -package e2e_test +package e2e -// TODO: Re-enable when group functionality is complete -/* import ( "context" "fmt" @@ -15,7 +13,6 @@ import ( "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/logger" - "github.com/stacklok/toolhive/test/e2e" ) func init() { @@ -24,14 +21,14 @@ func init() { var _ = Describe("List Group", func() { var ( - config *e2e.TestConfig + config *testConfig groupName string thvBinary string sharedTimestamp int64 ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Use a shared timestamp for all workload names in this test sharedTimestamp = time.Now().UnixNano() // Use a more unique group name to avoid conflicts between tests @@ -42,7 +39,7 @@ var _ = Describe("List Group", func() { } // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -85,11 +82,26 @@ var _ = Describe("List Group", func() { output, err := cmd.CombinedOutput() Expect(err).ToNot(HaveOccurred(), "Failed to add workload: %s", string(output)) - // Wait for workload to be fully registered - time.Sleep(3 * time.Second) + // Wait for workload to be fully registered and appear in the group + time.Sleep(5 * time.Second) }) It("should show only workloads in the specified group", func() { + // Wait for workload to appear in the group + By("Waiting for workload to appear in group") + deadline := time.Now().Add(10 * time.Second) + found := false + for time.Now().Before(deadline) { + cmd := exec.Command(thvBinary, "list", "--group", groupName) + output, err := cmd.CombinedOutput() + if err == nil && strings.Contains(string(output), workloadName) { + found = true + break + } + time.Sleep(500 * time.Millisecond) + } + Expect(found).To(BeTrue(), "Workload should appear in group within 10 seconds") + By("Listing workloads in the group") cmd := exec.Command(thvBinary, "list", "--group", groupName) output, err := cmd.CombinedOutput() @@ -344,8 +356,7 @@ var _ = Describe("List Group", func() { outputStr := string(output) Expect(outputStr).To(ContainSubstring("--group")) - Expect(outputStr).To(ContainSubstring("Filter workloads by group name")) + Expect(outputStr).To(ContainSubstring("Filter workloads by group")) }) }) }) -*/ diff --git a/test/e2e/mcp_client_helpers.go b/test/e2e/mcp_client_helpers.go index 538b58a39..b65b3f5dc 100644 --- a/test/e2e/mcp_client_helpers.go +++ b/test/e2e/mcp_client_helpers.go @@ -15,11 +15,11 @@ import ( // MCPClientHelper provides high-level MCP client operations for e2e tests type MCPClientHelper struct { client *client.Client - config *TestConfig + config *testConfig } // NewMCPClientForSSE creates a new MCP client for SSE transport -func NewMCPClientForSSE(config *TestConfig, serverURL string) (*MCPClientHelper, error) { +func NewMCPClientForSSE(config *testConfig, serverURL string) (*MCPClientHelper, error) { mcpClient, err := client.NewSSEMCPClient(serverURL) if err != nil { return nil, fmt.Errorf("failed to create SSE MCP client: %w", err) @@ -32,7 +32,7 @@ func NewMCPClientForSSE(config *TestConfig, serverURL string) (*MCPClientHelper, } // NewMCPClientForStreamableHTTP creates a new MCP client for streamable HTTP transport -func NewMCPClientForStreamableHTTP(config *TestConfig, serverURL string) (*MCPClientHelper, error) { +func NewMCPClientForStreamableHTTP(config *testConfig, serverURL string) (*MCPClientHelper, error) { mcpClient, err := client.NewStreamableHttpClient(serverURL) if err != nil { return nil, fmt.Errorf("failed to create Streamable HTTP MCP client: %w", err) @@ -149,7 +149,7 @@ func (h *MCPClientHelper) ExpectResourceExists(ctx context.Context, uri string) } // WaitForMCPServerReady waits for an MCP server to be ready and responsive -func WaitForMCPServerReady(config *TestConfig, serverURL string, mode string, timeout time.Duration) error { +func WaitForMCPServerReady(config *testConfig, serverURL string, mode string, timeout time.Duration) error { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -164,7 +164,7 @@ func WaitForMCPServerReady(config *TestConfig, serverURL string, mode string, ti case <-ctx.Done(): // Before timing out, debug the server state GinkgoWriter.Printf("MCP server connection timed out, debugging server state...\n") - DebugServerState(config, serverName) + debugServerState(config, serverName) return fmt.Errorf("timeout waiting for MCP server to be ready at %s", serverURL) case <-ticker.C: @@ -211,7 +211,7 @@ func extractServerNameFromURL(serverURL string) string { } // TestMCPServerBasicFunctionality tests basic MCP server functionality -func TestMCPServerBasicFunctionality(config *TestConfig, serverURL string) error { +func TestMCPServerBasicFunctionality(config *testConfig, serverURL string) error { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() diff --git a/test/e2e/osv_authz_test.go b/test/e2e/osv_authz_test.go index 61afa973c..2a5918e1d 100644 --- a/test/e2e/osv_authz_test.go +++ b/test/e2e/osv_authz_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "context" @@ -13,18 +13,16 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("OSV MCP Server with Authorization", Serial, func() { - var config *e2e.TestConfig + var config *testConfig BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -32,7 +30,7 @@ var _ = Describe("OSV MCP Server with Authorization", Serial, func() { Context("when authorization allows only one tool call for anybody", Ordered, func() { var serverName string var authzConfigPath string - var mcpClient *e2e.MCPClientHelper + var mcpClient *MCPClientHelper var serverURL string var cancel context.CancelFunc @@ -61,28 +59,28 @@ var _ = Describe("OSV MCP Server with Authorization", Serial, func() { Expect(err).ToNot(HaveOccurred()) // Start ONE server for ALL tests in this context with metrics enabled - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "--authz-config", authzConfigPath, "--otel-enable-prometheus-metrics-path", "osv").ExpectSuccess() - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) // Get server URL - serverURL, err = e2e.GetMCPServerURL(config, serverName) + serverURL, err = getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) - err = e2e.WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) BeforeEach(func() { // Create fresh MCP client for each test var err error - mcpClient, err = e2e.NewMCPClientForSSE(config, serverURL) + mcpClient, err = NewMCPClientForSSE(config, serverURL) Expect(err).ToNot(HaveOccurred()) // Create context that will be cancelled in AfterEach @@ -104,7 +102,7 @@ var _ = Describe("OSV MCP Server with Authorization", Serial, func() { AfterAll(func() { if config.CleanupAfter { // Clean up the shared server after all tests - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") // Clean up the temporary config file diff --git a/test/e2e/osv_mcp_server_test.go b/test/e2e/osv_mcp_server_test.go index 2c1d90f03..784414feb 100644 --- a/test/e2e/osv_mcp_server_test.go +++ b/test/e2e/osv_mcp_server_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "context" @@ -9,8 +9,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) // generateUniqueServerName creates a unique server name for OSV tests @@ -19,13 +17,13 @@ func generateUniqueServerName(prefix string) string { } var _ = Describe("OsvMcpServer", Serial, func() { - var config *e2e.TestConfig + var config *testConfig BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -40,14 +38,14 @@ var _ = Describe("OsvMcpServer", Serial, func() { AfterEach(func() { if config.CleanupAfter { // Clean up the server after each test in this context - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should successfully start and be accessible via SSE [Serial]", func() { By("Starting the OSV MCP server with SSE transport") - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "osv").ExpectSuccess() @@ -56,11 +54,11 @@ var _ = Describe("OsvMcpServer", Serial, func() { Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the OSV server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") By("Verifying the server appears in the list with SSE transport") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") Expect(stdout).To(ContainSubstring("sse"), "Server should show SSE transport") @@ -68,17 +66,17 @@ var _ = Describe("OsvMcpServer", Serial, func() { It("should be accessible via HTTP SSE endpoint [Serial]", func() { By("Starting the OSV MCP server") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "osv").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL") Expect(serverURL).To(ContainSubstring("http"), "URL should be HTTP-based") Expect(serverURL).To(ContainSubstring("/sse"), "URL should contain SSE endpoint") @@ -114,25 +112,25 @@ var _ = Describe("OsvMcpServer", Serial, func() { It("should respond to proper MCP protocol operations [Serial]", func() { By("Starting the OSV MCP server") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "osv").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) By("Waiting for MCP server to be ready") - err = e2e.WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) Expect(err).ToNot(HaveOccurred(), "MCP server should be ready for protocol operations") By("Creating MCP client and initializing connection") - mcpClient, err := e2e.NewMCPClientForSSE(config, serverURL) + mcpClient, err := NewMCPClientForSSE(config, serverURL) Expect(err).ToNot(HaveOccurred(), "Should be able to create MCP client") defer mcpClient.Close() @@ -159,7 +157,7 @@ var _ = Describe("OsvMcpServer", Serial, func() { }) Context("when testing OSV-specific functionality", Ordered, func() { - var mcpClient *e2e.MCPClientHelper + var mcpClient *MCPClientHelper var serverURL string var cancel context.CancelFunc var serverName string @@ -169,25 +167,25 @@ var _ = Describe("OsvMcpServer", Serial, func() { serverName = generateUniqueServerName("osv-functionality-test") // Start ONE server for ALL OSV-specific tests - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "osv").ExpectSuccess() - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) // Get server URL - serverURL, err = e2e.GetMCPServerURL(config, serverName) + serverURL, err = getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) - err = e2e.WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "sse", 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) BeforeEach(func() { // Create fresh MCP client for each test var err error - mcpClient, err = e2e.NewMCPClientForSSE(config, serverURL) + mcpClient, err = NewMCPClientForSSE(config, serverURL) Expect(err).ToNot(HaveOccurred()) // Create context that will be cancelled in AfterEach @@ -209,14 +207,14 @@ var _ = Describe("OsvMcpServer", Serial, func() { AfterAll(func() { if config.CleanupAfter { // Clean up the shared server after all tests - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should be listed in registry with OSV-specific information [Serial]", func() { By("Getting OSV server info from registry") - stdout, _ := e2e.NewTHVCommand(config, "registry", "info", "osv").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "registry", "info", "osv").ExpectSuccess() Expect(stdout).To(ContainSubstring("osv"), "Info should be about OSV server") Expect(stdout).To(ContainSubstring("vulnerability"), "Info should mention vulnerability scanning") }) @@ -315,30 +313,30 @@ var _ = Describe("OsvMcpServer", Serial, func() { serverName = generateUniqueServerName("osv-lifecycle-test") // Start a server for lifecycle tests - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "sse", "osv").ExpectSuccess() - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { if config.CleanupAfter { // Clean up the server after each lifecycle test - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should stop the SSE server successfully [Serial]", func() { By("Stopping the server") - stdout, _ := e2e.NewTHVCommand(config, "stop", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "stop", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Output should mention the server name") By("Verifying the server is stopped") Eventually(func() string { - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() return stdout }, 10*time.Second, 1*time.Second).Should(Or( // Server should either be in exited state or completely removed @@ -349,15 +347,15 @@ var _ = Describe("OsvMcpServer", Serial, func() { It("should restart the SSE server successfully [Serial]", func() { By("Restarting the server") - stdout, _ := e2e.NewTHVCommand(config, "restart", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "restart", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) By("Waiting for the server to be running again") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Verifying SSE endpoint is accessible again") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) client := &http.Client{Timeout: 5 * time.Second} @@ -381,7 +379,7 @@ var _ = Describe("OsvMcpServer", Serial, func() { AfterEach(func() { if config.CleanupAfter { // Clean up any server that might have been created - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) @@ -390,7 +388,7 @@ var _ = Describe("OsvMcpServer", Serial, func() { By("Trying to run OSV with stdio transport") // Note: This test assumes OSV doesn't support stdio. // If it does, this test should be adjusted or removed. - stdout, stderr, err := e2e.NewTHVCommand(config, "run", + stdout, stderr, err := NewTHVCommand(config, "run", "--name", serverName, "--transport", "stdio", "osv").Run() @@ -403,7 +401,7 @@ var _ = Describe("OsvMcpServer", Serial, func() { // If it succeeded, OSV supports both transports GinkgoWriter.Printf("Note: OSV server supports stdio transport: %s\n", stdout) // Clean up the successfully started server - _ = e2e.StopAndRemoveMCPServer(config, serverName) + _ = stopAndRemoveMCPServer(config, serverName) } }) }) diff --git a/test/e2e/osv_streamable_http_mcp_server_test.go b/test/e2e/osv_streamable_http_mcp_server_test.go index 1e1ecd62b..7b2cf5e52 100644 --- a/test/e2e/osv_streamable_http_mcp_server_test.go +++ b/test/e2e/osv_streamable_http_mcp_server_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "context" @@ -7,18 +7,16 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { - var config *e2e.TestConfig + var config *testConfig BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -33,14 +31,14 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { AfterEach(func() { if config.CleanupAfter { // Clean up the server after each test in this context - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should successfully start and be accessible via Streamable HTTP [Serial]", func() { By("Starting the OSV MCP server with Streamable HTTP transport") - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--transport", "streamable-http", "osv").ExpectSuccess() @@ -49,11 +47,11 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the OSV server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") By("Verifying the server appears in the list with Streamable HTTP transport") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") Expect(stdout).To(ContainSubstring("mcp"), "Server should show mcp endpoint") @@ -61,17 +59,17 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { It("should be accessible via HTTP Streamable HTTP endpoint [Serial]", func() { By("Starting the OSV MCP server") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "streamable-http", "osv").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL") Expect(serverURL).To(ContainSubstring("http"), "URL should be HTTP-based") Expect(serverURL).To(ContainSubstring("/mcp"), "URL should contain MCP endpoint") @@ -107,25 +105,25 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { It("should respond to proper MCP protocol operations [Serial]", func() { By("Starting the OSV MCP server") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "streamable-http", "osv").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) By("Waiting for MCP server to be ready") - err = e2e.WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) Expect(err).ToNot(HaveOccurred(), "MCP server should be ready for protocol operations") By("Creating MCP client and initializing connection") - mcpClient, err := e2e.NewMCPClientForStreamableHTTP(config, serverURL) + mcpClient, err := NewMCPClientForStreamableHTTP(config, serverURL) Expect(err).ToNot(HaveOccurred(), "Should be able to create MCP client") defer mcpClient.Close() @@ -152,7 +150,7 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { }) Context("when testing OSV-specific functionality", Ordered, func() { - var mcpClient *e2e.MCPClientHelper + var mcpClient *MCPClientHelper var serverURL string var cancel context.CancelFunc var serverName string @@ -162,25 +160,25 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { serverName = generateUniqueServerName("osv-functionality-test") // Start ONE server for ALL OSV-specific tests - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "streamable-http", "osv").ExpectSuccess() - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) // Get server URL - serverURL, err = e2e.GetMCPServerURL(config, serverName) + serverURL, err = getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) - err = e2e.WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) BeforeEach(func() { // Create fresh MCP client for each test var err error - mcpClient, err = e2e.NewMCPClientForStreamableHTTP(config, serverURL) + mcpClient, err = NewMCPClientForStreamableHTTP(config, serverURL) Expect(err).ToNot(HaveOccurred()) // Create context that will be cancelled in AfterEach @@ -202,14 +200,14 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { AfterAll(func() { if config.CleanupAfter { // Clean up the shared server after all tests - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should be listed in registry with OSV-specific information [Serial]", func() { By("Getting OSV server info from registry") - stdout, _ := e2e.NewTHVCommand(config, "registry", "info", "osv").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "registry", "info", "osv").ExpectSuccess() Expect(stdout).To(ContainSubstring("osv"), "Info should be about OSV server") Expect(stdout).To(ContainSubstring("vulnerability"), "Info should mention vulnerability scanning") }) @@ -308,30 +306,30 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { serverName = generateUniqueServerName("osv-lifecycle-test") // Start a server for lifecycle tests - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--transport", "streamable-http", "osv").ExpectSuccess() - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) AfterEach(func() { if config.CleanupAfter { // Clean up the server after each lifecycle test - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should stop the Streamable HTTP server successfully [Serial]", func() { By("Stopping the server") - stdout, _ := e2e.NewTHVCommand(config, "stop", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "stop", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Output should mention the server name") By("Verifying the server is stopped") Eventually(func() string { - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() return stdout }, 10*time.Second, 1*time.Second).Should(Or( // Server should either be in exited state or completely removed @@ -342,15 +340,15 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { It("should restart the Streamable HTTP server successfully [Serial]", func() { By("Restarting the server") - stdout, _ := e2e.NewTHVCommand(config, "restart", serverName).ExpectSuccess() + stdout, _ := NewTHVCommand(config, "restart", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) By("Waiting for the server to be running again") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Verifying Streamable HTTP endpoint is accessible again") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) client := &http.Client{Timeout: 5 * time.Second} @@ -374,7 +372,7 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { AfterEach(func() { if config.CleanupAfter { // Clean up any server that might have been created - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) @@ -383,7 +381,7 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { By("Trying to run OSV with stdio transport") // Note: This test assumes OSV doesn't support stdio. // If it does, this test should be adjusted or removed. - stdout, stderr, err := e2e.NewTHVCommand(config, "run", + stdout, stderr, err := NewTHVCommand(config, "run", "--name", serverName, "--transport", "stdio", "osv").Run() @@ -396,7 +394,7 @@ var _ = Describe("OsvStreamableHttpMcpServer", Serial, func() { // If it succeeded, OSV supports both transports GinkgoWriter.Printf("Note: OSV server supports stdio transport: %s\n", stdout) // Clean up the successfully started server - _ = e2e.StopAndRemoveMCPServer(config, serverName) + _ = stopAndRemoveMCPServer(config, serverName) } }) }) diff --git a/test/e2e/proxy_oauth_test.go b/test/e2e/proxy_oauth_test.go index 6872fad9e..485c88574 100644 --- a/test/e2e/proxy_oauth_test.go +++ b/test/e2e/proxy_oauth_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "bytes" @@ -18,7 +18,6 @@ import ( . "github.com/onsi/gomega" "github.com/stacklok/toolhive/pkg/networking" - "github.com/stacklok/toolhive/test/e2e" ) // generateUniqueOIDCServerName creates a unique server name for OIDC mock tests @@ -28,10 +27,10 @@ func generateUniqueOIDCServerName(prefix string) string { var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { var ( - config *e2e.TestConfig + config *testConfig mockOIDCPort int proxyPort int - mockOIDCServer *e2e.OIDCMockServer + mockOIDCServer *OIDCMockServer proxyCmd *exec.Cmd osvServerName string proxyServerName string @@ -41,10 +40,10 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available for testing") // Generate unique names for this test run @@ -64,12 +63,12 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { By("Starting mock OIDC server") specReport := CurrentSpecReport() if strings.Contains(specReport.FullText(), "Proxy OAuth Authentication E2E") { - mockOIDCServer, err = e2e.NewOIDCMockServer( + mockOIDCServer, err = NewOIDCMockServer( mockOIDCPort, clientID, clientSecret, - e2e.WithAccessTokenLifespan(2*time.Second), + WithAccessTokenLifespan(2*time.Second), ) } else { - mockOIDCServer, err = e2e.NewOIDCMockServer(mockOIDCPort, clientID, clientSecret) + mockOIDCServer, err = NewOIDCMockServer(mockOIDCPort, clientID, clientSecret) } Expect(err).ToNot(HaveOccurred()) @@ -86,13 +85,13 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { // Start OSV MCP server that will be our target By("Starting OSV MCP server as target") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", osvServerName, "--transport", "sse", "osv").ExpectSuccess() // Wait for OSV server to be ready - err = e2e.WaitForMCPServer(config, osvServerName, 60*time.Second) + err = waitForMCPServer(config, osvServerName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) }) @@ -107,7 +106,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { // Stop and remove OSV server if config.CleanupAfter { - err := e2e.StopAndRemoveMCPServer(config, osvServerName) + err := stopAndRemoveMCPServer(config, osvServerName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } @@ -123,7 +122,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { Context("when OAuth authentication is enabled", func() { It("should successfully start proxy with OAuth configuration", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) // remove path from server url @@ -166,7 +165,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { It("should handle OAuth auto-detection when target requires authentication", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) // remove path from server url @@ -198,7 +197,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { Context("when OAuth authentication fails", func() { It("should handle invalid OAuth credentials gracefully", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) // remove path from server url @@ -241,7 +240,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { It("should handle missing OAuth issuer gracefully when remote-auth is explicitly enabled", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) // remove path from server url @@ -284,7 +283,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { It("should handle auto-detection when target server returns WWW-Authenticate header", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) // remove path from server url @@ -316,7 +315,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { Context("when testing proxy functionality with MCP protocol", func() { It("should proxy MCP requests successfully after OAuth", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) By("Extracting base URL for transparent proxy") @@ -376,14 +375,14 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { proxyURL := fmt.Sprintf("http://localhost:%d/sse", proxyPort) // Wait for proxy to be ready for MCP connections - err = e2e.WaitForMCPServerReady(config, proxyURL, "sse", 60*time.Second) + err = WaitForMCPServerReady(config, proxyURL, "sse", 60*time.Second) if err != nil { GinkgoWriter.Printf("MCP connection through proxy failed: %v\n", err) Skip("Skipping MCP test due to proxy not being ready") } By("Creating MCP client through proxy") - mcpClient, err := e2e.NewMCPClientForSSE(config, proxyURL) + mcpClient, err := NewMCPClientForSSE(config, proxyURL) Expect(err).ToNot(HaveOccurred()) defer mcpClient.Close() @@ -406,7 +405,7 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { Context("when testing proxy functionality with MCP protocol and token refresh", func() { It("should refresh token after expiry and continue MCP operations", func() { By("Getting OSV server URL") - osvServerURL, err := e2e.GetMCPServerURL(config, osvServerName) + osvServerURL, err := getMCPServerURL(config, osvServerName) Expect(err).ToNot(HaveOccurred()) By("Extracting base URL for transparent proxy") @@ -443,10 +442,10 @@ var _ = Describe("Proxy OAuth Authentication E2E", Serial, func() { By("Reconnecting via MCP to trigger token refresh") proxyURL := fmt.Sprintf("http://localhost:%d/sse", proxyPort) - err = e2e.WaitForMCPServerReady(config, proxyURL, "sse", 10*time.Second) + err = WaitForMCPServerReady(config, proxyURL, "sse", 10*time.Second) Expect(err).ToNot(HaveOccurred(), "MCP server not ready after token expiry") - mcpClient, err := e2e.NewMCPClientForSSE(config, proxyURL) + mcpClient, err := NewMCPClientForSSE(config, proxyURL) Expect(err).ToNot(HaveOccurred()) defer mcpClient.Close() @@ -480,7 +479,7 @@ func checkServerHealth(healthUrl string) error { return fmt.Errorf("server not healthy, status: %d", resp.StatusCode) } -func startProxyWithOAuth(config *e2e.TestConfig, serverName, targetURL string, port int, issuer, clientID, clientSecret string) *exec.Cmd { +func startProxyWithOAuth(config *testConfig, serverName, targetURL string, port int, issuer, clientID, clientSecret string) *exec.Cmd { args := []string{ "proxy", "--host", "localhost", @@ -510,10 +509,10 @@ func startProxyWithOAuth(config *e2e.TestConfig, serverName, targetURL string, p // Log the command for debugging GinkgoWriter.Printf("Starting proxy with args: %v\n", args) - return e2e.StartLongRunningTHVCommand(config, args...) + return startLongRunningTHVCommand(config, args...) } -func startProxyWithOAuthDetection(config *e2e.TestConfig, serverName, targetURL string, port int, clientID, clientSecret string) *exec.Cmd { +func startProxyWithOAuthDetection(config *testConfig, serverName, targetURL string, port int, clientID, clientSecret string) *exec.Cmd { args := []string{ "proxy", "--host", "localhost", @@ -525,10 +524,10 @@ func startProxyWithOAuthDetection(config *e2e.TestConfig, serverName, targetURL serverName, } - return e2e.StartLongRunningTHVCommand(config, args...) + return startLongRunningTHVCommand(config, args...) } -func startProxyWithAutoDetection(config *e2e.TestConfig, serverName, targetURL string, port int, clientID, clientSecret string) *exec.Cmd { +func startProxyWithAutoDetection(config *testConfig, serverName, targetURL string, port int, clientID, clientSecret string) *exec.Cmd { args := []string{ "proxy", "--host", "localhost", @@ -543,10 +542,10 @@ func startProxyWithAutoDetection(config *e2e.TestConfig, serverName, targetURL s // Log the command for debugging GinkgoWriter.Printf("Starting proxy with auto-detection args: %v\n", args) - return e2e.StartLongRunningTHVCommand(config, args...) + return startLongRunningTHVCommand(config, args...) } -func startProxyWithOAuthForMCP(config *e2e.TestConfig, serverName, targetURL string, port int, issuer, clientID, clientSecret string) (*exec.Cmd, *bytes.Buffer) { +func startProxyWithOAuthForMCP(config *testConfig, serverName, targetURL string, port int, issuer, clientID, clientSecret string) (*exec.Cmd, *bytes.Buffer) { args := []string{ "proxy", "--host", "localhost", diff --git a/test/e2e/restart_test.go b/test/e2e/restart_test.go index cdfe2fc89..cb104080f 100644 --- a/test/e2e/restart_test.go +++ b/test/e2e/restart_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "fmt" @@ -7,29 +7,27 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("Server Restart", func() { var ( - config *e2e.TestConfig + config *testConfig serverName string ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() serverName = generateTestServerName("restart-test") // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) AfterEach(func() { if config.CleanupAfter { // Clean up the server if it exists - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) @@ -38,29 +36,29 @@ var _ = Describe("Server Restart", func() { Context("when restarting a running server", func() { It("should successfully restart and remain accessible", func() { By("Starting an OSV MCP server") - stdout, stderr := e2e.NewTHVCommand(config, "run", "--name", serverName, "osv").ExpectSuccess() + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "osv").ExpectSuccess() // The command should indicate success Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") // Get the server URL before restart - originalURL, err := e2e.GetMCPServerURL(config, serverName) + originalURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL") By("Restarting the server") - stdout, stderr = e2e.NewTHVCommand(config, "restart", serverName).ExpectSuccess() + stdout, stderr = NewTHVCommand(config, "restart", serverName).ExpectSuccess() Expect(stdout+stderr).To(ContainSubstring("restart"), "Output should mention restart operation") By("Waiting for the server to be running again") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running again within 60 seconds") // Get the server URL after restart - newURL, err := e2e.GetMCPServerURL(config, serverName) + newURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to get server URL after restart") // The URLs should be the same after restart @@ -68,7 +66,7 @@ var _ = Describe("Server Restart", func() { By("Verifying the server is functional after restart") // List server to verify it's operational - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should be listed") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") }) @@ -77,20 +75,20 @@ var _ = Describe("Server Restart", func() { Context("when restarting a stopped server", func() { It("should start the server if it was stopped", func() { By("Starting an OSV MCP server") - stdout, stderr := e2e.NewTHVCommand(config, "run", "--name", serverName, "osv").ExpectSuccess() + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "osv").ExpectSuccess() Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") By("Stopping the server") - stdout, _ = e2e.NewTHVCommand(config, "stop", serverName).ExpectSuccess() + stdout, _ = NewTHVCommand(config, "stop", serverName).ExpectSuccess() Expect(stdout).To(ContainSubstring("stop"), "Output should mention stop operation") By("Verifying the server is stopped") Eventually(func() bool { - stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() lines := strings.Split(stdout, "\n") for _, line := range lines { if strings.Contains(line, serverName) { @@ -102,106 +100,105 @@ var _ = Describe("Server Restart", func() { }, 10*time.Second, 1*time.Second).Should(BeTrue(), "Server should be stopped") By("Restarting the stopped server") - stdout, stderr = e2e.NewTHVCommand(config, "restart", serverName).ExpectSuccess() + stdout, stderr = NewTHVCommand(config, "restart", serverName).ExpectSuccess() Expect(stdout+stderr).To(ContainSubstring("restart"), "Output should mention restart operation") By("Waiting for the server to be running again") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running again within 60 seconds") By("Verifying the server is functional after restart") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should be listed") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") }) }) - // TODO: Uncomment when groups are fully supported - //Context("when restarting servers with --groups flag", func() { - // It("should restart servers belonging to the specified group", func() { - // // Define group name - // groupName := fmt.Sprintf("restart-group-%d", GinkgoRandomSeed()) - // - // // Create two servers - // serverName1 := generateTestServerName("restart-group-test1") - // serverName2 := generateTestServerName("restart-group-test2") - // - // By("Creating a group first") - // stdout, stderr := e2e.NewTHVCommand(config, "group", "create", groupName).ExpectSuccess() - // Expect(stdout+stderr).To(ContainSubstring("group"), "Output should mention group creation") - // - // By("Starting the first server") - // stdout, stderr = e2e.NewTHVCommand(config, "run", "--name", serverName1, "--group", groupName, "osv").ExpectSuccess() - // Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") - // - // By("Starting the second server") - // stdout, stderr = e2e.NewTHVCommand(config, "run", "--name", serverName2, "--group", groupName, "osv").ExpectSuccess() - // Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") - // - // By("Waiting for both servers to be running") - // err := e2e.WaitForMCPServer(config, serverName1, 60*time.Second) - // Expect(err).ToNot(HaveOccurred(), "First server should be running within 60 seconds") - // - // err = e2e.WaitForMCPServer(config, serverName2, 60*time.Second) - // Expect(err).ToNot(HaveOccurred(), "Second server should be running within 60 seconds") - // - // By("Stopping both servers") - // stdout, _ = e2e.NewTHVCommand(config, "stop", serverName1).ExpectSuccess() - // Expect(stdout).To(ContainSubstring("stop"), "Output should mention stop operation for first server") - // - // stdout, _ = e2e.NewTHVCommand(config, "stop", serverName2).ExpectSuccess() - // Expect(stdout).To(ContainSubstring("stop"), "Output should mention stop operation for second server") - // - // By("Verifying the servers are stopped") - // Eventually(func() bool { - // stdout, _ := e2e.NewTHVCommand(config, "list", "--all").ExpectSuccess() - // lines := strings.Split(stdout, "\n") - // server1Found := false - // server2Found := false - // server1Running := false - // server2Running := false - // - // for _, line := range lines { - // if strings.Contains(line, serverName1) { - // server1Found = true - // server1Running = strings.Contains(line, "running") - // } - // if strings.Contains(line, serverName2) { - // server2Found = true - // server2Running = strings.Contains(line, "running") - // } - // } - // - // // Both servers should be found and neither should be running - // return server1Found && server2Found && !server1Running && !server2Running - // }, 10*time.Second, 1*time.Second).Should(BeTrue(), "Both servers should be stopped") - // - // By("Restarting all servers in the group") - // stdout, stderr = e2e.NewTHVCommand(config, "restart", "--group", groupName).ExpectSuccess() - // Expect(stdout+stderr).To(ContainSubstring("restart"), "Output should mention restart operation") - // - // By("Waiting for both servers to be running again") - // err = e2e.WaitForMCPServer(config, serverName1, 60*time.Second) - // Expect(err).ToNot(HaveOccurred(), "First server should be running again within 60 seconds") - // - // err = e2e.WaitForMCPServer(config, serverName2, 60*time.Second) - // Expect(err).ToNot(HaveOccurred(), "Second server should be running again within 60 seconds") - // - // By("Verifying both servers are functional after restart") - // stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() - // Expect(stdout).To(ContainSubstring(serverName1), "First server should be listed") - // Expect(stdout).To(ContainSubstring(serverName2), "Second server should be listed") - // Expect(stdout).To(ContainSubstring("running"), "Servers should be in running state") - // - // // Clean up these specific servers at the end of the test - // defer func() { - // if config.CleanupAfter { - // _ = e2e.StopAndRemoveMCPServer(config, serverName1) - // _ = e2e.StopAndRemoveMCPServer(config, serverName2) - // } - // }() - // }) - //}) + Context("when restarting servers with --groups flag", func() { + It("should restart servers belonging to the specified group", func() { + // Define group name + groupName := fmt.Sprintf("restart-group-%d", GinkgoRandomSeed()) + + // Create two servers + serverName1 := generateTestServerName("restart-group-test1") + serverName2 := generateTestServerName("restart-group-test2") + + By("Creating a group first") + stdout, stderr := NewTHVCommand(config, "group", "create", groupName).ExpectSuccess() + Expect(stdout+stderr).To(ContainSubstring("group"), "Output should mention group creation") + + By("Starting the first server") + stdout, stderr = NewTHVCommand(config, "run", "--name", serverName1, "--group", groupName, "osv").ExpectSuccess() + Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") + + By("Starting the second server") + stdout, stderr = NewTHVCommand(config, "run", "--name", serverName2, "--group", groupName, "osv").ExpectSuccess() + Expect(stdout+stderr).To(ContainSubstring("osv"), "Output should mention the osv server") + + By("Waiting for both servers to be running") + err := waitForMCPServer(config, serverName1, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "First server should be running within 60 seconds") + + err = waitForMCPServer(config, serverName2, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "Second server should be running within 60 seconds") + + By("Stopping both servers") + stdout, _ = NewTHVCommand(config, "stop", serverName1).ExpectSuccess() + Expect(stdout).To(ContainSubstring("stop"), "Output should mention stop operation for first server") + + stdout, _ = NewTHVCommand(config, "stop", serverName2).ExpectSuccess() + Expect(stdout).To(ContainSubstring("stop"), "Output should mention stop operation for second server") + + By("Verifying the servers are stopped") + Eventually(func() bool { + stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() + lines := strings.Split(stdout, "\n") + server1Found := false + server2Found := false + server1Running := false + server2Running := false + + for _, line := range lines { + if strings.Contains(line, serverName1) { + server1Found = true + server1Running = strings.Contains(line, "running") + } + if strings.Contains(line, serverName2) { + server2Found = true + server2Running = strings.Contains(line, "running") + } + } + + // Both servers should be found and neither should be running + return server1Found && server2Found && !server1Running && !server2Running + }, 10*time.Second, 1*time.Second).Should(BeTrue(), "Both servers should be stopped") + + By("Restarting all servers in the group") + stdout, stderr = NewTHVCommand(config, "restart", "--group", groupName).ExpectSuccess() + Expect(stdout+stderr).To(ContainSubstring("restart"), "Output should mention restart operation") + + By("Waiting for both servers to be running again") + err = waitForMCPServer(config, serverName1, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "First server should be running again within 60 seconds") + + err = waitForMCPServer(config, serverName2, 60*time.Second) + Expect(err).ToNot(HaveOccurred(), "Second server should be running again within 60 seconds") + + By("Verifying both servers are functional after restart") + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() + Expect(stdout).To(ContainSubstring(serverName1), "First server should be listed") + Expect(stdout).To(ContainSubstring(serverName2), "Second server should be listed") + Expect(stdout).To(ContainSubstring("running"), "Servers should be in running state") + + // Clean up these specific servers at the end of the test + defer func() { + if config.CleanupAfter { + _ = stopAndRemoveMCPServer(config, serverName1) + _ = stopAndRemoveMCPServer(config, serverName2) + } + }() + }) + }) }) }) diff --git a/test/e2e/rm_group_test.go b/test/e2e/rm_group_test.go index ceb518bea..07c841dfb 100644 --- a/test/e2e/rm_group_test.go +++ b/test/e2e/rm_group_test.go @@ -1,10 +1,7 @@ package e2e -// TODO: add back in once we have a working group command, and update the docs -/* import ( "fmt" - "strings" "time" . "github.com/onsi/ginkgo/v2" @@ -12,7 +9,7 @@ import ( ) var _ = Describe("Group Remove E2E Tests", func() { - var config *TestConfig + var config *testConfig BeforeEach(func() { config = NewTestConfig() @@ -20,14 +17,6 @@ var _ = Describe("Group Remove E2E Tests", func() { // Check if thv binary is available err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") - - // Clean up any existing test groups and workloads - cleanupTestGroups() - }) - - AfterEach(func() { - // Clean up after each test - cleanupTestGroups() }) Describe("thv rm --group command", func() { @@ -47,6 +36,10 @@ var _ = Describe("Group Remove E2E Tests", func() { It("should return success when group exists but has no workloads", func() { groupName := fmt.Sprintf("rm-empty-group-%d", time.Now().UnixNano()) + + // Clean up the group after the test + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) stdout, stderr := NewTHVCommand(config, "rm", "--group", groupName).ExpectSuccess() @@ -56,6 +49,10 @@ var _ = Describe("Group Remove E2E Tests", func() { It("should remove single workload from group", func() { groupName := fmt.Sprintf("rm-group-single-%d", time.Now().UnixNano()) + + // Clean up the group after the test + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) workloadName := fmt.Sprintf("rm-group-workload-%d", time.Now().UnixNano()) createWorkloadInGroup(config, workloadName, groupName) @@ -72,6 +69,10 @@ var _ = Describe("Group Remove E2E Tests", func() { It("should remove multiple workloads from group", func() { groupName := fmt.Sprintf("rm-group-multi-%d", time.Now().UnixNano()) + + // Clean up the group after the test + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) workload1 := fmt.Sprintf("rm-group-workload-1-%d", time.Now().UnixNano()) workload2 := fmt.Sprintf("rm-group-workload-2-%d", time.Now().UnixNano()) @@ -82,7 +83,7 @@ var _ = Describe("Group Remove E2E Tests", func() { // Verify all workloads are running for _, workloadName := range []string{workload1, workload2, workload3} { - Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload %s did not appear in thv list within 3 seconds", workloadName) + Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload %s did not appear in thv list within 10 seconds", workloadName) } // Remove all workloads in the group @@ -99,6 +100,10 @@ var _ = Describe("Group Remove E2E Tests", func() { It("should handle mixed workloads (some in group, some not)", func() { groupName := fmt.Sprintf("rm-group-mixed-%d", time.Now().UnixNano()) + + // Clean up the group after the test + defer cleanupSpecificGroup(groupName) + createGroup(config, groupName) groupWorkload1 := fmt.Sprintf("rm-group-workload-1-%d", time.Now().UnixNano()) groupWorkload2 := fmt.Sprintf("rm-group-workload-2-%d", time.Now().UnixNano()) @@ -109,9 +114,9 @@ var _ = Describe("Group Remove E2E Tests", func() { createWorkload(config, nonGroupWorkload1) createWorkload(config, nonGroupWorkload2) - // Wait for the workloads to appear in thv list (up to 5 seconds) + // Wait for the workloads to appear in thv list (up to 10 seconds) for _, workloadName := range []string{groupWorkload1, groupWorkload2, nonGroupWorkload1, nonGroupWorkload2} { - Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload %s did not appear in thv list within 3 seconds", workloadName) + Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload %s did not appear in thv list within 10 seconds", workloadName) } // Remove all workloads in the group @@ -142,8 +147,8 @@ var _ = Describe("Group Remove E2E Tests", func() { workloadName := fmt.Sprintf("rm-test-workload-%d", time.Now().UnixNano()) createWorkload(config, workloadName) - // Wait for the workload to appear in thv list (up to 5 seconds) - Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload did not appear in thv list in time") + // Wait for the workload to appear in thv list (up to 10 seconds) + Expect(waitForWorkload(config, workloadName)).To(BeTrue(), "Workload did not appear in thv list within 10 seconds") // Remove workload using normal rm command NewTHVCommand(config, "rm", workloadName).ExpectSuccess() @@ -153,43 +158,3 @@ var _ = Describe("Group Remove E2E Tests", func() { }) }) }) - -// Helper functions - -func createGroup(config *TestConfig, groupName string) { - NewTHVCommand(config, "group", "create", groupName).ExpectSuccess() -} - -func createWorkloadInGroup(config *TestConfig, workloadName, groupName string) { - NewTHVCommand(config, "run", "fetch", "--group", groupName, "--name", workloadName).ExpectSuccess() -} - -func createWorkload(config *TestConfig, workloadName string) { - NewTHVCommand(config, "run", "fetch", "--name", workloadName).ExpectSuccess() -} - -func removeWorkload(config *TestConfig, workloadName string) { - NewTHVCommand(config, "rm", workloadName).ExpectSuccess() -} - -func isWorkloadRunning(config *TestConfig, workloadName string) bool { - stdout, _ := NewTHVCommand(config, "list", "--all").ExpectSuccess() - return strings.Contains(stdout, workloadName) -} - -func waitForWorkload(config *TestConfig, workloadName string) bool { - deadline := time.Now().Add(3 * time.Second) - for time.Now().Before(deadline) { - if isWorkloadRunning(config, workloadName) { - return true - } - time.Sleep(100 * time.Millisecond) - } - return false -} - -func cleanupTestGroups() { - // This is a simplified cleanup - in a real scenario, you might want to be more specific - // about which groups to clean up based on test naming conventions -} -*/ diff --git a/test/e2e/stdio_proxy_over_streamable_http_mcp_server_test.go b/test/e2e/stdio_proxy_over_streamable_http_mcp_server_test.go index 8651fadb7..854f3f1e4 100644 --- a/test/e2e/stdio_proxy_over_streamable_http_mcp_server_test.go +++ b/test/e2e/stdio_proxy_over_streamable_http_mcp_server_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "bytes" @@ -9,16 +9,14 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("TimeStreamableHttpMcpServer", Serial, func() { - var config *e2e.TestConfig + var config *testConfig BeforeEach(func() { - config = e2e.NewTestConfig() - err := e2e.CheckTHVBinaryAvailable(config) + config = NewTestConfig() + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") }) @@ -31,32 +29,32 @@ var _ = Describe("TimeStreamableHttpMcpServer", Serial, func() { AfterEach(func() { if config.CleanupAfter { - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") } }) It("should respond to a single get_current_time request and a batch request", func() { By("Starting the time MCP server with streamable-http proxy") - e2e.NewTHVCommand(config, "run", + NewTHVCommand(config, "run", "--name", serverName, "--proxy-mode", "streamable-http", "time").ExpectSuccess() By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Getting the server URL") - serverURL, err := e2e.GetMCPServerURL(config, serverName) + serverURL, err := getMCPServerURL(config, serverName) Expect(err).ToNot(HaveOccurred()) By("Waiting for MCP server to be ready") - err = e2e.WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) + err = WaitForMCPServerReady(config, serverURL, "streamable-http", 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Creating MCP client and initializing connection") - mcpClient, err := e2e.NewMCPClientForStreamableHTTP(config, serverURL) + mcpClient, err := NewMCPClientForStreamableHTTP(config, serverURL) Expect(err).ToNot(HaveOccurred()) defer mcpClient.Close() diff --git a/test/e2e/thvignore_test.go b/test/e2e/thvignore_test.go index 2c068ac03..b7c1d2f1e 100644 --- a/test/e2e/thvignore_test.go +++ b/test/e2e/thvignore_test.go @@ -1,4 +1,4 @@ -package e2e_test +package e2e import ( "fmt" @@ -9,23 +9,21 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - - "github.com/stacklok/toolhive/test/e2e" ) var _ = Describe("THVIgnore E2E Tests", func() { var ( - config *e2e.TestConfig + config *testConfig serverName string tempDir string ) BeforeEach(func() { - config = e2e.NewTestConfig() + config = NewTestConfig() serverName = fmt.Sprintf("thvignore-test-%d", GinkgoRandomSeed()) // Check if thv binary is available - err := e2e.CheckTHVBinaryAvailable(config) + err := CheckTHVBinaryAvailable(config) Expect(err).ToNot(HaveOccurred(), "thv binary should be available") // Create a temporary directory for test files @@ -36,7 +34,7 @@ var _ = Describe("THVIgnore E2E Tests", func() { AfterEach(func() { if config.CleanupAfter { // Clean up the server if it exists - err := e2e.StopAndRemoveMCPServer(config, serverName) + err := stopAndRemoveMCPServer(config, serverName) Expect(err).ToNot(HaveOccurred(), "Should be able to stop and remove server") // Clean up temporary directory @@ -94,7 +92,7 @@ node_modules/ By("Starting MCP server with volume mount and ignore processing") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-globally=false", // Only use local .thvignore @@ -104,11 +102,11 @@ node_modules/ Expect(stdout+stderr).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") By("Verifying the server appears in the list") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") @@ -117,7 +115,7 @@ node_modules/ containerName := fmt.Sprintf("toolhive-%s", serverName) // Use the existing StartDockerCommand helper to inspect the container - dockerCmd := e2e.StartDockerCommand("inspect", containerName) + dockerCmd := startDockerCommand("inspect", containerName) var dockerStdout, dockerStderr strings.Builder dockerCmd.Stdout = &dockerStdout dockerCmd.Stderr = &dockerStderr @@ -173,7 +171,7 @@ node_modules/ By("Starting MCP server with ignore processing") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--print-resolved-overlays", @@ -184,13 +182,13 @@ node_modules/ Expect(stdout+stderr).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should be running within 60 seconds") By("Inspecting the container to verify tmpfs overlays are applied") containerName := fmt.Sprintf("toolhive-%s", serverName) - dockerCmd := e2e.StartDockerCommand("inspect", containerName) + dockerCmd := startDockerCommand("inspect", containerName) var dockerStdout, dockerStderr strings.Builder dockerCmd.Stdout = &dockerStdout dockerCmd.Stderr = &dockerStderr @@ -267,7 +265,7 @@ node_modules/ By("Starting MCP server with global ignore file") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-file", globalIgnorePath, @@ -276,12 +274,12 @@ node_modules/ Expect(stdout+stderr).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Verifying global ignore patterns are applied") // The server should start successfully with global ignore patterns applied - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName), "Server should appear in the list") Expect(stdout).To(ContainSubstring("running"), "Server should be in running state") }) @@ -346,7 +344,7 @@ node_modules/ By("Starting MCP server with both global and local ignore patterns") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-file", globalIgnorePath, @@ -357,13 +355,13 @@ node_modules/ Expect(output).To(ContainSubstring("fetch"), "Output should mention the fetch server") By("Waiting for the server to be running") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) By("Inspecting the container to verify tmpfs overlays are applied for both global and local patterns") containerName := fmt.Sprintf("toolhive-%s", serverName) - dockerCmd := e2e.StartDockerCommand("inspect", containerName) + dockerCmd := startDockerCommand("inspect", containerName) var dockerStdout, dockerStderr strings.Builder dockerCmd.Stdout = &dockerStdout dockerCmd.Stderr = &dockerStderr @@ -429,7 +427,7 @@ node_modules/ By("Starting MCP server - should handle invalid patterns gracefully") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-globally=false", @@ -439,7 +437,7 @@ node_modules/ Expect(stdout + stderr).To(ContainSubstring("fetch")) By("Waiting for the server to be running") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should start despite invalid patterns") }) }) @@ -461,7 +459,7 @@ node_modules/ By("Starting MCP server without .thvignore file") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-globally=false", @@ -470,11 +468,11 @@ node_modules/ Expect(stdout + stderr).To(ContainSubstring("fetch")) By("Waiting for the server to be running") - err := e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err := waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred(), "Server should start normally without ignore file") By("Verifying server is running") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) Expect(stdout).To(ContainSubstring("running")) }) @@ -491,7 +489,7 @@ node_modules/ nonExistentPath := "/non/existent/path/thvignore" // This should either succeed (ignoring the missing file) or fail gracefully - stdout, stderr, err := e2e.NewTHVCommand(config, "run", + stdout, stderr, err := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-file", nonExistentPath, @@ -510,10 +508,10 @@ node_modules/ Expect(stdout + stderr).To(ContainSubstring("fetch")) // Clean up if server started - err = e2e.WaitForMCPServer(config, serverName, 10*time.Second) + err = waitForMCPServer(config, serverName, 10*time.Second) if err == nil { // Server started, verify it's running - listOutput, _ := e2e.NewTHVCommand(config, "list").ExpectSuccess() + listOutput, _ := NewTHVCommand(config, "list").ExpectSuccess() Expect(listOutput).To(ContainSubstring(serverName)) } } @@ -555,7 +553,7 @@ node_modules/ By("Starting fetch MCP server with ignore patterns") volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-globally=false", @@ -564,10 +562,10 @@ node_modules/ Expect(stdout + stderr).To(ContainSubstring("fetch")) By("Verifying server starts and runs successfully") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) Expect(stdout).To(ContainSubstring("running")) }) @@ -616,7 +614,7 @@ config/*.json startTime := time.Now() volumeMount := fmt.Sprintf("%s:/workspace", tempDir) - stdout, stderr := e2e.NewTHVCommand(config, "run", + stdout, stderr := NewTHVCommand(config, "run", "--name", serverName, "--volume", volumeMount, "--ignore-globally=false", @@ -625,7 +623,7 @@ config/*.json Expect(stdout + stderr).To(ContainSubstring("fetch")) By("Verifying server starts within reasonable time") - err = e2e.WaitForMCPServer(config, serverName, 60*time.Second) + err = waitForMCPServer(config, serverName, 60*time.Second) Expect(err).ToNot(HaveOccurred()) startupDuration := time.Since(startTime) @@ -635,7 +633,7 @@ config/*.json Expect(startupDuration).To(BeNumerically("<", 2*time.Minute), "Server should start within 2 minutes even with many files") - stdout, _ = e2e.NewTHVCommand(config, "list").ExpectSuccess() + stdout, _ = NewTHVCommand(config, "list").ExpectSuccess() Expect(stdout).To(ContainSubstring(serverName)) Expect(stdout).To(ContainSubstring("running")) })