diff --git a/AGENTS.md b/AGENTS.md index cc32420..bb90931 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -401,6 +401,192 @@ Commands should follow a consistent structure for maintainability and testabilit **Reference:** See `pkg/cmd/init.go` for a complete implementation of this pattern. +### Flag Binding Best Practices + +**IMPORTANT**: Always bind command flags directly to struct fields using the `*Var` variants (e.g., `StringVarP`, `BoolVarP`, `IntVarP`) instead of using the non-binding variants and then calling `GetString()`, `GetBool()`, etc. in `preRun`. + +**Benefits:** +- **Cleaner code**: No need to call `cmd.Flags().GetString()` and handle errors +- **Simpler testing**: Tests can set struct fields directly instead of creating and setting flags +- **Earlier binding**: Values are available immediately when preRun is called +- **Less error-prone**: No risk of typos in flag names when retrieving values + +**Pattern:** + +```go +// Command struct with fields for all flags +type myCmd struct { + verbose bool + output string + count int + manager instances.Manager +} + +// Bind flags to struct fields in the command factory +func NewMyCmd() *cobra.Command { + c := &myCmd{} + + cmd := &cobra.Command{ + Use: "my-command", + Short: "My command description", + Args: cobra.NoArgs, + PreRunE: c.preRun, + RunE: c.run, + } + + // GOOD: Bind flags directly to struct fields + cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + cmd.Flags().IntVarP(&c.count, "count", "c", 10, "Number of items to process") + + return cmd +} + +// Use the bound values directly in preRun +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + // Values are already available from struct fields + if m.output != "" && m.output != "json" { + return fmt.Errorf("unsupported output format: %s", m.output) + } + + // No need to call cmd.Flags().GetString("output") + return nil +} +``` + +**Avoid:** + +```go +// BAD: Don't define flags without binding +cmd.Flags().StringP("output", "o", "", "Output format") + +// BAD: Don't retrieve flag values in preRun +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + output, err := cmd.Flags().GetString("output") // Avoid this pattern + if err != nil { + return err + } + m.output = output + // ... +} +``` + +**Testing with Bound Flags:** + +```go +func TestMyCmd_PreRun(t *testing.T) { + t.Run("validates output format", func(t *testing.T) { + // Set struct fields directly - no need to set up flags + c := &myCmd{ + output: "xml", // Invalid format + } + cmd := &cobra.Command{} + + err := c.preRun(cmd, []string{}) + if err == nil { + t.Fatal("Expected error for invalid output format") + } + }) +} +``` + +**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for examples of proper flag binding. + +### JSON Output Support Pattern + +When adding JSON output support to commands, follow this pattern to ensure consistent error handling and output formatting: + +**Rules:** + +1. **Check output flag FIRST in preRun** - Validate the output format before any other validation +2. **Set SilenceErrors and SilenceUsage early** - Prevent Cobra's default error output when in JSON mode +3. **Use outputErrorIfJSON for ALL errors** - In preRun, run, and any helper methods (like outputJSON) + +**Pattern:** + +```go +type myCmd struct { + output string // Bound to --output flag + manager instances.Manager +} + +func (m *myCmd) preRun(cmd *cobra.Command, args []string) error { + // 1. FIRST: Validate output format + if m.output != "" && m.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", m.output) + } + + // 2. EARLY: Silence Cobra's error output in JSON mode + if m.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + + // 3. ALL subsequent errors use outputErrorIfJSON + storageDir, err := cmd.Flags().GetString("storage") + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to read --storage flag: %w", err)) + } + + manager, err := instances.NewManager(storageDir) + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to create manager: %w", err)) + } + m.manager = manager + + return nil +} + +func (m *myCmd) run(cmd *cobra.Command, args []string) error { + // ALL errors in run use outputErrorIfJSON + data, err := m.manager.GetData() + if err != nil { + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to get data: %w", err)) + } + + if m.output == "json" { + return m.outputJSON(cmd, data) + } + + // Text output + cmd.Println(data) + return nil +} + +func (m *myCmd) outputJSON(cmd *cobra.Command, data interface{}) error { + jsonData, err := json.MarshalIndent(data, "", " ") + if err != nil { + // Even unlikely errors in helper methods use outputErrorIfJSON + return outputErrorIfJSON(cmd, m.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} +``` + +**Why this pattern:** +- **Consistent error format**: All errors are JSON when `--output=json` is set +- **No Cobra pollution**: SilenceErrors prevents "Error: ..." prefix in JSON output +- **Early detection**: Output flag is validated before expensive operations +- **Helper methods work**: outputErrorIfJSON works in any method that has access to cmd and output flag + +**Helper function:** + +The `outputErrorIfJSON` helper in `pkg/cmd/conversion.go` handles the formatting: + +```go +func outputErrorIfJSON(cmd interface{ OutOrStdout() io.Writer }, output string, err error) error { + if output == "json" { + jsonErr, _ := formatErrorJSON(err) + fmt.Fprintln(cmd.OutOrStdout(), jsonErr) // Errors go to stdout in JSON mode + } + return err // Still return the error for proper exit codes +} +``` + +**Reference:** See `pkg/cmd/init.go`, `pkg/cmd/workspace_remove.go`, and `pkg/cmd/workspace_list.go` for complete implementations. + ### Testing Pattern for Commands Commands should have two types of tests following the pattern in `pkg/cmd/init_test.go`: @@ -520,16 +706,55 @@ if err != nil { ### Cross-Platform Path Handling -**IMPORTANT**: All path operations must be cross-platform compatible (Linux, macOS, Windows). +⚠️ **CRITICAL**: All path operations and tests MUST be cross-platform compatible (Linux, macOS, Windows). **Rules:** - Always use `filepath.Join()` for path construction (never hardcode "/" or "\\") - Convert relative paths to absolute with `filepath.Abs()` - Never hardcode paths with `~` - use `os.UserHomeDir()` instead - In tests, use `filepath.Join()` for all path assertions -- Use `t.TempDir()` for temporary directories in tests +- **Use `t.TempDir()` for ALL temporary directories in tests - never hardcode paths** -**Examples:** +#### Common Test Failures on Windows + +Tests often fail on Windows due to hardcoded Unix-style paths. These paths get normalized differently by `filepath.Abs()` on Windows vs Unix systems. + +**❌ NEVER do this in tests:** +```go +// BAD - Will fail on Windows because filepath.Abs() normalizes differently +instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: "/path/to/source", // ❌ Hardcoded Unix path + ConfigDir: "/path/to/config", // ❌ Hardcoded Unix path +}) + +// BAD - Will fail on Windows +invalidPath := "/this/path/does/not/exist" // ❌ Unix-style absolute path + +// BAD - Platform-specific separator +path := dir + "/subdir" // ❌ Hardcoded forward slash +``` + +**✅ ALWAYS do this in tests:** +```go +// GOOD - Cross-platform, works everywhere +sourceDir := t.TempDir() +configDir := t.TempDir() +instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, // ✅ Real temp directory + ConfigDir: configDir, // ✅ Real temp directory +}) + +// GOOD - Create invalid path cross-platform way +tempDir := t.TempDir() +notADir := filepath.Join(tempDir, "file") +os.WriteFile(notADir, []byte("test"), 0644) +invalidPath := filepath.Join(notADir, "subdir") // ✅ Will fail MkdirAll on all platforms + +// GOOD - Use filepath.Join() +path := filepath.Join(dir, "subdir") // ✅ Cross-platform +``` + +#### Examples for Production Code ```go // GOOD: Cross-platform path construction @@ -557,6 +782,8 @@ tempDir := t.TempDir() // Automatic cleanup sourcesDir := t.TempDir() ``` +**Why this matters:** Tests that pass on Linux/macOS may fail on Windows CI if they use hardcoded Unix paths. Always use `t.TempDir()` and `filepath.Join()` to ensure tests work on all platforms. + ## Documentation Standards ### Markdown Best Practices diff --git a/README.md b/README.md index af7aace..604880d 100644 --- a/README.md +++ b/README.md @@ -36,6 +36,187 @@ Pre-configured capabilities or specialized functions that can be enabled for an ### Workspace A registered directory containing your project source code and its configuration. Each workspace is tracked by kortex-cli with a unique ID and name for easy management. +## Scenarios + +### Managing Workspaces from a UI or Programmatically + +This scenario demonstrates how to manage workspaces programmatically using JSON output, which is ideal for UIs, scripts, or automation tools. All commands support the `--output json` (or `-o json`) flag for machine-readable output. + +**Step 1: Check existing workspaces** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [] +} +``` + +Exit code: `0` (success, but no workspaces registered) + +**Step 2: Register a new workspace** + +```bash +$ kortex-cli init /path/to/project -o json +``` + +```json +{ + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea" +} +``` + +Exit code: `0` (success) + +**Step 3: Register with verbose output to get full details** + +```bash +$ kortex-cli init /path/to/another-project -o json -v +``` + +```json +{ + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } +} +``` + +Exit code: `0` (success) + +**Step 4: List all workspaces** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [ + { + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea", + "name": "project", + "paths": { + "source": "/absolute/path/to/project", + "configuration": "/absolute/path/to/project/.kortex" + } + }, + { + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } + } + ] +} +``` + +Exit code: `0` (success) + +**Step 5: Remove a workspace** + +```bash +$ kortex-cli workspace remove 2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea -o json +``` + +```json +{ + "id": "2c5f16046476be368fcada501ac6cdc6bbd34ea80eb9ceb635530c0af64681ea" +} +``` + +Exit code: `0` (success) + +**Step 6: Verify removal** + +```bash +$ kortex-cli workspace list -o json +``` + +```json +{ + "items": [ + { + "id": "f6e5d4c3b2a1098765432109876543210987654321098765432109876543210a", + "name": "another-project", + "paths": { + "source": "/absolute/path/to/another-project", + "configuration": "/absolute/path/to/another-project/.kortex" + } + } + ] +} +``` + +Exit code: `0` (success) + +#### Error Handling + +All errors are returned in JSON format when using `--output json`, with the error written to **stdout** (not stderr) and a non-zero exit code. + +**Error: Non-existent directory** + +```bash +$ kortex-cli init /tmp/no-exist -o json +``` + +```json +{ + "error": "sources directory does not exist: /tmp/no-exist" +} +``` + +Exit code: `1` (error) + +**Error: Workspace not found** + +```bash +$ kortex-cli workspace remove unknown-id -o json +``` + +```json +{ + "error": "workspace not found: unknown-id" +} +``` + +Exit code: `1` (error) + +#### Best Practices for Programmatic Usage + +1. **Always check the exit code** to determine success (0) or failure (non-zero) +2. **Parse stdout** for JSON output in both success and error cases +3. **Use verbose mode** with init (`-v`) when you need full workspace details immediately after creation +4. **Handle both success and error JSON structures** in your code: + - Success responses have specific fields (e.g., `id`, `items`, `name`, `paths`) + - Error responses always have an `error` field + +**Example script pattern:** + +```bash +#!/bin/bash + +# Register a workspace +output=$(kortex-cli init /path/to/project -o json) +exit_code=$? + +if [ $exit_code -eq 0 ]; then + workspace_id=$(echo "$output" | jq -r '.id') + echo "Workspace created: $workspace_id" +else + error_msg=$(echo "$output" | jq -r '.error') + echo "Error: $error_msg" + exit 1 +fi +``` + ## Commands ### `init` - Register a New Workspace @@ -57,6 +238,7 @@ kortex-cli init [sources-directory] [flags] - `--workspace-configuration ` - Directory for workspace configuration files (default: `/.kortex`) - `--name, -n ` - Human-readable name for the workspace (default: generated from sources directory) - `--verbose, -v` - Show detailed output including all workspace information +- `--output, -o ` - Output format (supported: `json`) - `--storage ` - Storage directory for kortex-cli data (default: `$HOME/.kortex-cli`) #### Examples @@ -95,6 +277,38 @@ Registered workspace: Configuration directory: /absolute/path/to/myproject/.kortex ``` +**JSON output (default - ID only):** +```bash +kortex-cli init /path/to/myproject --output json +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6..." +} +``` + +**JSON output with verbose flag (full workspace details):** +```bash +kortex-cli init /path/to/myproject --output json --verbose +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6...", + "name": "myproject", + "paths": { + "source": "/absolute/path/to/myproject", + "configuration": "/absolute/path/to/myproject/.kortex" + } +} +``` + +**JSON output with short flags:** +```bash +kortex-cli init -o json -v +``` + #### Workspace Naming - If `--name` is not provided, the name is automatically generated from the last component of the sources directory path @@ -121,6 +335,10 @@ kortex-cli init /tmp/project --name "project" - The workspace ID is a unique identifier generated automatically - Workspaces can be listed using the `workspace list` command - The default configuration directory (`.kortex`) is created inside the sources directory unless specified otherwise +- JSON output format is useful for scripting and automation +- Without `--verbose`, JSON output returns only the workspace ID +- With `--verbose`, JSON output includes full workspace details (ID, name, paths) +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure ### `workspace list` - List All Registered Workspaces @@ -200,6 +418,7 @@ kortex-cli list -o json - When no workspaces are registered, the command displays "No workspaces registered" - The JSON output format is useful for scripting and automation - All paths are displayed as absolute paths for consistency +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure ### `workspace remove` - Remove a Workspace @@ -218,6 +437,7 @@ kortex-cli remove ID [flags] #### Flags +- `--output, -o ` - Output format (supported: `json`) - `--storage ` - Storage directory for kortex-cli data (default: `$HOME/.kortex-cli`) #### Examples @@ -242,9 +462,25 @@ kortex-cli list kortex-cli remove a1b2c3d4e5f6... ``` +**JSON output:** +```bash +kortex-cli workspace remove a1b2c3d4e5f6... --output json +``` +Output: +```json +{ + "id": "a1b2c3d4e5f6..." +} +``` + +**JSON output with short flag:** +```bash +kortex-cli remove a1b2c3d4e5f6... -o json +``` + #### Error Handling -**Workspace not found:** +**Workspace not found (text format):** ```bash kortex-cli remove invalid-id ``` @@ -254,9 +490,23 @@ Error: workspace not found: invalid-id Use 'workspace list' to see available workspaces ``` +**Workspace not found (JSON format):** +```bash +kortex-cli remove invalid-id --output json +``` +Output: +```json +{ + "error": "workspace not found: invalid-id" +} +``` + #### Notes - The workspace ID is required and can be obtained using the `workspace list` or `list` command - Removing a workspace only unregisters it from kortex-cli; it does not delete any files from the sources or configuration directories - If the workspace ID is not found, the command will fail with a helpful error message - Upon successful removal, the command outputs the ID of the removed workspace +- JSON output format is useful for scripting and automation +- When using `--output json`, errors are also returned in JSON format for consistent parsing +- **JSON error handling**: When `--output json` is used, errors are written to stdout (not stderr) in JSON format, and the CLI exits with code 1. Always check the exit code to determine success/failure diff --git a/go.mod b/go.mod index 11f8206..a3adc3a 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/kortex-hub/kortex-cli go 1.25.7 require ( - github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13 + github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68 github.com/spf13/cobra v1.10.2 ) diff --git a/go.sum b/go.sum index 5ed660e..92e54b0 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13 h1:JkzrfheDTegKzYzjOfo6K5oEgMmT1+1+oqxTfRghJCw= -github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260309100818-e0ea254e6f13/go.mod h1:jWKudiw26CIjuOsROQ80FOuuk2m2/5BVkuATpmD5xQQ= +github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68 h1:+rG9db97UcYp3RvWwyd3yQQDghsJgEPp2bFZcqz8qfc= +github.com/kortex-hub/kortex-cli-api/cli/go v0.0.0-20260312091024-8f627ff7db68/go.mod h1:jWKudiw26CIjuOsROQ80FOuuk2m2/5BVkuATpmD5xQQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= diff --git a/pkg/cmd/conversion.go b/pkg/cmd/conversion.go new file mode 100644 index 0000000..f4a6080 --- /dev/null +++ b/pkg/cmd/conversion.go @@ -0,0 +1,74 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + **********************************************************************/ + +package cmd + +import ( + "encoding/json" + "fmt" + "io" + + api "github.com/kortex-hub/kortex-cli-api/cli/go" + "github.com/kortex-hub/kortex-cli/pkg/instances" +) + +// instanceToWorkspaceId converts an Instance to an api.WorkspaceId +func instanceToWorkspaceId(instance instances.Instance) api.WorkspaceId { + return api.WorkspaceId{ + Id: instance.GetID(), + } +} + +// instanceToWorkspace converts an Instance to an api.Workspace +func instanceToWorkspace(instance instances.Instance) api.Workspace { + return api.Workspace{ + Id: instance.GetID(), + Name: instance.GetName(), + Paths: api.WorkspacePaths{ + Configuration: instance.GetConfigDir(), + Source: instance.GetSourceDir(), + }, + } +} + +// formatErrorJSON formats an error as JSON using api.Error schema +func formatErrorJSON(err error) (string, error) { + if err == nil { + return "", nil + } + errorResponse := api.Error{ + Error: err.Error(), + } + + jsonData, jsonErr := json.MarshalIndent(errorResponse, "", " ") + if jsonErr != nil { + return "", fmt.Errorf("failed to marshal error to JSON: %w", jsonErr) + } + + return string(jsonData), nil +} + +// outputErrorIfJSON outputs the error as JSON if output mode is "json", then returns the error. +// This helper reduces code duplication for error handling in commands. +func outputErrorIfJSON(cmd interface{ OutOrStdout() io.Writer }, output string, err error) error { + if output == "json" { + jsonErr, _ := formatErrorJSON(err) + fmt.Fprintln(cmd.OutOrStdout(), jsonErr) + } + return err +} diff --git a/pkg/cmd/conversion_test.go b/pkg/cmd/conversion_test.go new file mode 100644 index 0000000..260c9b5 --- /dev/null +++ b/pkg/cmd/conversion_test.go @@ -0,0 +1,378 @@ +/********************************************************************** + * Copyright (C) 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + **********************************************************************/ + +package cmd + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "testing" + + api "github.com/kortex-hub/kortex-cli-api/cli/go" + "github.com/kortex-hub/kortex-cli/pkg/instances" + "github.com/spf13/cobra" +) + +func TestInstanceToWorkspaceId(t *testing.T) { + t.Parallel() + + t.Run("converts instance to workspace ID", func(t *testing.T) { + t.Parallel() + + sourceDir := t.TempDir() + configDir := t.TempDir() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Name: "test-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Manually set ID (in real usage, Manager sets this) + instanceData := instance.Dump() + instanceData.ID = "test-id-123" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspaceId(instance) + + if result.Id != "test-id-123" { + t.Errorf("Expected ID 'test-id-123', got '%s'", result.Id) + } + + // Verify it only contains ID field by marshaling and checking JSON structure + jsonData, err := json.Marshal(result) + if err != nil { + t.Fatalf("Failed to marshal result: %v", err) + } + + var parsed map[string]interface{} + if err := json.Unmarshal(jsonData, &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + }) +} + +func TestInstanceToWorkspace(t *testing.T) { + t.Parallel() + + t.Run("converts instance to full workspace", func(t *testing.T) { + t.Parallel() + + sourceDir := t.TempDir() + configDir := t.TempDir() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Name: "test-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Manually set ID (in real usage, Manager sets this) + instanceData := instance.Dump() + instanceData.ID = "test-id-456" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspace(instance) + + if result.Id != "test-id-456" { + t.Errorf("Expected ID 'test-id-456', got '%s'", result.Id) + } + + if result.Name != "test-workspace" { + t.Errorf("Expected name 'test-workspace', got '%s'", result.Name) + } + + if result.Paths.Source != sourceDir { + t.Errorf("Expected source '%s', got '%s'", sourceDir, result.Paths.Source) + } + + if result.Paths.Configuration != configDir { + t.Errorf("Expected config '%s', got '%s'", configDir, result.Paths.Configuration) + } + }) + + t.Run("includes all required fields", func(t *testing.T) { + t.Parallel() + + sourceDir := t.TempDir() + configDir := t.TempDir() + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourceDir, + ConfigDir: configDir, + Name: "my-workspace", + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + // Set ID + instanceData := instance.Dump() + instanceData.ID = "full-test-id" + instance, _ = instances.NewInstanceFromData(instanceData) + + result := instanceToWorkspace(instance) + + // Marshal to JSON to verify structure + jsonData, err := json.Marshal(result) + if err != nil { + t.Fatalf("Failed to marshal result: %v", err) + } + + var parsed map[string]interface{} + if err := json.Unmarshal(jsonData, &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify all expected fields exist + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + if _, exists := parsed["name"]; !exists { + t.Error("Expected 'name' field in JSON") + } + if _, exists := parsed["paths"]; !exists { + t.Error("Expected 'paths' field in JSON") + } + + // Verify paths structure + paths, ok := parsed["paths"].(map[string]interface{}) + if !ok { + t.Fatal("Expected 'paths' to be an object") + } + + if _, exists := paths["source"]; !exists { + t.Error("Expected 'paths.source' field in JSON") + } + if _, exists := paths["configuration"]; !exists { + t.Error("Expected 'paths.configuration' field in JSON") + } + }) +} + +func TestFormatErrorJSON(t *testing.T) { + t.Parallel() + + t.Run("formats error as JSON", func(t *testing.T) { + t.Parallel() + + err := errors.New("something went wrong") + result, jsonErr := formatErrorJSON(err) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + // Parse the JSON to verify structure + var errorResponse api.Error + if err := json.Unmarshal([]byte(result), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + if errorResponse.Error != "something went wrong" { + t.Errorf("Expected error message 'something went wrong', got '%s'", errorResponse.Error) + } + }) + + t.Run("includes only error field", func(t *testing.T) { + t.Parallel() + + err := errors.New("test error") + result, jsonErr := formatErrorJSON(err) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + var parsed map[string]interface{} + if err := json.Unmarshal([]byte(result), &parsed); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("handles complex error messages", func(t *testing.T) { + t.Parallel() + + complexErr := errors.New("failed to process: invalid input \"test\"\nUse --help for more information") + result, jsonErr := formatErrorJSON(complexErr) + + if jsonErr != nil { + t.Fatalf("formatErrorJSON failed: %v", jsonErr) + } + + var errorResponse api.Error + if err := json.Unmarshal([]byte(result), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + expectedMsg := "failed to process: invalid input \"test\"\nUse --help for more information" + if errorResponse.Error != expectedMsg { + t.Errorf("Expected error message '%s', got '%s'", expectedMsg, errorResponse.Error) + } + }) +} + +func TestOutputErrorIfJSON(t *testing.T) { + t.Parallel() + + t.Run("outputs JSON when output is json", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "json", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON was written to output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v\nOutput was: %s", err, buf.String()) + } + + if errorResponse.Error != "test error message" { + t.Errorf("Expected error message 'test error message', got '%s'", errorResponse.Error) + } + }) + + t.Run("does not output JSON when output is empty", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify nothing was written to output + if buf.Len() != 0 { + t.Errorf("Expected no output, got: %s", buf.String()) + } + }) + + t.Run("does not output JSON when output is text", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("test error message") + returnedErr := outputErrorIfJSON(cmd, "text", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify nothing was written to output + if buf.Len() != 0 { + t.Errorf("Expected no output, got: %s", buf.String()) + } + }) + + t.Run("formats complex error messages correctly", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + testErr := errors.New("workspace not found: abc123\nUse 'workspace list' to see available workspaces") + returnedErr := outputErrorIfJSON(cmd, "json", testErr) + + // Verify the error is returned unchanged + if returnedErr != testErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON was written with full error message + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v", err) + } + + expectedMsg := "workspace not found: abc123\nUse 'workspace list' to see available workspaces" + if errorResponse.Error != expectedMsg { + t.Errorf("Expected error message '%s', got '%s'", expectedMsg, errorResponse.Error) + } + }) + + t.Run("handles wrapped errors", func(t *testing.T) { + t.Parallel() + + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + + baseErr := errors.New("base error") + wrappedErr := fmt.Errorf("wrapped error: %w", baseErr) + returnedErr := outputErrorIfJSON(cmd, "json", wrappedErr) + + // Verify the error is returned unchanged + if returnedErr != wrappedErr { + t.Errorf("Expected returned error to be the same as input error") + } + + // Verify JSON contains the full wrapped error message + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal JSON output: %v", err) + } + + if errorResponse.Error != "wrapped error: base error" { + t.Errorf("Expected error message 'wrapped error: base error', got '%s'", errorResponse.Error) + } + }) +} diff --git a/pkg/cmd/init.go b/pkg/cmd/init.go index b19513d..948d7e8 100644 --- a/pkg/cmd/init.go +++ b/pkg/cmd/init.go @@ -19,6 +19,7 @@ package cmd import ( + "encoding/json" "fmt" "os" "path/filepath" @@ -36,26 +37,39 @@ type initCmd struct { absConfigDir string manager instances.Manager verbose bool + output string } // preRun validates the parameters and flags func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if i.output != "" && i.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", i.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if i.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + // Get storage directory from global flag storageDir, err := cmd.Flags().GetString("storage") if err != nil { - return fmt.Errorf("failed to read --storage flag: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Convert to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve storage directory path: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve storage directory path: %w", err)) } // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to create manager: %w", err)) } i.manager = manager @@ -68,19 +82,19 @@ func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { // Convert to absolute path for clarity absSourcesDir, err := filepath.Abs(i.sourcesDir) if err != nil { - return fmt.Errorf("failed to resolve sources directory path: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve sources directory path: %w", err)) } i.absSourcesDir = absSourcesDir // Verify that the sources directory exists and is a directory fileInfo, err := os.Stat(i.absSourcesDir) if os.IsNotExist(err) { - return fmt.Errorf("sources directory does not exist: %s", i.absSourcesDir) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("sources directory does not exist: %s", i.absSourcesDir)) } else if err != nil { - return fmt.Errorf("failed to check sources directory: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to check sources directory: %w", err)) } if !fileInfo.IsDir() { - return fmt.Errorf("sources path is not a directory: %s", i.absSourcesDir) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("sources path is not a directory: %s", i.absSourcesDir)) } // If workspace-configuration flag was not explicitly set, default to .kortex/ inside sources directory @@ -91,7 +105,7 @@ func (i *initCmd) preRun(cmd *cobra.Command, args []string) error { // Convert workspace config to absolute path absConfigDir, err := filepath.Abs(i.workspaceConfigDir) if err != nil { - return fmt.Errorf("failed to resolve workspace configuration directory path: %w", err) + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to resolve workspace configuration directory path: %w", err)) } i.absConfigDir = absConfigDir @@ -107,15 +121,21 @@ func (i *initCmd) run(cmd *cobra.Command, args []string) error { Name: i.name, }) if err != nil { - return fmt.Errorf("failed to create instance: %w", err) + return outputErrorIfJSON(cmd, i.output, err) } // Add the instance to the manager addedInstance, err := i.manager.Add(instance) if err != nil { - return fmt.Errorf("failed to add instance: %w", err) + return outputErrorIfJSON(cmd, i.output, err) } + // Handle JSON output + if i.output == "json" { + return i.outputJSON(cmd, addedInstance) + } + + // Handle text output if i.verbose { cmd.Printf("Registered workspace:\n") cmd.Printf(" ID: %s\n", addedInstance.GetID()) @@ -129,6 +149,29 @@ func (i *initCmd) run(cmd *cobra.Command, args []string) error { return nil } +// outputJSON outputs the instance as JSON based on verbose flag +func (i *initCmd) outputJSON(cmd *cobra.Command, instance instances.Instance) error { + var jsonData []byte + var err error + + if i.verbose { + // Verbose mode: return full Workspace + workspace := instanceToWorkspace(instance) + jsonData, err = json.MarshalIndent(workspace, "", " ") + } else { + // Default mode: return only WorkspaceId + workspaceId := instanceToWorkspaceId(instance) + jsonData, err = json.MarshalIndent(workspaceId, "", " ") + } + + if err != nil { + return outputErrorIfJSON(cmd, i.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} + func NewInitCmd() *cobra.Command { c := &initCmd{} @@ -164,5 +207,8 @@ kortex-cli init --verbose`, // Add verbose flag cmd.Flags().BoolVarP(&c.verbose, "verbose", "v", false, "Show detailed output") + // Add output flag + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + return cmd } diff --git a/pkg/cmd/init_test.go b/pkg/cmd/init_test.go index 8632e6d..2d9e2ec 100644 --- a/pkg/cmd/init_test.go +++ b/pkg/cmd/init_test.go @@ -20,11 +20,13 @@ package cmd import ( "bytes" + "encoding/json" "os" "path/filepath" "strings" "testing" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/cmd/testutil" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" @@ -309,6 +311,119 @@ func TestInitCmd_PreRun(t *testing.T) { t.Errorf("Expected error to contain 'sources path is not a directory', got: %v", err) } }) + + t.Run("accepts empty output flag", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "", // Default empty output + } + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "" { + t.Errorf("Expected output to be empty, got %s", c.output) + } + }) + + t.Run("accepts json output format", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "json", + } + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "json" { + t.Errorf("Expected output to be 'json', got %s", c.output) + } + }) + + t.Run("rejects invalid output format", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + + c := &initCmd{ + output: "xml", + } + cmd := &cobra.Command{} + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", tempDir, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid output format") + } + + if !strings.Contains(err.Error(), "unsupported output format") { + t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) + } + if !strings.Contains(err.Error(), "xml") { + t.Errorf("Expected error to mention 'xml', got: %v", err) + } + }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") + + c := &initCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("workspace-configuration", "", "test flag") + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestInitCmd_E2E(t *testing.T) { @@ -1003,6 +1118,198 @@ func TestInitCmd_E2E(t *testing.T) { t.Errorf("Expected 0 instances, got %d", len(instancesList)) } }) + + t.Run("json output returns workspace ID by default", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--output", "json"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify ID is not empty + if workspaceId.Id == "" { + t.Error("Expected non-empty ID in JSON output") + } + + // Verify only ID field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + + // Verify instance was actually created + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instance, err := manager.Get(workspaceId.Id) + if err != nil { + t.Fatalf("Failed to get instance: %v", err) + } + + if instance.GetID() != workspaceId.Id { + t.Errorf("Expected instance ID %s, got %s", workspaceId.Id, instance.GetID()) + } + }) + + t.Run("json output with verbose returns full workspace", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--output", "json", "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspace api.Workspace + if err := json.Unmarshal(buf.Bytes(), &workspace); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify all fields are populated + if workspace.Id == "" { + t.Error("Expected non-empty ID in JSON output") + } + + if workspace.Name == "" { + t.Error("Expected non-empty Name in JSON output") + } + + if workspace.Paths.Source == "" { + t.Error("Expected non-empty Source path in JSON output") + } + + if workspace.Paths.Configuration == "" { + t.Error("Expected non-empty Configuration path in JSON output") + } + + // Verify paths are absolute + if !filepath.IsAbs(workspace.Paths.Source) { + t.Errorf("Expected absolute source path, got %s", workspace.Paths.Source) + } + + if !filepath.IsAbs(workspace.Paths.Configuration) { + t.Errorf("Expected absolute configuration path, got %s", workspace.Paths.Configuration) + } + + // Verify all expected fields exist + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + if _, exists := parsed["name"]; !exists { + t.Error("Expected 'name' field in JSON") + } + if _, exists := parsed["paths"]; !exists { + t.Error("Expected 'paths' field in JSON") + } + }) + + t.Run("json output error for non-existent directory", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + nonExistentDir := filepath.Join(storageDir, "does-not-exist") + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", nonExistentDir, "--output", "json"}) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected Execute() to fail with non-existent directory") + } + + // Parse JSON error output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + // Verify error message + if !strings.Contains(errorResponse.Error, "sources directory does not exist") { + t.Errorf("Expected error to contain 'sources directory does not exist', got: %s", errorResponse.Error) + } + + // Verify only error field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in error JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("json output uses custom name", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + customName := "my-custom-workspace" + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"--storage", storageDir, "init", sourcesDir, "--name", customName, "--output", "json", "--verbose"}) + + err := rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspace api.Workspace + if err := json.Unmarshal(buf.Bytes(), &workspace); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + if workspace.Name != customName { + t.Errorf("Expected name %s in JSON output, got %s", customName, workspace.Name) + } + }) } func TestInitCmd_Examples(t *testing.T) { diff --git a/pkg/cmd/workspace_list.go b/pkg/cmd/workspace_list.go index d01867b..921a7f6 100644 --- a/pkg/cmd/workspace_list.go +++ b/pkg/cmd/workspace_list.go @@ -36,34 +36,34 @@ type workspaceListCmd struct { // preRun validates the parameters and flags func (w *workspaceListCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if w.output != "" && w.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", w.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if w.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + // Get storage directory from global flag storageDir, err := cmd.Flags().GetString("storage") if err != nil { - return fmt.Errorf("failed to read --storage flag: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Convert to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve storage directory path: %w", err) - } - - // Get output format flag - output, err := cmd.Flags().GetString("output") - if err != nil { - return fmt.Errorf("failed to read --output flag: %w", err) - } - - // Validate output format if specified - if output != "" && output != "json" { - return fmt.Errorf("unsupported output format: %s (supported: json)", output) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to resolve storage directory path: %w", err)) } - w.output = output // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to create manager: %w", err)) } w.manager = manager @@ -75,7 +75,7 @@ func (w *workspaceListCmd) run(cmd *cobra.Command, args []string) error { // Get all instances instancesList, err := w.manager.List() if err != nil { - return fmt.Errorf("failed to list instances: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to list instances: %w", err)) } // Handle JSON output format @@ -105,14 +105,7 @@ func (w *workspaceListCmd) outputJSON(cmd *cobra.Command, instancesList []instan // Convert instances to API Workspace format workspaces := make([]api.Workspace, 0, len(instancesList)) for _, instance := range instancesList { - workspace := api.Workspace{ - Id: instance.GetID(), - Name: instance.GetName(), - Paths: api.WorkspacePaths{ - Configuration: instance.GetConfigDir(), - Source: instance.GetSourceDir(), - }, - } + workspace := instanceToWorkspace(instance) workspaces = append(workspaces, workspace) } @@ -124,7 +117,7 @@ func (w *workspaceListCmd) outputJSON(cmd *cobra.Command, instancesList []instan // Marshal to JSON with indentation jsonData, err := json.MarshalIndent(workspacesList, "", " ") if err != nil { - return fmt.Errorf("failed to marshal workspaces to JSON: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to marshal workspaces to JSON: %w", err)) } // Output the JSON to stdout @@ -152,7 +145,7 @@ kortex-cli workspace list -o json`, RunE: c.run, } - cmd.Flags().StringP("output", "o", "", "Output format (supported: json)") + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") return cmd } diff --git a/pkg/cmd/workspace_list_test.go b/pkg/cmd/workspace_list_test.go index 2f8dbb2..cec5d65 100644 --- a/pkg/cmd/workspace_list_test.go +++ b/pkg/cmd/workspace_list_test.go @@ -21,6 +21,7 @@ package cmd import ( "bytes" "encoding/json" + "os" "path/filepath" "strings" "testing" @@ -55,7 +56,6 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { c := &workspaceListCmd{} cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") args := []string{} @@ -74,10 +74,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") args := []string{} @@ -96,11 +97,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "json", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "json") args := []string{} @@ -119,11 +120,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "xml", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "xml") args := []string{} @@ -142,11 +143,11 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { storageDir := t.TempDir() - c := &workspaceListCmd{} + c := &workspaceListCmd{ + output: "yaml", + } cmd := &cobra.Command{} cmd.Flags().String("storage", storageDir, "test storage flag") - cmd.Flags().String("output", "", "test output flag") - cmd.Flags().Set("output", "yaml") args := []string{} @@ -159,6 +160,43 @@ func TestWorkspaceListCmd_PreRun(t *testing.T) { t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) } }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") + + c := &workspaceListCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestWorkspaceListCmd_E2E(t *testing.T) { diff --git a/pkg/cmd/workspace_remove.go b/pkg/cmd/workspace_remove.go index 75cb38e..4ef8158 100644 --- a/pkg/cmd/workspace_remove.go +++ b/pkg/cmd/workspace_remove.go @@ -19,10 +19,12 @@ package cmd import ( + "encoding/json" "errors" "fmt" "path/filepath" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" ) @@ -31,28 +33,41 @@ import ( type workspaceRemoveCmd struct { manager instances.Manager id string + output string } // preRun validates the parameters and flags func (w *workspaceRemoveCmd) preRun(cmd *cobra.Command, args []string) error { + // Validate output format if specified + if w.output != "" && w.output != "json" { + return fmt.Errorf("unsupported output format: %s (supported: json)", w.output) + } + + // Silence Cobra's error and usage output when JSON mode is enabled + // This prevents "Error: ..." and usage info from being printed + if w.output == "json" { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + w.id = args[0] // Get storage directory from global flag storageDir, err := cmd.Flags().GetString("storage") if err != nil { - return fmt.Errorf("failed to read --storage flag: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to read --storage flag: %w", err)) } // Normalize storage path to absolute path absStorageDir, err := filepath.Abs(storageDir) if err != nil { - return fmt.Errorf("failed to resolve absolute path for storage directory: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to resolve absolute path for storage directory: %w", err)) } // Create manager manager, err := instances.NewManager(absStorageDir) if err != nil { - return fmt.Errorf("failed to create manager: %w", err) + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to create manager: %w", err)) } w.manager = manager @@ -65,16 +80,40 @@ func (w *workspaceRemoveCmd) run(cmd *cobra.Command, args []string) error { err := w.manager.Delete(w.id) if err != nil { if errors.Is(err, instances.ErrInstanceNotFound) { + if w.output == "json" { + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("workspace not found: %s", w.id)) + } return fmt.Errorf("workspace not found: %s\nUse 'workspace list' to see available workspaces", w.id) } - return fmt.Errorf("failed to remove workspace: %w", err) + return outputErrorIfJSON(cmd, w.output, err) } - // Output only the ID + // Handle JSON output + if w.output == "json" { + return w.outputJSON(cmd) + } + + // Output only the ID (text mode) cmd.Println(w.id) return nil } +// outputJSON outputs the workspace ID as JSON +func (w *workspaceRemoveCmd) outputJSON(cmd *cobra.Command) error { + // Return only the ID (per OpenAPI spec) + workspaceId := api.WorkspaceId{ + Id: w.id, + } + + jsonData, err := json.MarshalIndent(workspaceId, "", " ") + if err != nil { + return outputErrorIfJSON(cmd, w.output, fmt.Errorf("failed to marshal to JSON: %w", err)) + } + + fmt.Fprintln(cmd.OutOrStdout(), string(jsonData)) + return nil +} + func NewWorkspaceRemoveCmd() *cobra.Command { c := &workspaceRemoveCmd{} @@ -89,5 +128,7 @@ kortex-cli workspace remove abc123`, RunE: c.run, } + cmd.Flags().StringVarP(&c.output, "output", "o", "", "Output format (supported: json)") + return cmd } diff --git a/pkg/cmd/workspace_remove_test.go b/pkg/cmd/workspace_remove_test.go index 9a8224b..29704a2 100644 --- a/pkg/cmd/workspace_remove_test.go +++ b/pkg/cmd/workspace_remove_test.go @@ -20,10 +20,13 @@ package cmd import ( "bytes" + "encoding/json" + "os" "path/filepath" "strings" "testing" + api "github.com/kortex-hub/kortex-cli-api/cli/go" "github.com/kortex-hub/kortex-cli/pkg/cmd/testutil" "github.com/kortex-hub/kortex-cli/pkg/instances" "github.com/spf13/cobra" @@ -69,6 +72,115 @@ func TestWorkspaceRemoveCmd_PreRun(t *testing.T) { t.Errorf("Expected id to be 'test-workspace-id', got %s", c.id) } }) + + t.Run("accepts empty output flag", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "" { + t.Errorf("Expected output to be empty, got %s", c.output) + } + }) + + t.Run("accepts json output format", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "json", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err != nil { + t.Fatalf("preRun() failed: %v", err) + } + + if c.output != "json" { + t.Errorf("Expected output to be 'json', got %s", c.output) + } + }) + + t.Run("rejects invalid output format", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + c := &workspaceRemoveCmd{ + output: "yaml", + } + cmd := &cobra.Command{} + cmd.Flags().String("storage", storageDir, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid output format") + } + + if !strings.Contains(err.Error(), "unsupported output format") { + t.Errorf("Expected error to contain 'unsupported output format', got: %v", err) + } + if !strings.Contains(err.Error(), "yaml") { + t.Errorf("Expected error to mention 'yaml', got: %v", err) + } + }) + + t.Run("outputs JSON error when manager creation fails with json output", func(t *testing.T) { + t.Parallel() + + tempDir := t.TempDir() + // Create a file and try to use it as a parent directory - will fail cross-platform + notADir := filepath.Join(tempDir, "file") + if err := os.WriteFile(notADir, []byte("test"), 0644); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + invalidStorage := filepath.Join(notADir, "subdir") + + c := &workspaceRemoveCmd{ + output: "json", + } + cmd := &cobra.Command{} + buf := new(bytes.Buffer) + cmd.SetOut(buf) + cmd.Flags().String("storage", invalidStorage, "test storage flag") + + args := []string{"test-id"} + + err := c.preRun(cmd, args) + if err == nil { + t.Fatal("Expected preRun() to fail with invalid storage path") + } + + // Verify JSON error was output + var errorResponse api.Error + if jsonErr := json.Unmarshal(buf.Bytes(), &errorResponse); jsonErr != nil { + t.Fatalf("Failed to unmarshal error JSON: %v\nOutput was: %s", jsonErr, buf.String()) + } + + if !strings.Contains(errorResponse.Error, "failed to create manager") { + t.Errorf("Expected error to contain 'failed to create manager', got: %s", errorResponse.Error) + } + }) } func TestWorkspaceRemoveCmd_E2E(t *testing.T) { @@ -327,6 +439,201 @@ func TestWorkspaceRemoveCmd_E2E(t *testing.T) { t.Errorf("Expected 0 instances after removal, got %d", len(instancesList)) } }) + + t.Run("json output returns workspace ID", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir := t.TempDir() + + // Create a workspace + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instance, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir, + ConfigDir: filepath.Join(sourcesDir, ".kortex"), + }) + if err != nil { + t.Fatalf("Failed to create instance: %v", err) + } + + addedInstance, err := manager.Add(instance) + if err != nil { + t.Fatalf("Failed to add instance: %v", err) + } + + instanceID := addedInstance.GetID() + + // Remove with JSON output + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", instanceID, "--storage", storageDir, "--output", "json"}) + + err = rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify ID matches + if workspaceId.Id != instanceID { + t.Errorf("Expected ID %s in JSON output, got %s", instanceID, workspaceId.Id) + } + + // Verify only ID field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["id"]; !exists { + t.Error("Expected 'id' field in JSON") + } + + // Verify workspace is actually removed + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 0 { + t.Errorf("Expected 0 instances after removal, got %d", len(instancesList)) + } + }) + + t.Run("json output error for non-existent workspace", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", "invalid-id", "--storage", storageDir, "--output", "json"}) + + err := rootCmd.Execute() + if err == nil { + t.Fatal("Expected Execute() to fail with non-existent workspace") + } + + // Parse JSON error output + var errorResponse api.Error + if err := json.Unmarshal(buf.Bytes(), &errorResponse); err != nil { + t.Fatalf("Failed to unmarshal error JSON: %v", err) + } + + // Verify error message + if !strings.Contains(errorResponse.Error, "workspace not found") { + t.Errorf("Expected error to contain 'workspace not found', got: %s", errorResponse.Error) + } + + if !strings.Contains(errorResponse.Error, "invalid-id") { + t.Errorf("Expected error to contain 'invalid-id', got: %s", errorResponse.Error) + } + + // Verify only error field exists + var parsed map[string]interface{} + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("Failed to unmarshal to map: %v", err) + } + + if len(parsed) != 1 { + t.Errorf("Expected only 1 field in error JSON, got %d: %v", len(parsed), parsed) + } + + if _, exists := parsed["error"]; !exists { + t.Error("Expected 'error' field in JSON") + } + }) + + t.Run("json output removes correct workspace when multiple exist", func(t *testing.T) { + t.Parallel() + + storageDir := t.TempDir() + sourcesDir1 := t.TempDir() + sourcesDir2 := t.TempDir() + + // Create two workspaces + manager, err := instances.NewManager(storageDir) + if err != nil { + t.Fatalf("Failed to create manager: %v", err) + } + + instance1, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir1, + ConfigDir: filepath.Join(sourcesDir1, ".kortex"), + }) + if err != nil { + t.Fatalf("Failed to create instance 1: %v", err) + } + + instance2, err := instances.NewInstance(instances.NewInstanceParams{ + SourceDir: sourcesDir2, + ConfigDir: filepath.Join(sourcesDir2, ".kortex"), + }) + if err != nil { + t.Fatalf("Failed to create instance 2: %v", err) + } + + addedInstance1, err := manager.Add(instance1) + if err != nil { + t.Fatalf("Failed to add instance 1: %v", err) + } + + addedInstance2, err := manager.Add(instance2) + if err != nil { + t.Fatalf("Failed to add instance 2: %v", err) + } + + // Remove first workspace with JSON output + rootCmd := NewRootCmd() + buf := new(bytes.Buffer) + rootCmd.SetOut(buf) + rootCmd.SetArgs([]string{"workspace", "remove", addedInstance1.GetID(), "--storage", storageDir, "--output", "json"}) + + err = rootCmd.Execute() + if err != nil { + t.Fatalf("Execute() failed: %v", err) + } + + // Parse JSON output + var workspaceId api.WorkspaceId + if err := json.Unmarshal(buf.Bytes(), &workspaceId); err != nil { + t.Fatalf("Failed to unmarshal JSON: %v", err) + } + + // Verify correct ID was removed + if workspaceId.Id != addedInstance1.GetID() { + t.Errorf("Expected ID %s in JSON output, got %s", addedInstance1.GetID(), workspaceId.Id) + } + + // Verify only instance2 remains + instancesList, err := manager.List() + if err != nil { + t.Fatalf("Failed to list instances: %v", err) + } + + if len(instancesList) != 1 { + t.Fatalf("Expected 1 instance after removal, got %d", len(instancesList)) + } + + if instancesList[0].GetID() != addedInstance2.GetID() { + t.Errorf("Expected remaining instance ID %s, got %s", addedInstance2.GetID(), instancesList[0].GetID()) + } + }) } func TestWorkspaceRemoveCmd_Examples(t *testing.T) {