From 4985d5bfceae7e574dbb975461b02e1a768d24c0 Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 14 Nov 2025 00:24:24 -0500 Subject: [PATCH 1/2] refactor: remove unnecessary facade methods from runtime package - Remove PrintEnvVars, PrintEnvVarsExport, and PrintAliases facade methods - Update cmd/env.go to call Shell methods directly - Remove associated tests for deleted facade methods --- cmd/check.go | 8 +- cmd/context.go | 4 +- cmd/env.go | 16 ++- cmd/exec.go | 4 +- pkg/runtime/runtime.go | 48 ------- pkg/runtime/runtime_test.go | 247 ------------------------------------ 6 files changed, 19 insertions(+), 308 deletions(-) diff --git a/cmd/check.go b/cmd/check.go index 1b9f919fd..063e9a56c 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -36,11 +36,11 @@ var checkCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.CheckTrustedDirectory(); err != nil { + if err := execCtx.Shell.CheckTrustedDirectory(); err != nil { return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return err } @@ -81,11 +81,11 @@ var checkNodeHealthCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.CheckTrustedDirectory(); err != nil { + if err := execCtx.Shell.CheckTrustedDirectory(); err != nil { return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return err } diff --git a/cmd/context.go b/cmd/context.go index 8577fa6e1..254415117 100644 --- a/cmd/context.go +++ b/cmd/context.go @@ -26,7 +26,7 @@ var getContextCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return fmt.Errorf("failed to load config: %w", err) } @@ -56,7 +56,7 @@ var setContextCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return fmt.Errorf("failed to load config: %w", err) } diff --git a/cmd/env.go b/cmd/env.go index 30574dfcd..593345114 100644 --- a/cmd/env.go +++ b/cmd/env.go @@ -36,7 +36,7 @@ var envCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.CheckTrustedDirectory(); err != nil { + if err := execCtx.Shell.CheckTrustedDirectory(); err != nil { return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") } @@ -44,7 +44,7 @@ var envCmd = &cobra.Command{ return err } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return err } @@ -62,10 +62,16 @@ var envCmd = &cobra.Command{ } if hook { - outputFunc(execCtx.PrintEnvVarsExport()) - outputFunc(execCtx.PrintAliases()) + if execCtx.Shell != nil && len(execCtx.GetEnvVars()) > 0 { + outputFunc(execCtx.Shell.RenderEnvVars(execCtx.GetEnvVars(), true)) + } + if execCtx.Shell != nil && len(execCtx.GetAliases()) > 0 { + outputFunc(execCtx.Shell.RenderAliases(execCtx.GetAliases())) + } } else { - outputFunc(execCtx.PrintEnvVars()) + if execCtx.Shell != nil && len(execCtx.GetEnvVars()) > 0 { + outputFunc(execCtx.Shell.RenderEnvVars(execCtx.GetEnvVars(), false)) + } } return nil diff --git a/cmd/exec.go b/cmd/exec.go index dce80b169..332c92bca 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -34,7 +34,7 @@ var execCmd = &cobra.Command{ return fmt.Errorf("failed to initialize context: %w", err) } - if err := execCtx.CheckTrustedDirectory(); err != nil { + if err := execCtx.Shell.CheckTrustedDirectory(); err != nil { return fmt.Errorf("not in a trusted directory. If you are in a Windsor project, run 'windsor init' to approve") } @@ -42,7 +42,7 @@ var execCmd = &cobra.Command{ return err } - if err := execCtx.LoadConfig(); err != nil { + if err := execCtx.ConfigHandler.LoadConfig(); err != nil { return err } diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 50c5c767b..23d74bb3f 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -163,28 +163,6 @@ func NewRuntime(ctx *Runtime) (*Runtime, error) { // Public Methods // ============================================================================= -// CheckTrustedDirectory verifies that the current directory is in the trusted file list. -// It delegates to the Shell's CheckTrustedDirectory method. Returns an error if the -// directory is not trusted or if Shell is not initialized. -func (ctx *Runtime) CheckTrustedDirectory() error { - if ctx.Shell == nil { - return fmt.Errorf("shell not initialized") - } - return ctx.Shell.CheckTrustedDirectory() -} - -// LoadConfig loads configuration from all sources. -// The context paths (ContextName, ProjectRoot, ConfigRoot, TemplateRoot) are already -// set up in the constructor, so this method only needs to load the configuration data. -// Returns an error if configuration loading fails or if required dependencies are missing. -func (ctx *Runtime) LoadConfig() error { - if ctx.ConfigHandler == nil { - return fmt.Errorf("config handler not initialized") - } - - return ctx.ConfigHandler.LoadConfig() -} - // HandleSessionReset checks for reset flags and session tokens, then resets managed environment // variables if needed. It checks for WINDSOR_SESSION_TOKEN and uses the shell's CheckResetFlags // method to determine if a reset should occur. If reset is needed, it calls Shell.Reset() and @@ -281,32 +259,6 @@ func (ctx *Runtime) LoadEnvironment(decrypt bool) error { return nil } -// PrintEnvVars returns all collected environment variables in key=value format. -// If no environment variables are loaded, returns an empty string. -func (ctx *Runtime) PrintEnvVars() string { - if ctx.Shell == nil || len(ctx.envVars) == 0 { - return "" - } - return ctx.Shell.RenderEnvVars(ctx.envVars, false) -} - -// PrintEnvVarsExport returns all collected environment variables in export key=value format. -// If no environment variables are loaded, returns an empty string. -func (ctx *Runtime) PrintEnvVarsExport() string { - if ctx.Shell == nil || len(ctx.envVars) == 0 { - return "" - } - return ctx.Shell.RenderEnvVars(ctx.envVars, true) -} - -// PrintAliases returns all collected aliases using the shell's RenderAliases method. -// If no aliases are loaded, returns an empty string. -func (ctx *Runtime) PrintAliases() string { - if ctx.Shell == nil || len(ctx.aliases) == 0 { - return "" - } - return ctx.Shell.RenderAliases(ctx.aliases) -} // GetEnvVars returns a copy of the collected environment variables. func (ctx *Runtime) GetEnvVars() map[string]string { diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index fe5364a2e..c485d11c2 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -307,75 +307,6 @@ func TestRuntime_LoadEnvironment(t *testing.T) { }) } -// ============================================================================= -// Test PrintEnvVars -// ============================================================================= - -func TestRuntime_PrintEnvVars(t *testing.T) { - t.Run("PrintsEnvironmentVariables", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - // Set up environment variables - ctx.envVars = map[string]string{ - "TEST_VAR1": "value1", - "TEST_VAR2": "value2", - } - - output := ctx.PrintEnvVarsExport() - - if output == "" { - t.Error("Expected output to be generated") - } - }) - - t.Run("HandlesEmptyEnvironmentVariables", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - output := ctx.PrintEnvVars() - - if output != "" { - t.Error("Expected no output for empty environment variables") - } - }) - -} - -// ============================================================================= -// Test PrintAliases -// ============================================================================= - -func TestRuntime_PrintAliases(t *testing.T) { - t.Run("PrintsAliases", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - // Set up aliases - ctx.aliases = map[string]string{ - "test1": "echo test1", - "test2": "echo test2", - } - - output := ctx.PrintAliases() - - if output == "" { - t.Error("Expected output to be generated") - } - }) - - t.Run("HandlesEmptyAliases", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - output := ctx.PrintAliases() - - if output != "" { - t.Error("Expected no output for empty aliases") - } - }) - -} // ============================================================================= @@ -650,80 +581,6 @@ func TestRuntime_LoadEnvironment_WithSecrets(t *testing.T) { }) } -func TestRuntime_PrintEnvVars_EdgeCases(t *testing.T) { - t.Run("HandlesNilShell", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.Shell = nil - - // This should not panic - result := ctx.PrintEnvVars() - if result != "" { - t.Errorf("Expected empty string with nil shell, got: %s", result) - } - }) - - t.Run("HandlesEmptyEnvVars", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.envVars = make(map[string]string) - - result := ctx.PrintEnvVars() - if result != "" { - t.Errorf("Expected empty string with empty env vars, got: %s", result) - } - }) -} - -func TestRuntime_PrintEnvVarsExport_EdgeCases(t *testing.T) { - t.Run("HandlesNilShell", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.Shell = nil - - // This should not panic - result := ctx.PrintEnvVarsExport() - if result != "" { - t.Errorf("Expected empty string with nil shell, got: %s", result) - } - }) - - t.Run("HandlesEmptyEnvVars", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.envVars = make(map[string]string) - - result := ctx.PrintEnvVarsExport() - if result != "" { - t.Errorf("Expected empty string with empty env vars, got: %s", result) - } - }) -} - -func TestRuntime_PrintAliases_EdgeCases(t *testing.T) { - t.Run("HandlesNilShell", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.Shell = nil - - // This should not panic - result := ctx.PrintAliases() - if result != "" { - t.Errorf("Expected empty string with nil shell, got: %s", result) - } - }) - - t.Run("HandlesEmptyAliases", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - ctx.aliases = make(map[string]string) - - result := ctx.PrintAliases() - if result != "" { - t.Errorf("Expected empty string with empty aliases, got: %s", result) - } - }) -} func TestRuntime_initializeComponents_EdgeCases(t *testing.T) { t.Run("HandlesToolsManagerInitializationError", func(t *testing.T) { @@ -986,110 +843,6 @@ func TestRuntime_CheckTools(t *testing.T) { } -func TestRuntime_CheckTrustedDirectory(t *testing.T) { - t.Run("Success", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - mockShell := mocks.Shell.(*shell.MockShell) - mockShell.CheckTrustedDirectoryFunc = func() error { - return nil - } - - err := ctx.CheckTrustedDirectory() - - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - }) - - t.Run("ErrorWhenShellNotInitialized", func(t *testing.T) { - ctx := &Runtime{} - - err := ctx.CheckTrustedDirectory() - - if err == nil { - t.Error("Expected error when Shell is nil") - } - - if !strings.Contains(err.Error(), "shell not initialized") { - t.Errorf("Expected error about shell not initialized, got: %v", err) - } - }) - - t.Run("ErrorWhenCheckFails", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - mockShell := mocks.Shell.(*shell.MockShell) - mockShell.CheckTrustedDirectoryFunc = func() error { - return fmt.Errorf("directory not trusted") - } - - err := ctx.CheckTrustedDirectory() - - if err == nil { - t.Error("Expected error when check fails") - } - - if !strings.Contains(err.Error(), "directory not trusted") { - t.Errorf("Expected error about directory not trusted, got: %v", err) - } - }) -} - -func TestRuntime_LoadConfig(t *testing.T) { - t.Run("Success", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) - mockConfigHandler.LoadConfigFunc = func() error { - return nil - } - - err := ctx.LoadConfig() - - if err != nil { - t.Errorf("Expected no error, got: %v", err) - } - }) - - t.Run("ErrorWhenConfigHandlerNotInitialized", func(t *testing.T) { - ctx := &Runtime{} - - err := ctx.LoadConfig() - - if err == nil { - t.Error("Expected error when ConfigHandler is nil") - } - - if !strings.Contains(err.Error(), "config handler not initialized") { - t.Errorf("Expected error about config handler not initialized, got: %v", err) - } - }) - - t.Run("ErrorWhenLoadConfigFails", func(t *testing.T) { - mocks := setupEnvironmentMocks(t) - ctx := mocks.Runtime - - mockConfigHandler := mocks.ConfigHandler.(*config.MockConfigHandler) - mockConfigHandler.LoadConfigFunc = func() error { - return fmt.Errorf("load config failed") - } - - err := ctx.LoadConfig() - - if err == nil { - t.Error("Expected error when LoadConfig fails") - } - - if !strings.Contains(err.Error(), "load config failed") { - t.Errorf("Expected error about load config failed, got: %v", err) - } - }) -} - func TestRuntime_HandleSessionReset(t *testing.T) { t.Run("ResetsWhenNoSessionToken", func(t *testing.T) { t.Cleanup(func() { From bacd0928d0af6fcbf9a6a5fca62d09b1698afb8e Mon Sep 17 00:00:00 2001 From: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> Date: Fri, 14 Nov 2025 00:42:18 -0500 Subject: [PATCH 2/2] Add GenerateBlueprint facade Signed-off-by: Ryan VanGundy <85766511+rmvangun@users.noreply.github.com> --- cmd/down.go | 10 ++++++++-- cmd/install.go | 5 ++++- cmd/up.go | 5 ++++- pkg/composer/composer.go | 14 ++++++++++++++ pkg/composer/composer_test.go | 15 --------------- 5 files changed, 30 insertions(+), 19 deletions(-) diff --git a/cmd/down.go b/cmd/down.go index c48ad1124..875b5191d 100644 --- a/cmd/down.go +++ b/cmd/down.go @@ -45,7 +45,10 @@ var downCmd = &cobra.Command{ } if !skipK8sFlag { - blueprint := proj.Composer.BlueprintHandler.Generate() + blueprint, err := proj.Composer.GenerateBlueprint() + if err != nil { + return fmt.Errorf("error generating blueprint: %w", err) + } if err := proj.Provisioner.Uninstall(blueprint); err != nil { return fmt.Errorf("error running blueprint cleanup: %w", err) } @@ -54,7 +57,10 @@ var downCmd = &cobra.Command{ } if !skipTerraformFlag { - blueprint := proj.Composer.BlueprintHandler.Generate() + blueprint, err := proj.Composer.GenerateBlueprint() + if err != nil { + return fmt.Errorf("error generating blueprint: %w", err) + } if err := proj.Provisioner.Down(blueprint); err != nil { return fmt.Errorf("error tearing down infrastructure: %w", err) } diff --git a/cmd/install.go b/cmd/install.go index bee132423..4649e5f10 100644 --- a/cmd/install.go +++ b/cmd/install.go @@ -37,7 +37,10 @@ var installCmd = &cobra.Command{ return err } - blueprint := proj.Composer.BlueprintHandler.Generate() + blueprint, err := proj.Composer.GenerateBlueprint() + if err != nil { + return fmt.Errorf("error generating blueprint: %w", err) + } if err := proj.Provisioner.Install(blueprint); err != nil { return fmt.Errorf("error installing blueprint: %w", err) } diff --git a/cmd/up.go b/cmd/up.go index f34605090..ad2429875 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -47,7 +47,10 @@ var upCmd = &cobra.Command{ } } - blueprint := proj.Composer.BlueprintHandler.Generate() + blueprint, err := proj.Composer.GenerateBlueprint() + if err != nil { + return fmt.Errorf("error generating blueprint: %w", err) + } if err := proj.Provisioner.Up(blueprint); err != nil { return fmt.Errorf("error starting infrastructure: %w", err) } diff --git a/pkg/composer/composer.go b/pkg/composer/composer.go index 8281e4c68..f7d3a802d 100644 --- a/pkg/composer/composer.go +++ b/pkg/composer/composer.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" + blueprintv1alpha1 "github.com/windsorcli/cli/api/v1alpha1" "github.com/windsorcli/cli/pkg/composer/artifact" "github.com/windsorcli/cli/pkg/composer/blueprint" "github.com/windsorcli/cli/pkg/composer/terraform" @@ -159,6 +160,19 @@ func (r *Composer) Generate(overwrite ...bool) error { return nil } +// GenerateBlueprint generates and returns the blueprint from the blueprint handler. +// It initializes the blueprint handler if needed and loads the blueprint data before generating. +// Returns the generated blueprint or an error if initialization or loading fails. +func (r *Composer) GenerateBlueprint() (*blueprintv1alpha1.Blueprint, error) { + if err := r.BlueprintHandler.Initialize(); err != nil { + return nil, fmt.Errorf("failed to initialize blueprint handler: %w", err) + } + if err := r.BlueprintHandler.LoadBlueprint(); err != nil { + return nil, fmt.Errorf("failed to load blueprint data: %w", err) + } + return r.BlueprintHandler.Generate(), nil +} + // ============================================================================= // Private Methods // ============================================================================= diff --git a/pkg/composer/composer_test.go b/pkg/composer/composer_test.go index 24d74121a..68b62f237 100644 --- a/pkg/composer/composer_test.go +++ b/pkg/composer/composer_test.go @@ -121,21 +121,6 @@ func TestCreateComposer(t *testing.T) { // Test Public Methods // ============================================================================= -func TestComposer_Bundle(t *testing.T) { - t.Run("HandlesBundleSuccessfully", func(t *testing.T) { - mocks := setupComposerMocks(t) - composer := NewComposer(mocks.ComposerRuntime) - - // This test would need proper mocking of the artifact builder - // For now, we'll just test that the method exists and handles errors - _, err := composer.Bundle("/test/output", "v1.0.0") - // We expect an error here because we don't have proper mocks set up - if err == nil { - t.Error("Expected error due to missing mocks, but got nil") - } - }) -} - func TestComposer_Push(t *testing.T) { t.Run("HandlesPushSuccessfully", func(t *testing.T) { mocks := setupComposerMocks(t)