From 9812aa2b7d456c3d47770a87529d7175024fa350 Mon Sep 17 00:00:00 2001 From: indaco Date: Wed, 11 Mar 2026 07:50:36 +0100 Subject: [PATCH 1/2] refactor(plugins): remove per-package registry singletons and legacy global func vars Replace mutable var XxxFn globals with direct function calls --- cmd/sley/main.go | 4 +- cmd/sley/main_test.go | 43 ----- internal/clix/clix.go | 6 +- internal/clix/clix_test.go | 4 +- internal/commands/bump/auto.go | 4 +- internal/commands/bump/bump_auto_test.go | 105 +++--------- internal/commands/bump/bump_basic_test.go | 140 +++++++--------- internal/commands/bump/bump_plugins_test.go | 90 +---------- internal/commands/bump/bump_pre_test.go | 22 +-- internal/commands/bump/common.go | 2 +- internal/commands/bump/major.go | 2 +- internal/commands/bump/minor.go | 2 +- internal/commands/bump/patch.go | 2 +- internal/commands/bump/pre.go | 2 +- internal/commands/bump/release.go | 2 +- internal/commands/doctor/doctorcmd.go | 2 +- internal/commands/extension/list.go | 2 +- internal/commands/extension/list_test.go | 36 +++-- internal/commands/pre/precmd_test.go | 2 +- internal/config/config.go | 17 +- internal/config/config_extensions_test.go | 10 +- internal/config/config_load_test.go | 24 +-- internal/config/config_plugins_test.go | 10 +- internal/config/config_save_test.go | 8 +- internal/config/config_workspace_test.go | 16 +- internal/config/validator_plugins.go | 2 +- internal/extensionmgr/addextension_test.go | 2 +- internal/extensionmgr/integration_test.go | 2 +- internal/extensionmgr/register_test.go | 8 +- internal/hooks/hooks.go | 9 +- internal/hooks/hooks_test.go | 2 +- internal/hooks/loader.go | 5 +- internal/hooks/loader_test.go | 8 +- internal/plugins/auditlog/config.go | 19 +++ internal/plugins/auditlog/registry.go | 57 ------- internal/plugins/auditlog/registry_test.go | 112 ------------- .../plugins/changeloggenerator/registry.go | 45 ------ .../changeloggenerator/registry_test.go | 153 ------------------ .../plugins/changelogparser/plugin_test.go | 67 -------- internal/plugins/changelogparser/registry.go | 42 ----- .../plugins/changelogparser/registry_test.go | 48 ------ internal/plugins/commitparser/plugin.go | 9 -- internal/plugins/commitparser/plugin_test.go | 16 -- internal/plugins/commitparser/registry.go | 31 ---- .../plugins/commitparser/registry_test.go | 137 ---------------- .../plugins/dependencycheck/plugin_test.go | 58 ------- internal/plugins/dependencycheck/registry.go | 29 ---- .../plugins/dependencycheck/registry_test.go | 61 ------- internal/plugins/loader_test.go | 87 ---------- internal/plugins/releasegate/registry.go | 24 --- internal/plugins/releasegate/registry_test.go | 48 ------ .../plugins/releasegate/releasegate_test.go | 48 ------ internal/plugins/tagmanager/plugin_test.go | 68 -------- internal/plugins/tagmanager/registry.go | 37 ----- .../plugins/versionvalidator/plugin_test.go | 30 ---- internal/plugins/versionvalidator/registry.go | 24 --- .../plugins/versionvalidator/registry_test.go | 48 ------ 57 files changed, 205 insertions(+), 1688 deletions(-) delete mode 100644 internal/plugins/auditlog/registry.go delete mode 100644 internal/plugins/auditlog/registry_test.go delete mode 100644 internal/plugins/changeloggenerator/registry.go delete mode 100644 internal/plugins/changeloggenerator/registry_test.go delete mode 100644 internal/plugins/changelogparser/registry.go delete mode 100644 internal/plugins/changelogparser/registry_test.go delete mode 100644 internal/plugins/commitparser/registry.go delete mode 100644 internal/plugins/commitparser/registry_test.go delete mode 100644 internal/plugins/dependencycheck/registry.go delete mode 100644 internal/plugins/dependencycheck/registry_test.go delete mode 100644 internal/plugins/releasegate/registry.go delete mode 100644 internal/plugins/releasegate/registry_test.go delete mode 100644 internal/plugins/tagmanager/registry.go delete mode 100644 internal/plugins/versionvalidator/registry.go delete mode 100644 internal/plugins/versionvalidator/registry_test.go diff --git a/cmd/sley/main.go b/cmd/sley/main.go index 74517e34..ae4b5c39 100644 --- a/cmd/sley/main.go +++ b/cmd/sley/main.go @@ -19,7 +19,7 @@ func main() { } func runCLI(args []string) error { - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { return err } @@ -37,7 +37,7 @@ func runCLI(args []string) error { registry := plugins.NewPluginRegistry() plugins.RegisterBuiltinPlugins(cfg, registry) - if err := hooks.LoadPreReleaseHooksFromConfigFn(cfg); err != nil { + if err := hooks.LoadPreReleaseHooksFromConfig(cfg); err != nil { return fmt.Errorf("failed to load pre-release hooks: %w", err) } diff --git a/cmd/sley/main_test.go b/cmd/sley/main_test.go index 0987eb3f..3d9312a0 100644 --- a/cmd/sley/main_test.go +++ b/cmd/sley/main_test.go @@ -2,14 +2,10 @@ package main import ( "bytes" - "fmt" "os" "path/filepath" "strings" "testing" - - "github.com/indaco/sley/internal/config" - "github.com/indaco/sley/internal/hooks" ) func TestRunMain_ShowVersion(t *testing.T) { @@ -128,42 +124,3 @@ func TestRunMain_LoadConfigError(t *testing.T) { t.Errorf("unexpected error: %v", err) } } - -func TestRunMain_LoadPreReleaseHooksError(t *testing.T) { - tmp := t.TempDir() - - versionPath := filepath.Join(tmp, ".version") - if err := os.WriteFile(versionPath, []byte("1.2.3\n"), 0600); err != nil { - t.Fatal(err) - } - - origDir, err := os.Getwd() - if err != nil { - t.Fatal(err) - } - if err := os.Chdir(tmp); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - if err := os.Chdir(origDir); err != nil { - t.Fatalf("failed to restore working directory: %v", err) - } - }) - - // Backup and override hooks.LoadPreReleaseHooksFromConfig - originalFn := hooks.LoadPreReleaseHooksFromConfigFn - hooks.LoadPreReleaseHooksFromConfigFn = func(cfg *config.Config) error { - return fmt.Errorf("mock pre-release hook load error") - } - t.Cleanup(func() { - hooks.LoadPreReleaseHooksFromConfigFn = originalFn - }) - - err = runCLI([]string{"sley", "bump", "patch"}) - if err == nil { - t.Fatal("expected error from LoadPreReleaseHooksFromConfig, got nil") - } - if !strings.Contains(err.Error(), "failed to load pre-release hooks") { - t.Errorf("unexpected error: %v", err) - } -} diff --git a/internal/clix/clix.go b/internal/clix/clix.go index 440ac1be..9a5a5568 100644 --- a/internal/clix/clix.go +++ b/internal/clix/clix.go @@ -16,11 +16,9 @@ import ( "github.com/urfave/cli/v3" ) -var FromCommandFn = fromCommand - -// fromCommand extracts the --path and --strict flags from a cli.Command, +// FromCommand extracts the --path and --strict flags from a cli.Command, // and passes them to GetOrInitVersionFile. -func fromCommand(cmd *cli.Command) (bool, error) { +func FromCommand(cmd *cli.Command) (bool, error) { return GetOrInitVersionFile(cmd.String("path"), cmd.Bool("strict")) } diff --git a/internal/clix/clix_test.go b/internal/clix/clix_test.go index a5a0c801..d2047572 100644 --- a/internal/clix/clix_test.go +++ b/internal/clix/clix_test.go @@ -90,7 +90,7 @@ func TestFromCommand(t *testing.T) { cfg := &config.Config{Path: tmpFile} appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{}) - created, err := FromCommandFn(appCli) + created, err := FromCommand(appCli) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -106,7 +106,7 @@ func TestFromCommand(t *testing.T) { cfg := &config.Config{Path: targetPath} appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{}) - created, err := FromCommandFn(appCli) + created, err := FromCommand(appCli) if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/commands/bump/auto.go b/internal/commands/bump/auto.go index b646d67a..779a2751 100644 --- a/internal/commands/bump/auto.go +++ b/internal/commands/bump/auto.go @@ -91,7 +91,7 @@ func runBumpAuto(ctx context.Context, cfg *config.Config, registry *plugins.Plug disableInfer := isNoInferFlag || (cfg != nil && cfg.Plugins != nil && !cfg.Plugins.CommitParser) // Run pre-release hooks first (before any version operations) - if err := hooks.RunPreReleaseHooksFn(ctx, isSkipHooks); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, isSkipHooks); err != nil { return err } @@ -152,7 +152,7 @@ func determineBumpType(registry *plugins.PluginRegistry, label string, disableIn // runSingleModuleAuto handles the single-module auto bump operation. func runSingleModuleAuto(ctx context.Context, cmd *cli.Command, cfg *config.Config, registry *plugins.PluginRegistry, path, label, meta, since, until string, isPreserveMeta, disableInfer, skipHooks bool) error { - if _, err := clix.FromCommandFn(cmd); err != nil { + if _, err := clix.FromCommand(cmd); err != nil { return err } diff --git a/internal/commands/bump/bump_auto_test.go b/internal/commands/bump/bump_auto_test.go index b8507a6a..e5597f77 100644 --- a/internal/commands/bump/bump_auto_test.go +++ b/internal/commands/bump/bump_auto_test.go @@ -11,7 +11,6 @@ import ( "github.com/indaco/sley/internal/config" "github.com/indaco/sley/internal/plugins" - "github.com/indaco/sley/internal/plugins/commitparser" "github.com/indaco/sley/internal/plugins/commitparser/gitlog" "github.com/indaco/sley/internal/plugins/tagmanager" "github.com/indaco/sley/internal/semver" @@ -308,21 +307,20 @@ func TestTryInferBumpTypeFromCommitParserPlugin_GetCommitsError(t *testing.T) { testutils.WithMock(func() { // Mock GetCommits to fail originalGetCommits := gitlog.GetCommitsFn - originalParser := commitparser.GetCommitParserFn gitlog.GetCommitsFn = func(since, until string) ([]string, error) { return nil, fmt.Errorf("simulated gitlog error") } - commitparser.GetCommitParserFn = func() commitparser.CommitParser { - return testutils.MockCommitParser{} // Return any parser - } t.Cleanup(func() { gitlog.GetCommitsFn = originalGetCommits - commitparser.GetCommitParserFn = originalParser }) }, func() { registry := plugins.NewPluginRegistry() + parser := testutils.MockCommitParser{} + if err := registry.RegisterCommitParser(&parser); err != nil { + t.Fatalf("failed to register parser: %v", err) + } label := tryInferBumpTypeFromCommitParserPlugin(registry, "", "") if label != "" { t.Errorf("expected empty label on gitlog error, got %q", label) @@ -337,12 +335,13 @@ func TestTryInferBumpTypeFromCommitParserPlugin_ParserError(t *testing.T) { gitlog.GetCommitsFn = func(since, until string) ([]string, error) { return []string{"fix: something"}, nil } - commitparser.GetCommitParserFn = func() commitparser.CommitParser { - return testutils.MockCommitParser{Err: fmt.Errorf("parser error")} - } }, func() { registry := plugins.NewPluginRegistry() + parser := testutils.MockCommitParser{Err: fmt.Errorf("parser error")} + if err := registry.RegisterCommitParser(&parser); err != nil { + t.Fatalf("failed to register parser: %v", err) + } label := tryInferBumpTypeFromCommitParserPlugin(registry, "", "") if label != "" { t.Errorf("expected empty label on parser error, got %q", label) @@ -695,14 +694,8 @@ func TestGetNextVersion(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestBumpAuto_CallsCreateTagAfterBump_WithEnabledTagManager(t *testing.T) { - // This test verifies that createTagAfterBump is called when tag manager is enabled - // by testing the function directly with the "auto" bump type parameter version := semver.SemVersion{Major: 99, Minor: 88, Patch: 77} - // Save original function and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - // Create an enabled tag manager plugin plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: true, @@ -710,41 +703,27 @@ func TestBumpAuto_CallsCreateTagAfterBump_WithEnabledTagManager(t *testing.T) { Prefix: "v", Annotate: true, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } registry := plugins.NewPluginRegistry() + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } - // Call createTagAfterBump directly with "auto" bump type - // This is the call that runSingleModuleAuto makes at the end err := createTagAfterBump(registry, version, "auto") - // The test environment may have various outcomes depending on git state - // The important thing is that the function is called and attempts tag creation if err != nil { errStr := err.Error() - // These are acceptable errors when running in a test environment - if !strings.Contains(errStr, "failed to create tag") && !strings.Contains(errStr, "already exists") { - t.Fatalf("expected tag creation error, tag exists error, or no error, got: %v", err) + if !strings.Contains(errStr, "failed to create tag") && !strings.Contains(errStr, "already exists") && !strings.Contains(errStr, "failed to commit") { + t.Fatalf("expected tag creation error, tag exists error, commit error, or no error, got: %v", err) } } } func TestBumpAuto_EndToEnd_WithMockTagManager(t *testing.T) { - // This test verifies the full bump auto flow with a mock tag manager - // that tracks whether CreateTag is called tmpDir := t.TempDir() versionPath := filepath.Join(tmpDir, ".version") testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") - // Save original function and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - // Create a mock that tracks calls - note: createTagAfterBump uses type assertion - // to *tagmanager.TagManagerPlugin, so mocks will be treated as disabled - // We use this to verify the flow works when tag manager returns nil for registry - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil } - cfg := &config.Config{Path: versionPath} registry := plugins.NewPluginRegistry() appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)}) @@ -757,7 +736,6 @@ func TestBumpAuto_EndToEnd_WithMockTagManager(t *testing.T) { t.Fatalf("expected no error, got: %v", err) } - // Verify the version was bumped got := testutils.ReadTempVersionFile(t, tmpDir) want := "1.2.4" if got != want { @@ -770,16 +748,10 @@ func TestBumpAuto_SkipsTagCreation_WhenTagManagerDisabled(t *testing.T) { versionPath := filepath.Join(tmpDir, ".version") testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") - // Save original function and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - // Create a disabled tag manager plugin plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: false, AutoCreate: false, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } cfg := &config.Config{Path: versionPath} registry := plugins.NewPluginRegistry() @@ -808,13 +780,6 @@ func TestBumpAuto_SkipsTagCreation_WhenNoTagManagerRegistered(t *testing.T) { versionPath := filepath.Join(tmpDir, ".version") testutils.WriteTempVersionFile(t, tmpDir, "1.2.3-alpha.1") - // Save original function and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - // No tag manager registered - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil } - cfg := &config.Config{Path: versionPath} registry := plugins.NewPluginRegistry() appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)}) @@ -838,35 +803,24 @@ func TestBumpAuto_TagCreatedWithCorrectParameters(t *testing.T) { version := semver.SemVersion{Major: 1, Minor: 2, Patch: 4} t.Run("calls createTagAfterBump with auto bump type", func(t *testing.T) { - // Save original and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - // Create enabled tag manager plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: true, AutoCreate: true, Prefix: "v", }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } registry := plugins.NewPluginRegistry() - // Note: createTagAfterBump checks for *TagManagerPlugin type assertion - // When using a real plugin, it will try to create a tag (which fails without git) + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } err := createTagAfterBump(registry, version, "auto") - // In test environment without git, we expect a tag creation error - if err != nil && !strings.Contains(err.Error(), "failed to create tag") { + if err != nil && !strings.Contains(err.Error(), "failed to create tag") && !strings.Contains(err.Error(), "failed to commit") { t.Errorf("unexpected error type: %v", err) } }) t.Run("returns nil when tag manager is nil", func(t *testing.T) { - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil } - registry := plugins.NewPluginRegistry() err := createTagAfterBump(registry, version, "auto") if err != nil { @@ -875,16 +829,15 @@ func TestBumpAuto_TagCreatedWithCorrectParameters(t *testing.T) { }) t.Run("returns nil when tag manager is disabled", func(t *testing.T) { - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: false, AutoCreate: false, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } registry := plugins.NewPluginRegistry() + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } err := createTagAfterBump(registry, version, "auto") if err != nil { t.Errorf("expected nil error when tag manager is disabled, got: %v", err) @@ -897,19 +850,16 @@ func TestBumpAuto_TagCreation_OnPreReleasePromotion(t *testing.T) { versionPath := filepath.Join(tmpDir, ".version") testutils.WriteTempVersionFile(t, tmpDir, "2.0.0-rc.1") - // Save original function and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - - // Create a disabled tag manager to verify bump succeeds without tag creation plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: false, AutoCreate: false, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } cfg := &config.Config{Path: versionPath} registry := plugins.NewPluginRegistry() + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)}) err := appCli.Run(context.Background(), []string{ @@ -932,28 +882,25 @@ func TestBumpAuto_InferredMinorBump_WithTagManager(t *testing.T) { versionPath := filepath.Join(tmpDir, ".version") testutils.WriteTempVersionFile(t, tmpDir, "1.0.0") - // Save and restore original functions - origGetTagManagerFn := tagmanager.GetTagManagerFn originalInfer := tryInferBumpTypeFromCommitParserPluginFn defer func() { - tagmanager.GetTagManagerFn = origGetTagManagerFn tryInferBumpTypeFromCommitParserPluginFn = originalInfer }() - // Mock inference to return "minor" tryInferBumpTypeFromCommitParserPluginFn = func(registry *plugins.PluginRegistry, since, until string) string { return "minor" } - // Create a disabled tag manager (to avoid git dependency in tests) plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: false, AutoCreate: false, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } cfg := &config.Config{Path: versionPath, Plugins: &config.PluginConfig{CommitParser: true}} registry := plugins.NewPluginRegistry() + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)}) err := appCli.Run(context.Background(), []string{ diff --git a/internal/commands/bump/bump_basic_test.go b/internal/commands/bump/bump_basic_test.go index dc93f35e..928d9519 100644 --- a/internal/commands/bump/bump_basic_test.go +++ b/internal/commands/bump/bump_basic_test.go @@ -7,7 +7,6 @@ import ( "strings" "testing" - "github.com/indaco/sley/internal/clix" "github.com/indaco/sley/internal/config" "github.com/indaco/sley/internal/hooks" "github.com/indaco/sley/internal/plugins" @@ -86,131 +85,106 @@ func TestCLI_BumpCommand_AutoInitFeedback(t *testing.T) { } } +// errorHook is a PreReleaseHook that always returns an error. +type errorHook struct{} + +func (errorHook) HookName() string { return "error-hook" } +func (errorHook) Run(_ context.Context) error { + return fmt.Errorf("mock pre-release hooks error") +} + func TestCLI_BumpSubcommands_EarlyFailures(t *testing.T) { tests := []struct { name string args []string - override func() func() // returns restore function + setup func(t *testing.T, tmpDir string) expectedErr string }{ { - name: "patch - FromCommand fails", - args: []string{"sley", "bump", "patch"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } + name: "patch - FromCommand fails (strict + missing file)", + args: []string{"sley", "bump", "patch", "--strict"}, + setup: func(t *testing.T, tmpDir string) { + // Do NOT create .version file so strict mode fails }, - expectedErr: "mock FromCommand error", + expectedErr: "version file not found", }, { name: "patch - RunPreReleaseHooks fails", args: []string{"sley", "bump", "patch"}, - override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + setup: func(t *testing.T, tmpDir string) { + testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + t.Cleanup(func() { hooks.ResetPreReleaseHooks() }) }, expectedErr: "mock pre-release hooks error", }, { - name: "minor - FromCommand fails", - args: []string{"sley", "bump", "minor"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } - }, - expectedErr: "mock FromCommand error", + name: "minor - FromCommand fails (strict + missing file)", + args: []string{"sley", "bump", "minor", "--strict"}, + setup: func(t *testing.T, tmpDir string) {}, + expectedErr: "version file not found", }, { name: "minor - RunPreReleaseHooks fails", args: []string{"sley", "bump", "minor"}, - override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + setup: func(t *testing.T, tmpDir string) { + testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + t.Cleanup(func() { hooks.ResetPreReleaseHooks() }) }, expectedErr: "mock pre-release hooks error", }, { - name: "major - FromCommand fails", - args: []string{"sley", "bump", "major"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } - }, - expectedErr: "mock FromCommand error", + name: "major - FromCommand fails (strict + missing file)", + args: []string{"sley", "bump", "major", "--strict"}, + setup: func(t *testing.T, tmpDir string) {}, + expectedErr: "version file not found", }, { name: "major - RunPreReleaseHooks fails", args: []string{"sley", "bump", "major"}, - override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + setup: func(t *testing.T, tmpDir string) { + testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + t.Cleanup(func() { hooks.ResetPreReleaseHooks() }) }, expectedErr: "mock pre-release hooks error", }, { - name: "auto - FromCommand fails", - args: []string{"sley", "bump", "auto"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } - }, - expectedErr: "mock FromCommand error", + name: "auto - FromCommand fails (strict + missing file)", + args: []string{"sley", "bump", "auto", "--strict"}, + setup: func(t *testing.T, tmpDir string) {}, + expectedErr: "version file not found", }, { name: "auto - RunPreReleaseHooks fails", args: []string{"sley", "bump", "auto"}, - override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + setup: func(t *testing.T, tmpDir string) { + testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + t.Cleanup(func() { hooks.ResetPreReleaseHooks() }) }, expectedErr: "mock pre-release hooks error", }, { - name: "release - FromCommand fails", - args: []string{"sley", "bump", "release"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } - }, - expectedErr: "mock FromCommand error", + name: "release - FromCommand fails (strict + missing file)", + args: []string{"sley", "bump", "release", "--strict"}, + setup: func(t *testing.T, tmpDir string) {}, + expectedErr: "version file not found", }, { name: "release - RunPreReleaseHooks fails", args: []string{"sley", "bump", "release"}, - override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + setup: func(t *testing.T, tmpDir string) { + testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + t.Cleanup(func() { hooks.ResetPreReleaseHooks() }) }, expectedErr: "mock pre-release hooks error", }, @@ -220,10 +194,8 @@ func TestCLI_BumpSubcommands_EarlyFailures(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tmpDir := t.TempDir() versionPath := filepath.Join(tmpDir, ".version") - testutils.WriteTempVersionFile(t, tmpDir, "1.2.3") - restore := tt.override() - defer restore() + tt.setup(t, tmpDir) cfg := &config.Config{Path: versionPath} registry := plugins.NewPluginRegistry() diff --git a/internal/commands/bump/bump_plugins_test.go b/internal/commands/bump/bump_plugins_test.go index fb0e9f40..c42737a3 100644 --- a/internal/commands/bump/bump_plugins_test.go +++ b/internal/commands/bump/bump_plugins_test.go @@ -11,12 +11,8 @@ import ( "github.com/indaco/sley/internal/commands/depsync" "github.com/indaco/sley/internal/config" "github.com/indaco/sley/internal/plugins" - "github.com/indaco/sley/internal/plugins/auditlog" - "github.com/indaco/sley/internal/plugins/changeloggenerator" "github.com/indaco/sley/internal/plugins/dependencycheck" - "github.com/indaco/sley/internal/plugins/releasegate" "github.com/indaco/sley/internal/plugins/tagmanager" - "github.com/indaco/sley/internal/plugins/versionvalidator" "github.com/indaco/sley/internal/semver" "github.com/indaco/sley/internal/testutils" "github.com/urfave/cli/v3" @@ -27,25 +23,10 @@ import ( /* ------------------------------------------------------------------------- */ func TestValidateTagAvailable(t *testing.T) { - // Save original and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - version := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil tag manager returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil } - err := validateTagAvailable(registry, version) - if err != nil { - t.Errorf("expected nil error, got %v", err) - } - }) - - t.Run("mock tag manager validates", func(t *testing.T) { - registry := plugins.NewPluginRegistry() - mock := &mockTagManager{} - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return mock } err := validateTagAvailable(registry, version) if err != nil { t.Errorf("expected nil error, got %v", err) @@ -70,15 +51,10 @@ func TestValidateTagAvailable(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestCreateTagAfterBump(t *testing.T) { - // Save original and restore after test - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - version := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil tag manager returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil } err := createTagAfterBump(registry, version, "minor") if err != nil { t.Errorf("expected nil error, got %v", err) @@ -94,26 +70,11 @@ func TestCreateTagAfterBump(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestValidateVersionPolicy(t *testing.T) { - // Save original and restore after test - origGetVersionValidatorFn := versionvalidator.GetVersionValidatorFn - defer func() { versionvalidator.GetVersionValidatorFn = origGetVersionValidatorFn }() - newVersion := semver.SemVersion{Major: 2, Minor: 0, Patch: 0} prevVersion := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil validator returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - versionvalidator.GetVersionValidatorFn = func() versionvalidator.VersionValidator { return nil } - err := validateVersionPolicy(registry, newVersion, prevVersion, "major") - if err != nil { - t.Errorf("expected nil error, got %v", err) - } - }) - - t.Run("mock validator validates successfully", func(t *testing.T) { - registry := plugins.NewPluginRegistry() - mock := &mockVersionValidator{} - versionvalidator.GetVersionValidatorFn = func() versionvalidator.VersionValidator { return mock } err := validateVersionPolicy(registry, newVersion, prevVersion, "major") if err != nil { t.Errorf("expected nil error, got %v", err) @@ -138,26 +99,11 @@ func TestValidateVersionPolicy(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestValidateReleaseGate(t *testing.T) { - // Save original and restore after test - origGetReleaseGateFn := releasegate.GetReleaseGateFn - defer func() { releasegate.GetReleaseGateFn = origGetReleaseGateFn }() - newVersion := semver.SemVersion{Major: 2, Minor: 0, Patch: 0} prevVersion := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil gate returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - releasegate.GetReleaseGateFn = func() releasegate.ReleaseGate { return nil } - err := validateReleaseGate(registry, newVersion, prevVersion, "major") - if err != nil { - t.Errorf("expected nil error, got %v", err) - } - }) - - t.Run("mock gate validates successfully", func(t *testing.T) { - registry := plugins.NewPluginRegistry() - mock := &mockReleaseGate{} - releasegate.GetReleaseGateFn = func() releasegate.ReleaseGate { return mock } err := validateReleaseGate(registry, newVersion, prevVersion, "major") if err != nil { t.Errorf("expected nil error, got %v", err) @@ -182,15 +128,10 @@ func TestValidateReleaseGate(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestValidateDependencyConsistency(t *testing.T) { - // Save original and restore after test - origGetDependencyCheckerFn := dependencycheck.GetDependencyCheckerFn - defer func() { dependencycheck.GetDependencyCheckerFn = origGetDependencyCheckerFn }() - version := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil checker returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - dependencycheck.GetDependencyCheckerFn = func() dependencycheck.DependencyChecker { return nil } err := validateDependencyConsistency(registry, version) if err != nil { t.Errorf("expected nil error, got %v", err) @@ -206,15 +147,10 @@ func TestValidateDependencyConsistency(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestSyncDependencies(t *testing.T) { - // Save original and restore after test - origGetDependencyCheckerFn := dependencycheck.GetDependencyCheckerFn - defer func() { dependencycheck.GetDependencyCheckerFn = origGetDependencyCheckerFn }() - version := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil checker returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - dependencycheck.GetDependencyCheckerFn = func() dependencycheck.DependencyChecker { return nil } err := depsync.SyncDependencies(registry, version) if err != nil { t.Errorf("expected nil error, got %v", err) @@ -230,16 +166,11 @@ func TestSyncDependencies(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestGenerateChangelogAfterBump(t *testing.T) { - // Save original and restore after test - origGetChangelogGeneratorFn := changeloggenerator.GetChangelogGeneratorFn - defer func() { changeloggenerator.GetChangelogGeneratorFn = origGetChangelogGeneratorFn }() - version := semver.SemVersion{Major: 2, Minor: 0, Patch: 0} prevVersion := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil generator returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - changeloggenerator.GetChangelogGeneratorFn = func() changeloggenerator.ChangelogGenerator { return nil } err := generateChangelogAfterBump(registry, version, prevVersion, "major") if err != nil { t.Errorf("expected nil error, got %v", err) @@ -255,16 +186,11 @@ func TestGenerateChangelogAfterBump(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestRecordAuditLogEntry(t *testing.T) { - // Save original and restore after test - origGetAuditLogFn := auditlog.GetAuditLogFn - defer func() { auditlog.GetAuditLogFn = origGetAuditLogFn }() - version := semver.SemVersion{Major: 2, Minor: 0, Patch: 0} prevVersion := semver.SemVersion{Major: 1, Minor: 0, Patch: 0} t.Run("nil audit log returns nil", func(t *testing.T) { registry := plugins.NewPluginRegistry() - auditlog.GetAuditLogFn = func() auditlog.AuditLog { return nil } err := recordAuditLogEntry(registry, version, prevVersion, "major") if err != nil { t.Errorf("expected nil error, got %v", err) @@ -322,10 +248,6 @@ func TestRunPostBumpExtensionHooks(t *testing.T) { /* ------------------------------------------------------------------------- */ func TestCreateTagAfterBump_Enabled(t *testing.T) { - // Save and restore - origGetTagManagerFn := tagmanager.GetTagManagerFn - defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }() - version := semver.SemVersion{Major: 1, Minor: 2, Patch: 3} t.Run("disabled plugin returns nil", func(t *testing.T) { @@ -333,7 +255,9 @@ func TestCreateTagAfterBump_Enabled(t *testing.T) { plugin := tagmanager.NewTagManager(&tagmanager.Config{ Enabled: false, }) - tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin } + if err := registry.RegisterTagManager(plugin); err != nil { + t.Fatalf("failed to register tag manager: %v", err) + } err := createTagAfterBump(registry, version, "patch") if err != nil { @@ -519,10 +443,6 @@ func TestSingleModuleBump_ValidateDependencyConsistencyFails(t *testing.T) { t.Fatal(err) } - // Save and restore - origGetDependencyCheckerFn := dependencycheck.GetDependencyCheckerFn - defer func() { dependencycheck.GetDependencyCheckerFn = origGetDependencyCheckerFn }() - // Create dependency checker that finds inconsistencies plugin := dependencycheck.NewDependencyChecker(&dependencycheck.Config{ Enabled: true, @@ -557,10 +477,6 @@ func TestSingleModuleBump_ValidateDependencyConsistencyWithAutoSync(t *testing.T t.Fatal(err) } - // Save and restore - origGetDependencyCheckerFn := dependencycheck.GetDependencyCheckerFn - defer func() { dependencycheck.GetDependencyCheckerFn = origGetDependencyCheckerFn }() - // Create dependency checker with auto-sync enabled - should NOT fail on inconsistencies plugin := dependencycheck.NewDependencyChecker(&dependencycheck.Config{ Enabled: true, diff --git a/internal/commands/bump/bump_pre_test.go b/internal/commands/bump/bump_pre_test.go index 34c36761..7ec9d896 100644 --- a/internal/commands/bump/bump_pre_test.go +++ b/internal/commands/bump/bump_pre_test.go @@ -2,12 +2,10 @@ package bump import ( "context" - "fmt" "path/filepath" "strings" "testing" - "github.com/indaco/sley/internal/clix" "github.com/indaco/sley/internal/config" "github.com/indaco/sley/internal/hooks" "github.com/indaco/sley/internal/plugins" @@ -85,27 +83,13 @@ func TestCLI_BumpPreCmd_EarlyFailures(t *testing.T) { override func() func() expectedErr string }{ - { - name: "FromCommand fails", - args: []string{"sley", "bump", "pre", "--label", "rc"}, - override: func() func() { - original := clix.FromCommandFn - clix.FromCommandFn = func(cmd *cli.Command) (bool, error) { - return false, fmt.Errorf("mock FromCommand error") - } - return func() { clix.FromCommandFn = original } - }, - expectedErr: "mock FromCommand error", - }, { name: "RunPreReleaseHooks fails", args: []string{"sley", "bump", "pre", "--label", "rc"}, override: func() func() { - original := hooks.RunPreReleaseHooksFn - hooks.RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { - return fmt.Errorf("mock pre-release hooks error") - } - return func() { hooks.RunPreReleaseHooksFn = original } + hooks.ResetPreReleaseHooks() + hooks.RegisterPreReleaseHook(errorHook{}) + return func() { hooks.ResetPreReleaseHooks() } }, expectedErr: "mock pre-release hooks error", }, diff --git a/internal/commands/bump/common.go b/internal/commands/bump/common.go index ed5ee64d..1134d61e 100644 --- a/internal/commands/bump/common.go +++ b/internal/commands/bump/common.go @@ -36,7 +36,7 @@ func executeSingleModuleBump( params bumpParams, ) error { // Validate command context - if _, err := clix.FromCommandFn(cmd); err != nil { + if _, err := clix.FromCommand(cmd); err != nil { return err } diff --git a/internal/commands/bump/major.go b/internal/commands/bump/major.go index fa74bdce..e4e7f2f3 100644 --- a/internal/commands/bump/major.go +++ b/internal/commands/bump/major.go @@ -30,7 +30,7 @@ func majorCmd(cfg *config.Config, registry *plugins.PluginRegistry) *cli.Command // runBumpMajor increments the major version and resets minor and patch. func runBumpMajor(ctx context.Context, cmd *cli.Command, cfg *config.Config, registry *plugins.PluginRegistry) error { - if err := hooks.RunPreReleaseHooksFn(ctx, cmd.Bool("skip-hooks")); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, cmd.Bool("skip-hooks")); err != nil { return err } diff --git a/internal/commands/bump/minor.go b/internal/commands/bump/minor.go index 5593197d..3bbe90ac 100644 --- a/internal/commands/bump/minor.go +++ b/internal/commands/bump/minor.go @@ -30,7 +30,7 @@ func minorCmd(cfg *config.Config, registry *plugins.PluginRegistry) *cli.Command // runBumpMinor increments the minor version and resets patch. func runBumpMinor(ctx context.Context, cmd *cli.Command, cfg *config.Config, registry *plugins.PluginRegistry) error { - if err := hooks.RunPreReleaseHooksFn(ctx, cmd.Bool("skip-hooks")); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, cmd.Bool("skip-hooks")); err != nil { return err } diff --git a/internal/commands/bump/patch.go b/internal/commands/bump/patch.go index a679d7f6..6b1ce8e8 100644 --- a/internal/commands/bump/patch.go +++ b/internal/commands/bump/patch.go @@ -30,7 +30,7 @@ func patchCmd(cfg *config.Config, registry *plugins.PluginRegistry) *cli.Command // runBumpPatch executes the patch bump logic. func runBumpPatch(ctx context.Context, cmd *cli.Command, cfg *config.Config, registry *plugins.PluginRegistry) error { - if err := hooks.RunPreReleaseHooksFn(ctx, cmd.Bool("skip-hooks")); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, cmd.Bool("skip-hooks")); err != nil { return err } diff --git a/internal/commands/bump/pre.go b/internal/commands/bump/pre.go index fe8ea4a2..2bd84d6b 100644 --- a/internal/commands/bump/pre.go +++ b/internal/commands/bump/pre.go @@ -53,7 +53,7 @@ func runBumpPre(ctx context.Context, cmd *cli.Command, cfg *config.Config, regis isPreserveMeta := cmd.Bool("preserve-meta") isSkipHooks := cmd.Bool("skip-hooks") - if err := hooks.RunPreReleaseHooksFn(ctx, isSkipHooks); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, isSkipHooks); err != nil { return err } diff --git a/internal/commands/bump/release.go b/internal/commands/bump/release.go index 846afbcb..29b59091 100644 --- a/internal/commands/bump/release.go +++ b/internal/commands/bump/release.go @@ -35,7 +35,7 @@ func runBumpRelease(ctx context.Context, cmd *cli.Command, cfg *config.Config, r isSkipHooks := cmd.Bool("skip-hooks") // Run pre-release hooks first (before any version operations) - if err := hooks.RunPreReleaseHooksFn(ctx, isSkipHooks); err != nil { + if err := hooks.RunPreReleaseHooks(ctx, isSkipHooks); err != nil { return err } diff --git a/internal/commands/doctor/doctorcmd.go b/internal/commands/doctor/doctorcmd.go index 29dbd667..2035c729 100644 --- a/internal/commands/doctor/doctorcmd.go +++ b/internal/commands/doctor/doctorcmd.go @@ -201,7 +201,7 @@ func printConfigValidationSummary(results []config.ValidationResult) { // runSingleModuleValidate handles the single-module validate operation. func runSingleModuleValidate(cmd *cli.Command, path string) error { - if _, err := clix.FromCommandFn(cmd); err != nil { + if _, err := clix.FromCommand(cmd); err != nil { return err } diff --git a/internal/commands/extension/list.go b/internal/commands/extension/list.go index 234d515c..1ae1e3d0 100644 --- a/internal/commands/extension/list.go +++ b/internal/commands/extension/list.go @@ -23,7 +23,7 @@ func listCmd() *cli.Command { // runExtensionList lists installed extensions. func runExtensionList() error { - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { printer.PrintError(fmt.Sprintf("failed to load configuration: %v", err)) return nil diff --git a/internal/commands/extension/list_test.go b/internal/commands/extension/list_test.go index b5da4a05..b64aea97 100644 --- a/internal/commands/extension/list_test.go +++ b/internal/commands/extension/list_test.go @@ -80,29 +80,37 @@ extensions: } func TestExtensionListCmd_LoadConfigError(t *testing.T) { - // Create a mock of the LoadConfig function that returns an error - originalLoadConfig := config.LoadConfigFn - defer func() { - // Restore the original LoadConfig function after the test - config.LoadConfigFn = originalLoadConfig - }() + // Create a directory with an invalid .sley.yaml to trigger a real load error + tmpDir := t.TempDir() - // Mock the LoadConfig function to simulate an error - config.LoadConfigFn = func() (*config.Config, error) { - return nil, fmt.Errorf("failed to load configuration") + // Write an invalid YAML file that will cause a parse error + invalidYAML := ": this is invalid yaml" + if err := os.WriteFile(filepath.Join(tmpDir, ".sley.yaml"), []byte(invalidYAML), 0644); err != nil { + t.Fatalf("failed to write invalid config: %v", err) } - // Set up a temporary directory for the config file (not used here, since LoadConfig will fail) - tmpDir := t.TempDir() + // Change to the temp directory so LoadConfig finds the invalid .sley.yaml + origDir, err := os.Getwd() + if err != nil { + t.Fatalf("failed to get working directory: %v", err) + } + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("failed to chdir: %v", err) + } + t.Cleanup(func() { + if err := os.Chdir(origDir); err != nil { + t.Fatalf("failed to restore working directory: %v", err) + } + }) // Prepare and run the CLI command - cfg := &config.Config{Path: tmpDir} + cfg := &config.Config{Path: filepath.Join(tmpDir, ".version")} appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run()}) - // Capture the output of the plugin list command again + // Capture the output of the extension list command output, err := testutils.CaptureStdout(func() { err := appCli.Run(context.Background(), []string{"sley", "extension", "list"}) - // Capture the actual error during execution + // The command prints the error and returns nil if err != nil { t.Fatalf("Expected no error, got: %v", err) } diff --git a/internal/commands/pre/precmd_test.go b/internal/commands/pre/precmd_test.go index 00ba3b19..2b4b0af4 100644 --- a/internal/commands/pre/precmd_test.go +++ b/internal/commands/pre/precmd_test.go @@ -140,7 +140,7 @@ func TestCLI_PreCommand_SaveVersionFails(t *testing.T) { } // TestCLI_PreCommand_FromCommandFails is obsolete - the pre command now uses -// GetExecutionContext instead of FromCommandFn, and auto-initializes missing files. +// GetExecutionContext instead of FromCommand, and auto-initializes missing files. // Keeping this as a placeholder for potential future execution context error testing. /* ------------------------------------------------------------------------- */ diff --git a/internal/config/config.go b/internal/config/config.go index ad1a645d..9b6e7927 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -127,14 +127,15 @@ func (s *ConfigSaver) SaveTo(cfg *Config, configFile string) error { // defaultConfigSaver is the default ConfigSaver instance for backward compatibility. var defaultConfigSaver = NewConfigSaver(nil, nil, nil) -// LoadConfigFn and SaveConfigFn are kept for backward compatibility during migration. -// They delegate to the interface-based implementations. -var ( - LoadConfigFn = loadConfig - SaveConfigFn = func(cfg *Config) error { - return defaultConfigSaver.Save(cfg) - } -) +// LoadConfig loads the configuration from environment, YAML file, or defaults. +func LoadConfig() (*Config, error) { + return loadConfig() +} + +// SaveConfig saves the configuration to the default config file. +func SaveConfig(cfg *Config) error { + return defaultConfigSaver.Save(cfg) +} func loadConfig() (*Config, error) { // Highest priority: ENV variable diff --git a/internal/config/config_extensions_test.go b/internal/config/config_extensions_test.go index 887e9038..8c701821 100644 --- a/internal/config/config_extensions_test.go +++ b/internal/config/config_extensions_test.go @@ -121,9 +121,9 @@ extensions: [] t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if (err != nil) != tt.wantErr { - t.Fatalf("LoadConfigFn() error = %v, wantErr = %v", err, tt.wantErr) + t.Fatalf("LoadConfig() error = %v, wantErr = %v", err, tt.wantErr) } if !tt.wantErr && cfg != nil { tt.check(t, cfg) @@ -195,9 +195,9 @@ func TestSaveConfig_WithExtensions(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tmp := t.TempDir() runInTempDir(t, filepath.Join(tmp, "dummy"), func() { - err := SaveConfigFn(tt.cfg) + err := SaveConfig(tt.cfg) if (err != nil) != tt.wantErr { - t.Fatalf("SaveConfigFn() error = %v, wantErr = %v", err, tt.wantErr) + t.Fatalf("SaveConfig() error = %v, wantErr = %v", err, tt.wantErr) } if !tt.wantErr { @@ -208,7 +208,7 @@ func TestSaveConfig_WithExtensions(t *testing.T) { } // Reload and verify - reloaded, err := LoadConfigFn() + reloaded, err := LoadConfig() if err != nil { t.Fatalf("failed to reload config: %v", err) } diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index 0d8eb5e3..d164b33b 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -17,7 +17,7 @@ func TestLoadConfig(t *testing.T) { os.Setenv("SLEY_PATH", "env-defined/.version") defer os.Unsetenv("SLEY_PATH") - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) checkConfigPath(t, cfg, false, "env-defined/.version") @@ -27,7 +27,7 @@ func TestLoadConfig(t *testing.T) { os.Setenv("SLEY_PATH", "../../../etc/.version") defer os.Unsetenv("SLEY_PATH") - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, true) checkConfigNil(t, cfg, true) if err != nil && err.Error() != "invalid SLEY_PATH: path traversal not allowed, use absolute path instead" { @@ -39,7 +39,7 @@ func TestLoadConfig(t *testing.T) { os.Setenv("SLEY_PATH", "/tmp/project/.version") defer os.Unsetenv("SLEY_PATH") - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) checkConfigPath(t, cfg, false, "/tmp/project/.version") @@ -49,7 +49,7 @@ func TestLoadConfig(t *testing.T) { content := "path: ./my-folder/.version\n" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) checkConfigPath(t, cfg, false, "./my-folder/.version") @@ -59,7 +59,7 @@ func TestLoadConfig(t *testing.T) { t.Run("missing file fallback", func(t *testing.T) { tmpDir := t.TempDir() runInTempDir(t, filepath.Join(tmpDir, "dummy"), func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, true) }) @@ -69,7 +69,7 @@ func TestLoadConfig(t *testing.T) { content := "{}\n" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) checkConfigPath(t, cfg, false, ".version") @@ -80,7 +80,7 @@ func TestLoadConfig(t *testing.T) { content := "not_yaml::: true" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, true) checkConfigNil(t, cfg, true) }) @@ -90,7 +90,7 @@ func TestLoadConfig(t *testing.T) { content := ": this is invalid" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, true) checkConfigNil(t, cfg, true) }) @@ -102,7 +102,7 @@ func TestLoadConfig(t *testing.T) { if err := os.Mkdir(".sley.yaml", 0755); err != nil { t.Fatal(err) } - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, true) checkConfigNil(t, cfg, true) }) @@ -118,7 +118,7 @@ func TestLoadConfigWithTheme(t *testing.T) { content := "path: .version\ntheme: dracula\n" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) if cfg.Theme != "dracula" { @@ -131,7 +131,7 @@ func TestLoadConfigWithTheme(t *testing.T) { content := "path: .version\n" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) if cfg.Theme != "" { @@ -144,7 +144,7 @@ func TestLoadConfigWithTheme(t *testing.T) { content := "path: .version\ntheme: \"\"\n" tmpPath := testutils.WriteTempConfig(t, content) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() checkError(t, err, false) checkConfigNil(t, cfg, false) if cfg.Theme != "" { diff --git a/internal/config/config_plugins_test.go b/internal/config/config_plugins_test.go index 665c5ed8..02ec82c2 100644 --- a/internal/config/config_plugins_test.go +++ b/internal/config/config_plugins_test.go @@ -289,7 +289,7 @@ plugins: t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -387,7 +387,7 @@ plugins: t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -484,7 +484,7 @@ extensions: t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error loading legacy config: %v", err) } @@ -700,7 +700,7 @@ plugins: t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -779,7 +779,7 @@ plugins: t.Run(tt.name, func(t *testing.T) { tmpPath := testutils.WriteTempConfig(t, tt.yamlInput) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/config/config_save_test.go b/internal/config/config_save_test.go index 237e3dda..0a148d2a 100644 --- a/internal/config/config_save_test.go +++ b/internal/config/config_save_test.go @@ -190,14 +190,14 @@ func TestConfigSaver_WriteError(t *testing.T) { } } -// TestSaveConfigFn_BackwardCompatibility ensures the backward-compatible SaveConfigFn still works. -func TestSaveConfigFn_BackwardCompatibility(t *testing.T) { +// TestSaveConfig_BackwardCompatibility ensures the backward-compatible SaveConfig still works. +func TestSaveConfig_BackwardCompatibility(t *testing.T) { tmp := t.TempDir() runInTempDir(t, filepath.Join(tmp, "dummy"), func() { cfg := &Config{Path: "test.version"} - err := SaveConfigFn(cfg) + err := SaveConfig(cfg) if err != nil { - t.Fatalf("SaveConfigFn() error = %v", err) + t.Fatalf("SaveConfig() error = %v", err) } if _, err := os.Stat(".sley.yaml"); err != nil { diff --git a/internal/config/config_workspace_test.go b/internal/config/config_workspace_test.go index e28bb055..ca5f0705 100644 --- a/internal/config/config_workspace_test.go +++ b/internal/config/config_workspace_test.go @@ -60,7 +60,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -81,7 +81,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -101,7 +101,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -145,7 +145,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -173,7 +173,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -216,7 +216,7 @@ func TestConfig_WorkspaceDefaults(t *testing.T) { yamlContent := `path: .version` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -239,7 +239,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -262,7 +262,7 @@ workspace: ` tmpPath := testutils.WriteTempConfig(t, yamlContent) runInTempDir(t, tmpPath, func() { - cfg, err := LoadConfigFn() + cfg, err := LoadConfig() if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/internal/config/validator_plugins.go b/internal/config/validator_plugins.go index 7bfd7f7f..de6228e8 100644 --- a/internal/config/validator_plugins.go +++ b/internal/config/validator_plugins.go @@ -17,7 +17,7 @@ func (v *Validator) validateYAMLSyntax(ctx context.Context) { return } - // If we got here, the config was successfully loaded (validated in LoadConfigFn) + // If we got here, the config was successfully loaded (validated in LoadConfig) v.addValidation("YAML Syntax", true, "Configuration file is valid YAML", false) } diff --git a/internal/extensionmgr/addextension_test.go b/internal/extensionmgr/addextension_test.go index fd067813..bae1ee3f 100644 --- a/internal/extensionmgr/addextension_test.go +++ b/internal/extensionmgr/addextension_test.go @@ -101,7 +101,7 @@ extensions: } // Ensure the config file still has only one plugin - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("expected no error loading config, got: %v", err) } diff --git a/internal/extensionmgr/integration_test.go b/internal/extensionmgr/integration_test.go index 3470dc61..e53ce414 100644 --- a/internal/extensionmgr/integration_test.go +++ b/internal/extensionmgr/integration_test.go @@ -91,7 +91,7 @@ func verifyConfigUpdated(t *testing.T, configPath string) *config.Config { t.Error("config file is empty") } - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("failed to load config: %v", err) } diff --git a/internal/extensionmgr/register_test.go b/internal/extensionmgr/register_test.go index 06990589..888ea8f0 100644 --- a/internal/extensionmgr/register_test.go +++ b/internal/extensionmgr/register_test.go @@ -201,7 +201,7 @@ func TestRegisterLocalExtension_DefaultConfigPathUsed_CurrentWorkingDir(t *testi } // Ensure the config file has the extension registered - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("expected no error loading config, got: %v", err) } @@ -250,7 +250,7 @@ func TestRegisterLocalExtension_DefaultConfigPathUsed_OtherDir(t *testing.T) { } // Ensure the config file has the extension registered - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("expected no error loading config, got: %v", err) } @@ -308,7 +308,7 @@ func TestRegisterLocalExtension_DotExtensionDir(t *testing.T) { } // Ensure the config file has the extension registered - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("expected no error loading config, got: %v", err) } @@ -567,7 +567,7 @@ func verifyConfigHasOneExtension(t *testing.T, configPath, expectedPath string) } // Load config - cfg, err := config.LoadConfigFn() + cfg, err := config.LoadConfig() if err != nil { t.Fatalf("expected no error loading config, got: %v", err) } diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 646b04a8..a213ddf9 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -100,8 +100,8 @@ var ( // defaultRunner is the default PreReleaseHookRunner for backward compatibility. var defaultRunner = NewPreReleaseHookRunner(nil, nil) -// RunPreReleaseHooksFn is kept for backward compatibility during migration. -var RunPreReleaseHooksFn = func(ctx context.Context, skip bool) error { +// RunPreReleaseHooks runs all registered pre-release hooks. +func RunPreReleaseHooks(ctx context.Context, skip bool) error { return defaultRunner.Run(ctx, skip) } @@ -125,8 +125,3 @@ func ResetPreReleaseHooks() { defer hooksMu.Unlock() preReleaseHooks = nil } - -// runPreReleaseHooks is kept for direct testing. -func runPreReleaseHooks(ctx context.Context, skip bool) error { - return defaultRunner.Run(ctx, skip) -} diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index e541cf71..a0ef0985 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -94,7 +94,7 @@ func TestRunPreReleaseHooks(t *testing.T) { } ctx := context.Background() - err := runPreReleaseHooks(ctx, tt.skip) + err := RunPreReleaseHooks(ctx, tt.skip) if (err != nil) != tt.wantErr { t.Fatalf("expected error=%v, got error=%v", tt.wantErr, err != nil) diff --git a/internal/hooks/loader.go b/internal/hooks/loader.go index 81942cad..e2989c3a 100644 --- a/internal/hooks/loader.go +++ b/internal/hooks/loader.go @@ -6,9 +6,8 @@ import ( "github.com/indaco/sley/internal/config" ) -var LoadPreReleaseHooksFromConfigFn = loadPreReleaseHooksFromConfig - -func loadPreReleaseHooksFromConfig(cfg *config.Config) error { +// LoadPreReleaseHooksFromConfig loads pre-release hooks from the configuration. +func LoadPreReleaseHooksFromConfig(cfg *config.Config) error { if cfg == nil || cfg.PreReleaseHooks == nil { return nil } diff --git a/internal/hooks/loader_test.go b/internal/hooks/loader_test.go index 4c42e084..7b734f8b 100644 --- a/internal/hooks/loader_test.go +++ b/internal/hooks/loader_test.go @@ -23,7 +23,7 @@ func TestLoadPreReleaseHooksFromConfig(t *testing.T) { }, } - err := LoadPreReleaseHooksFromConfigFn(cfg) + err := LoadPreReleaseHooksFromConfig(cfg) if err != nil { t.Fatalf("expected no error, got: %v", err) } @@ -45,7 +45,7 @@ func TestLoadPreReleaseHooksFromConfig_NilConfig(t *testing.T) { ResetPreReleaseHooks() t.Cleanup(func() { ResetPreReleaseHooks() }) - err := LoadPreReleaseHooksFromConfigFn(nil) + err := LoadPreReleaseHooksFromConfig(nil) if err != nil { t.Fatalf("expected no error, got: %v", err) } @@ -64,7 +64,7 @@ func TestLoadPreReleaseHooksFromConfig_NilPreReleaseHooks(t *testing.T) { PreReleaseHooks: nil, } - err := LoadPreReleaseHooksFromConfigFn(cfg) + err := LoadPreReleaseHooksFromConfig(cfg) if err != nil { t.Fatalf("expected no error, got: %v", err) } @@ -88,7 +88,7 @@ func TestLoadPreReleaseHooksFromConfig_SkipMissingCommand(t *testing.T) { } output, err := testutils.CaptureStdout(func() { - err := LoadPreReleaseHooksFromConfigFn(cfg) + err := LoadPreReleaseHooksFromConfig(cfg) if err != nil { t.Fatalf("expected no error, got: %v", err) } diff --git a/internal/plugins/auditlog/config.go b/internal/plugins/auditlog/config.go index 628fa75f..1f025755 100644 --- a/internal/plugins/auditlog/config.go +++ b/internal/plugins/auditlog/config.go @@ -1,5 +1,7 @@ package auditlog +import "github.com/indaco/sley/internal/config" + // Config holds configuration for the audit log plugin. type Config struct { // Enabled controls whether the plugin is active. @@ -52,3 +54,20 @@ func (c *Config) GetFormat() string { } return c.Format } + +// FromConfigStruct converts the config package struct to internal config. +func FromConfigStruct(cfg *config.AuditLogConfig) *Config { + if cfg == nil { + return DefaultConfig() + } + + return &Config{ + Enabled: cfg.Enabled, + Path: cfg.GetPath(), + Format: cfg.GetFormat(), + IncludeAuthor: cfg.IncludeAuthor, + IncludeTimestamp: cfg.IncludeTimestamp, + IncludeCommitSHA: cfg.IncludeCommitSHA, + IncludeBranch: cfg.IncludeBranch, + } +} diff --git a/internal/plugins/auditlog/registry.go b/internal/plugins/auditlog/registry.go deleted file mode 100644 index 8f3d54c7..00000000 --- a/internal/plugins/auditlog/registry.go +++ /dev/null @@ -1,57 +0,0 @@ -package auditlog - -import ( - "fmt" - "os" - - "github.com/indaco/sley/internal/config" -) - -var ( - defaultAuditLog AuditLog - RegisterAuditLogFn = registerAuditLog - GetAuditLogFn = getAuditLog -) - -func registerAuditLog(al AuditLog) { - if defaultAuditLog != nil { - fmt.Fprintf(os.Stderr, - "WARNING: Ignoring audit log %q: another audit log (%q) is already registered.\n", - al.Name(), defaultAuditLog.Name(), - ) - return - } - defaultAuditLog = al -} - -func getAuditLog() AuditLog { - return defaultAuditLog -} - -// ResetAuditLog clears the registered audit log (for testing). -func ResetAuditLog() { - defaultAuditLog = nil -} - -// Register registers the audit log plugin with the given configuration. -func Register(cfg *config.AuditLogConfig) { - internalCfg := FromConfigStruct(cfg) - RegisterAuditLogFn(NewAuditLog(internalCfg)) -} - -// FromConfigStruct converts the config package struct to internal config. -func FromConfigStruct(cfg *config.AuditLogConfig) *Config { - if cfg == nil { - return DefaultConfig() - } - - return &Config{ - Enabled: cfg.Enabled, - Path: cfg.GetPath(), - Format: cfg.GetFormat(), - IncludeAuthor: cfg.IncludeAuthor, - IncludeTimestamp: cfg.IncludeTimestamp, - IncludeCommitSHA: cfg.IncludeCommitSHA, - IncludeBranch: cfg.IncludeBranch, - } -} diff --git a/internal/plugins/auditlog/registry_test.go b/internal/plugins/auditlog/registry_test.go deleted file mode 100644 index ae7d0418..00000000 --- a/internal/plugins/auditlog/registry_test.go +++ /dev/null @@ -1,112 +0,0 @@ -package auditlog - -import ( - "testing" - - "github.com/indaco/sley/internal/config" -) - -func TestRegister(t *testing.T) { - // Reset before test - ResetAuditLog() - - cfg := &config.AuditLogConfig{ - Enabled: true, - Path: "test.json", - Format: "json", - } - - Register(cfg) - - al := GetAuditLogFn() - if al == nil { - t.Fatal("expected audit log to be registered") - } - - plugin, ok := al.(*AuditLogPlugin) - if !ok { - t.Fatal("expected AuditLogPlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } - - if plugin.GetConfig().GetPath() != "test.json" { - t.Errorf("expected path 'test.json', got %q", plugin.GetConfig().GetPath()) - } - - // Cleanup - ResetAuditLog() -} - -func TestRegisterMultiple(t *testing.T) { - // Reset before test - ResetAuditLog() - - cfg1 := &config.AuditLogConfig{ - Enabled: true, - Path: "first.json", - } - - Register(cfg1) - first := GetAuditLogFn() - - // Try to register another - cfg2 := &config.AuditLogConfig{ - Enabled: true, - Path: "second.json", - } - - Register(cfg2) - second := GetAuditLogFn() - - // Should still be the first one - if first != second { - t.Error("expected second registration to be ignored") - } - - plugin := first.(*AuditLogPlugin) - if plugin.GetConfig().GetPath() != "first.json" { - t.Error("expected first registration to remain") - } - - // Cleanup - ResetAuditLog() -} - -func TestFromConfigStruct(t *testing.T) { - tests := []struct { - name string - input *config.AuditLogConfig - wantPath string - }{ - { - name: "nil config uses defaults", - input: nil, - wantPath: ".version-history.json", - }, - { - name: "custom config", - input: &config.AuditLogConfig{ - Enabled: true, - Path: "custom.json", - Format: "yaml", - IncludeAuthor: true, - IncludeTimestamp: false, - IncludeCommitSHA: true, - IncludeBranch: false, - }, - wantPath: "custom.json", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - cfg := FromConfigStruct(tt.input) - if cfg.GetPath() != tt.wantPath { - t.Errorf("expected path %q, got %q", tt.wantPath, cfg.GetPath()) - } - }) - } -} diff --git a/internal/plugins/changeloggenerator/registry.go b/internal/plugins/changeloggenerator/registry.go deleted file mode 100644 index 335859bd..00000000 --- a/internal/plugins/changeloggenerator/registry.go +++ /dev/null @@ -1,45 +0,0 @@ -package changeloggenerator - -import ( - "fmt" - "os" - - "github.com/indaco/sley/internal/config" -) - -var ( - defaultChangelogGenerator ChangelogGenerator - RegisterChangelogGeneratorFn = registerChangelogGenerator - GetChangelogGeneratorFn = getChangelogGenerator -) - -func registerChangelogGenerator(cg ChangelogGenerator) { - if defaultChangelogGenerator != nil { - fmt.Fprintf(os.Stderr, - "WARNING: Ignoring changelog generator %q: another generator (%q) is already registered.\n", - cg.Name(), defaultChangelogGenerator.Name(), - ) - return - } - defaultChangelogGenerator = cg -} - -func getChangelogGenerator() ChangelogGenerator { - return defaultChangelogGenerator -} - -// ResetChangelogGenerator clears the registered changelog generator (for testing). -func ResetChangelogGenerator() { - defaultChangelogGenerator = nil -} - -// Register registers the changelog generator plugin with the given configuration. -func Register(cfg *config.ChangelogGeneratorConfig) error { - internalCfg := FromConfigStruct(cfg) - generator, err := NewChangelogGenerator(internalCfg) - if err != nil { - return fmt.Errorf("failed to create changelog generator: %w", err) - } - RegisterChangelogGeneratorFn(generator) - return nil -} diff --git a/internal/plugins/changeloggenerator/registry_test.go b/internal/plugins/changeloggenerator/registry_test.go deleted file mode 100644 index 9968e9f4..00000000 --- a/internal/plugins/changeloggenerator/registry_test.go +++ /dev/null @@ -1,153 +0,0 @@ -package changeloggenerator - -import ( - "testing" - - "github.com/indaco/sley/internal/config" -) - -func TestRegisterChangelogGenerator(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - defer ResetChangelogGenerator() - - // Create a plugin - cfg := DefaultConfig() - plugin, err := NewChangelogGenerator(cfg) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - // Register - registerChangelogGenerator(plugin) - - // Verify registration - got := getChangelogGenerator() - if got == nil { - t.Fatal("expected registered generator, got nil") - } - if got.Name() != plugin.Name() { - t.Errorf("Name() = %q, want %q", got.Name(), plugin.Name()) - } -} - -func TestRegisterChangelogGenerator_Duplicate(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - defer ResetChangelogGenerator() - - // Create two plugins - cfg := DefaultConfig() - plugin1, err := NewChangelogGenerator(cfg) - if err != nil { - t.Fatalf("unexpected error creating plugin1: %v", err) - } - plugin2, err := NewChangelogGenerator(cfg) - if err != nil { - t.Fatalf("unexpected error creating plugin2: %v", err) - } - - // Register first - registerChangelogGenerator(plugin1) - - // Register second (should be ignored with warning) - registerChangelogGenerator(plugin2) - - // Verify first is still registered - got := getChangelogGenerator() - if got == nil { - t.Fatal("expected registered generator") - } - // Both have same name, but first should still be there - if got != plugin1 { - t.Error("expected first registered plugin to remain") - } -} - -func TestGetChangelogGenerator_None(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - defer ResetChangelogGenerator() - - got := getChangelogGenerator() - if got != nil { - t.Errorf("expected nil, got %v", got) - } -} - -func TestResetChangelogGenerator(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - - // Register - cfg := DefaultConfig() - plugin, err := NewChangelogGenerator(cfg) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - registerChangelogGenerator(plugin) - - // Verify registered - if getChangelogGenerator() == nil { - t.Fatal("expected registered generator") - } - - // Reset - ResetChangelogGenerator() - - // Verify cleared - if getChangelogGenerator() != nil { - t.Error("expected nil after reset") - } -} - -func TestRegister(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - defer ResetChangelogGenerator() - - cfg := &config.ChangelogGeneratorConfig{ - Enabled: true, - Mode: "versioned", - } - - err := Register(cfg) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - got := getChangelogGenerator() - if got == nil { - t.Fatal("expected registered generator") - } - if !got.IsEnabled() { - t.Error("expected generator to be enabled") - } -} - -func TestRegister_NilConfig(t *testing.T) { - // Reset before test - ResetChangelogGenerator() - defer ResetChangelogGenerator() - - // Should use default config when nil - err := Register(nil) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - got := getChangelogGenerator() - if got == nil { - t.Fatal("expected registered generator even with nil config") - } -} - -func TestFunctionVariables(t *testing.T) { - // Test that function variables are properly set - if RegisterChangelogGeneratorFn == nil { - t.Error("RegisterChangelogGeneratorFn should not be nil") - } - if GetChangelogGeneratorFn == nil { - t.Error("GetChangelogGeneratorFn should not be nil") - } -} diff --git a/internal/plugins/changelogparser/plugin_test.go b/internal/plugins/changelogparser/plugin_test.go index 0d3f9ab9..1db0951e 100644 --- a/internal/plugins/changelogparser/plugin_test.go +++ b/internal/plugins/changelogparser/plugin_test.go @@ -541,73 +541,6 @@ func TestPluginInterface(t *testing.T) { var _ ChangelogInferrer = (*ChangelogParserPlugin)(nil) } -func TestRegistry(t *testing.T) { - origParser := defaultChangelogParser - defer func() { defaultChangelogParser = origParser }() - - t.Run("register and get", func(t *testing.T) { - ResetChangelogParser() - - cfg := &Config{Enabled: true} - Register(cfg) - - parser := GetChangelogParserFn() - if parser == nil { - t.Fatal("expected parser to be registered") - } - - plugin, ok := parser.(*ChangelogParserPlugin) - if !ok { - t.Fatal("expected ChangelogParserPlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } - }) - - t.Run("unregister", func(t *testing.T) { - ResetChangelogParser() - - cfg := &Config{Enabled: true} - Register(cfg) - - Unregister() - - parser := GetChangelogParserFn() - if parser != nil { - t.Error("expected parser to be nil after unregister") - } - }) - - t.Run("reset", func(t *testing.T) { - cfg := &Config{Enabled: true} - Register(cfg) - - ResetChangelogParser() - - parser := GetChangelogParserFn() - if parser != nil { - t.Error("expected parser to be nil after reset") - } - }) - - t.Run("duplicate registration warning", func(t *testing.T) { - ResetChangelogParser() - - cfg1 := &Config{Enabled: true} - Register(cfg1) - - cfg2 := &Config{Enabled: true} - Register(cfg2) - - parser := GetChangelogParserFn() - if parser == nil { - t.Fatal("expected first parser to remain registered") - } - }) -} - func TestChangelogParserPlugin_ErrorScenarios(t *testing.T) { origOpenFile := openFileFn defer func() { openFileFn = origOpenFile }() diff --git a/internal/plugins/changelogparser/registry.go b/internal/plugins/changelogparser/registry.go deleted file mode 100644 index b8900f26..00000000 --- a/internal/plugins/changelogparser/registry.go +++ /dev/null @@ -1,42 +0,0 @@ -package changelogparser - -import ( - "fmt" - "os" -) - -var ( - defaultChangelogParser ChangelogInferrer - RegisterChangelogParserFn = registerChangelogParser - GetChangelogParserFn = getChangelogParser -) - -func registerChangelogParser(p ChangelogInferrer) { - if defaultChangelogParser != nil { - fmt.Fprintf(os.Stderr, - "WARNING: Ignoring changelog parser %q: another parser (%q) is already registered.\n", - p.Name(), defaultChangelogParser.Name(), - ) - return - } - defaultChangelogParser = p -} - -func getChangelogParser() ChangelogInferrer { - return defaultChangelogParser -} - -// Register registers the changelog parser plugin with the sley plugin system. -func Register(cfg *Config) { - RegisterChangelogParserFn(NewChangelogParser(cfg)) -} - -// Unregister removes the changelog parser plugin. -func Unregister() { - defaultChangelogParser = nil -} - -// ResetChangelogParser clears the registered changelog parser (for testing). -func ResetChangelogParser() { - defaultChangelogParser = nil -} diff --git a/internal/plugins/changelogparser/registry_test.go b/internal/plugins/changelogparser/registry_test.go deleted file mode 100644 index 6bd7ea8c..00000000 --- a/internal/plugins/changelogparser/registry_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package changelogparser - -import "testing" - -func TestRegisterAndGet(t *testing.T) { - ResetChangelogParser() - defer ResetChangelogParser() - - cfg := &Config{Enabled: true, Path: "CHANGELOG.md"} - Register(cfg) - - cp := GetChangelogParserFn() - if cp == nil { - t.Fatal("expected changelog parser to be registered") - } - - plugin, ok := cp.(*ChangelogParserPlugin) - if !ok { - t.Fatal("expected ChangelogParserPlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } -} - -func TestGetReturnsNilWhenNotRegistered(t *testing.T) { - ResetChangelogParser() - defer ResetChangelogParser() - - cp := GetChangelogParserFn() - if cp != nil { - t.Error("expected nil when not registered") - } -} - -func TestReset(t *testing.T) { - ResetChangelogParser() - defer ResetChangelogParser() - - Register(&Config{Enabled: true}) - ResetChangelogParser() - - cp := GetChangelogParserFn() - if cp != nil { - t.Error("expected nil after reset") - } -} diff --git a/internal/plugins/commitparser/plugin.go b/internal/plugins/commitparser/plugin.go index 12a82942..5ea66d6a 100644 --- a/internal/plugins/commitparser/plugin.go +++ b/internal/plugins/commitparser/plugin.go @@ -94,12 +94,3 @@ func (p *CommitParserPlugin) Parse(commits []string) (string, error) { return "", errors.New("no bump type could be inferred") } } - -/* ------------------------------------------------------------------------- */ -/* REGISTRATION */ -/* ------------------------------------------------------------------------- */ - -// Register registers the commit parser plugin with the sley plugin system. -func Register() { - RegisterCommitParserFn(&CommitParserPlugin{}) -} diff --git a/internal/plugins/commitparser/plugin_test.go b/internal/plugins/commitparser/plugin_test.go index af8fd18c..713e40a5 100644 --- a/internal/plugins/commitparser/plugin_test.go +++ b/internal/plugins/commitparser/plugin_test.go @@ -77,19 +77,3 @@ func TestCommitParser_Parse(t *testing.T) { }) } } - -func TestRegisterCommitParser(t *testing.T) { - called := false - - original := RegisterCommitParserFn - RegisterCommitParserFn = func(_ CommitParser) { - called = true - } - defer func() { RegisterCommitParserFn = original }() - - Register() - - if !called { - t.Errorf("expected RegisterCommitParser to be called") - } -} diff --git a/internal/plugins/commitparser/registry.go b/internal/plugins/commitparser/registry.go deleted file mode 100644 index eb3f5729..00000000 --- a/internal/plugins/commitparser/registry.go +++ /dev/null @@ -1,31 +0,0 @@ -package commitparser - -import ( - "fmt" - "os" -) - -var ( - defaultCommitParser CommitParser - RegisterCommitParserFn = registerCommitParser - GetCommitParserFn = getCommitParser -) - -func registerCommitParser(p CommitParser) { - if defaultCommitParser != nil { - fmt.Fprintf(os.Stderr, - "WARNING: Ignoring commit parser %q: another parser (%q) is already registered.\n", - p.Name(), defaultCommitParser.Name(), - ) - return - } - defaultCommitParser = p -} - -func getCommitParser() CommitParser { - return defaultCommitParser -} - -func ResetCommitParser() { - defaultCommitParser = nil -} diff --git a/internal/plugins/commitparser/registry_test.go b/internal/plugins/commitparser/registry_test.go deleted file mode 100644 index a903fbf6..00000000 --- a/internal/plugins/commitparser/registry_test.go +++ /dev/null @@ -1,137 +0,0 @@ -package commitparser - -import ( - "bytes" - "errors" - "os" - "strings" - "testing" -) - -/* ------------------------------------------------------------------------- */ -/* MOCKS */ -/* ------------------------------------------------------------------------- */ - -type fakeCommitParser struct { - name string - parse func([]string) (string, error) -} - -func (f fakeCommitParser) Name() string { return f.name } -func (f fakeCommitParser) Description() string { return "fake commit parser" } -func (f fakeCommitParser) Version() string { return "v1.0.0" } - -func (f fakeCommitParser) Parse(commits []string) (string, error) { - return f.parse(commits) -} - -/* ------------------------------------------------------------------------- */ -/* TESTS */ -/* ------------------------------------------------------------------------- */ - -func TestRegisterCommitParser_SetsOnce(t *testing.T) { - ResetCommitParser() - defer ResetCommitParser() - - p := fakeCommitParser{name: "first"} - RegisterCommitParserFn(p) - - registered := GetCommitParserFn() - if registered == nil || registered.Name() != "first" { - t.Fatalf("expected first parser to be registered, got %v", registered) - } - - RegisterCommitParserFn(fakeCommitParser{name: "second"}) - still := GetCommitParserFn() - if still.Name() != "first" { - t.Errorf("expected original parser to remain, got %q", still.Name()) - } -} - -func TestCommitParser_ParseDelegates(t *testing.T) { - ResetCommitParser() - defer ResetCommitParser() - - expected := "minor" - RegisterCommitParserFn(fakeCommitParser{ - name: "test", - parse: func(commits []string) (string, error) { - return expected, nil - }, - }) - - parser := GetCommitParserFn() - result, err := parser.Parse([]string{"feat: add button"}) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if result != expected { - t.Errorf("expected %q, got %q", expected, result) - } -} - -func TestCommitParser_ParseReturnsError(t *testing.T) { - ResetCommitParser() - defer ResetCommitParser() - - RegisterCommitParserFn(fakeCommitParser{ - name: "failing", - parse: func(commits []string) (string, error) { - return "", errors.New("parse failed") - }, - }) - - _, err := GetCommitParserFn().Parse([]string{"invalid commit"}) - if err == nil { - t.Errorf("expected error from parser, got nil") - } -} - -func TestGetCommitParser_ReturnsNilIfUnset(t *testing.T) { - ResetCommitParser() - defer ResetCommitParser() - - if GetCommitParserFn() != nil { - t.Errorf("expected nil parser when unset") - } -} - -func TestRegisterCommitParser_Single(t *testing.T) { - defer ResetCommitParser() - - p := &fakeCommitParser{name: "primary"} - RegisterCommitParserFn(p) - - if GetCommitParserFn() == nil || GetCommitParserFn().Name() != "primary" { - t.Errorf("expected commit parser to be 'primary'") - } -} - -func TestRegisterCommitParser_SecondShowsWarning(t *testing.T) { - ResetCommitParser() - defer ResetCommitParser() - - // Capture stderr - origStderr := os.Stderr - r, w, _ := os.Pipe() - os.Stderr = w - - // Register the first parser - RegisterCommitParserFn(&fakeCommitParser{name: "first"}) - - // Register a second parser, should trigger warning - RegisterCommitParserFn(&fakeCommitParser{name: "second"}) - - // Restore stderr and read output - _ = w.Close() - os.Stderr = origStderr - - var buf bytes.Buffer - _, _ = buf.ReadFrom(r) - output := buf.String() - - expected := "Ignoring commit parser \"second\": another parser (\"first\") is already registered." - if !strings.Contains(output, expected) { - t.Errorf("expected warning %q, got: %s", expected, output) - } -} diff --git a/internal/plugins/dependencycheck/plugin_test.go b/internal/plugins/dependencycheck/plugin_test.go index 679972bd..f80a75fa 100644 --- a/internal/plugins/dependencycheck/plugin_test.go +++ b/internal/plugins/dependencycheck/plugin_test.go @@ -342,64 +342,6 @@ func TestNormalizeVersion(t *testing.T) { } } -func TestRegistry(t *testing.T) { - // Save original and restore after test - originalRegister := RegisterDependencyCheckerFn - originalGet := GetDependencyCheckerFn - defer func() { - RegisterDependencyCheckerFn = originalRegister - GetDependencyCheckerFn = originalGet - ResetDependencyChecker() - }() - - t.Run("Register and Get", func(t *testing.T) { - ResetDependencyChecker() - - cfg := &Config{Enabled: true} - Register(cfg) - - dc := GetDependencyCheckerFn() - if dc == nil { - t.Fatal("GetDependencyCheckerFn() returned nil") - } - - plugin, ok := dc.(*DependencyCheckerPlugin) - if !ok { - t.Fatal("Returned plugin is not *DependencyCheckerPlugin") - } - - if !plugin.IsEnabled() { - t.Error("Plugin should be enabled") - } - }) - - t.Run("Unregister", func(t *testing.T) { - ResetDependencyChecker() - - cfg := &Config{Enabled: true} - Register(cfg) - - Unregister() - - dc := GetDependencyCheckerFn() - if dc != nil { - t.Error("GetDependencyCheckerFn() should return nil after Unregister") - } - }) - - t.Run("ResetDependencyChecker", func(t *testing.T) { - cfg := &Config{Enabled: true} - Register(cfg) - - ResetDependencyChecker() - - dc := GetDependencyCheckerFn() - if dc != nil { - t.Error("GetDependencyCheckerFn() should return nil after ResetDependencyChecker") - } - }) -} - func TestDefaultConfig(t *testing.T) { cfg := DefaultConfig() diff --git a/internal/plugins/dependencycheck/registry.go b/internal/plugins/dependencycheck/registry.go deleted file mode 100644 index 2f4d4236..00000000 --- a/internal/plugins/dependencycheck/registry.go +++ /dev/null @@ -1,29 +0,0 @@ -package dependencycheck - -// defaultDependencyChecker is the singleton instance of DependencyChecker. -var defaultDependencyChecker DependencyChecker - -// RegisterDependencyCheckerFn allows overriding the registration function for testing. -var RegisterDependencyCheckerFn = func(dc DependencyChecker) { - defaultDependencyChecker = dc -} - -// GetDependencyCheckerFn allows overriding the getter function for testing. -var GetDependencyCheckerFn = func() DependencyChecker { - return defaultDependencyChecker -} - -// Register initializes the dependency checker plugin with the given configuration. -func Register(cfg *Config) { - RegisterDependencyCheckerFn(NewDependencyChecker(cfg)) -} - -// Unregister removes the dependency checker plugin. -func Unregister() { - defaultDependencyChecker = nil -} - -// ResetDependencyChecker clears the registered dependency checker (for testing). -func ResetDependencyChecker() { - defaultDependencyChecker = nil -} diff --git a/internal/plugins/dependencycheck/registry_test.go b/internal/plugins/dependencycheck/registry_test.go deleted file mode 100644 index 2ad0915c..00000000 --- a/internal/plugins/dependencycheck/registry_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package dependencycheck - -import "testing" - -func TestRegisterAndGet(t *testing.T) { - ResetDependencyChecker() - defer ResetDependencyChecker() - - cfg := &Config{Enabled: true, AutoSync: true} - Register(cfg) - - dc := GetDependencyCheckerFn() - if dc == nil { - t.Fatal("expected dependency checker to be registered") - } - - plugin, ok := dc.(*DependencyCheckerPlugin) - if !ok { - t.Fatal("expected DependencyCheckerPlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } -} - -func TestGetReturnsNilWhenNotRegistered(t *testing.T) { - ResetDependencyChecker() - defer ResetDependencyChecker() - - dc := GetDependencyCheckerFn() - if dc != nil { - t.Error("expected nil when not registered") - } -} - -func TestReset(t *testing.T) { - ResetDependencyChecker() - defer ResetDependencyChecker() - - Register(&Config{Enabled: true}) - ResetDependencyChecker() - - dc := GetDependencyCheckerFn() - if dc != nil { - t.Error("expected nil after reset") - } -} - -func TestUnregister(t *testing.T) { - ResetDependencyChecker() - defer ResetDependencyChecker() - - Register(&Config{Enabled: true}) - Unregister() - - dc := GetDependencyCheckerFn() - if dc != nil { - t.Error("expected nil after unregister") - } -} diff --git a/internal/plugins/loader_test.go b/internal/plugins/loader_test.go index 4887119d..2a1bd1c5 100644 --- a/internal/plugins/loader_test.go +++ b/internal/plugins/loader_test.go @@ -4,19 +4,10 @@ import ( "testing" "github.com/indaco/sley/internal/config" - "github.com/indaco/sley/internal/plugins/auditlog" - "github.com/indaco/sley/internal/plugins/changeloggenerator" - "github.com/indaco/sley/internal/plugins/changelogparser" - "github.com/indaco/sley/internal/plugins/commitparser" - "github.com/indaco/sley/internal/plugins/dependencycheck" - "github.com/indaco/sley/internal/plugins/releasegate" - "github.com/indaco/sley/internal/plugins/tagmanager" "github.com/indaco/sley/internal/plugins/versionvalidator" ) func TestRegisterConfiguredPlugins_WithCommitParser(t *testing.T) { - commitparser.ResetCommitParser() - cfg := &config.Config{ Plugins: &config.PluginConfig{ CommitParser: true, @@ -37,8 +28,6 @@ func TestRegisterConfiguredPlugins_WithCommitParser(t *testing.T) { } func TestRegisterConfiguredPlugins_DisabledCommitParser(t *testing.T) { - commitparser.ResetCommitParser() - cfg := &config.Config{ Plugins: &config.PluginConfig{ CommitParser: false, @@ -54,8 +43,6 @@ func TestRegisterConfiguredPlugins_DisabledCommitParser(t *testing.T) { } func TestRegisterConfiguredPlugins_NilConfig(t *testing.T) { - commitparser.ResetCommitParser() - registry := NewPluginRegistry() RegisterBuiltinPlugins(nil, registry) @@ -65,8 +52,6 @@ func TestRegisterConfiguredPlugins_NilConfig(t *testing.T) { } func TestRegisterConfiguredPlugins_NilPluginsField(t *testing.T) { - commitparser.ResetCommitParser() - cfg := &config.Config{ Plugins: nil, // explicit nil } @@ -80,9 +65,6 @@ func TestRegisterConfiguredPlugins_NilPluginsField(t *testing.T) { } func TestRegisterConfiguredPlugins_WithTagManager(t *testing.T) { - tagmanager.ResetTagManager() - defer tagmanager.ResetTagManager() - autoCreate := true annotate := true cfg := &config.Config{ @@ -111,9 +93,6 @@ func TestRegisterConfiguredPlugins_WithTagManager(t *testing.T) { } func TestRegisterConfiguredPlugins_TagManagerDisabled(t *testing.T) { - tagmanager.ResetTagManager() - defer tagmanager.ResetTagManager() - cfg := &config.Config{ Plugins: &config.PluginConfig{ TagManager: &config.TagManagerConfig{ @@ -131,9 +110,6 @@ func TestRegisterConfiguredPlugins_TagManagerDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_TagManagerNil(t *testing.T) { - tagmanager.ResetTagManager() - defer tagmanager.ResetTagManager() - cfg := &config.Config{ Plugins: &config.PluginConfig{ TagManager: nil, @@ -149,9 +125,6 @@ func TestRegisterConfiguredPlugins_TagManagerNil(t *testing.T) { } func TestRegisterConfiguredPlugins_WithVersionValidator(t *testing.T) { - versionvalidator.Unregister() - defer versionvalidator.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ VersionValidator: &config.VersionValidatorConfig{ @@ -178,9 +151,6 @@ func TestRegisterConfiguredPlugins_WithVersionValidator(t *testing.T) { } func TestRegisterConfiguredPlugins_VersionValidatorDisabled(t *testing.T) { - versionvalidator.Unregister() - defer versionvalidator.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ VersionValidator: &config.VersionValidatorConfig{ @@ -198,9 +168,6 @@ func TestRegisterConfiguredPlugins_VersionValidatorDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_VersionValidatorNil(t *testing.T) { - versionvalidator.Unregister() - defer versionvalidator.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ VersionValidator: nil, @@ -262,15 +229,6 @@ func TestConvertValidationRules_Empty(t *testing.T) { } func TestRegisterConfiguredPlugins_AllPlugins(t *testing.T) { - commitparser.ResetCommitParser() - tagmanager.ResetTagManager() - versionvalidator.Unregister() - defer func() { - commitparser.ResetCommitParser() - tagmanager.ResetTagManager() - versionvalidator.Unregister() - }() - autoCreate := true cfg := &config.Config{ Plugins: &config.PluginConfig{ @@ -301,9 +259,6 @@ func TestRegisterConfiguredPlugins_AllPlugins(t *testing.T) { } func TestRegisterConfiguredPlugins_WithDependencyCheck(t *testing.T) { - dependencycheck.ResetDependencyChecker() - defer dependencycheck.ResetDependencyChecker() - cfg := &config.Config{ Plugins: &config.PluginConfig{ DependencyCheck: &config.DependencyCheckConfig{ @@ -330,9 +285,6 @@ func TestRegisterConfiguredPlugins_WithDependencyCheck(t *testing.T) { } func TestRegisterConfiguredPlugins_DependencyCheckDisabled(t *testing.T) { - dependencycheck.ResetDependencyChecker() - defer dependencycheck.ResetDependencyChecker() - cfg := &config.Config{ Plugins: &config.PluginConfig{ DependencyCheck: &config.DependencyCheckConfig{ @@ -350,9 +302,6 @@ func TestRegisterConfiguredPlugins_DependencyCheckDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_DependencyCheckNil(t *testing.T) { - dependencycheck.ResetDependencyChecker() - defer dependencycheck.ResetDependencyChecker() - cfg := &config.Config{ Plugins: &config.PluginConfig{ DependencyCheck: nil, @@ -368,9 +317,6 @@ func TestRegisterConfiguredPlugins_DependencyCheckNil(t *testing.T) { } func TestRegisterConfiguredPlugins_WithChangelogParser(t *testing.T) { - changelogparser.ResetChangelogParser() - defer changelogparser.ResetChangelogParser() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogParser: &config.ChangelogParserConfig{ @@ -397,9 +343,6 @@ func TestRegisterConfiguredPlugins_WithChangelogParser(t *testing.T) { } func TestRegisterConfiguredPlugins_ChangelogParserDisabled(t *testing.T) { - changelogparser.ResetChangelogParser() - defer changelogparser.ResetChangelogParser() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogParser: &config.ChangelogParserConfig{ @@ -417,9 +360,6 @@ func TestRegisterConfiguredPlugins_ChangelogParserDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_ChangelogParserNil(t *testing.T) { - changelogparser.ResetChangelogParser() - defer changelogparser.ResetChangelogParser() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogParser: nil, @@ -435,9 +375,6 @@ func TestRegisterConfiguredPlugins_ChangelogParserNil(t *testing.T) { } func TestRegisterConfiguredPlugins_WithChangelogGenerator(t *testing.T) { - changeloggenerator.ResetChangelogGenerator() - defer changeloggenerator.ResetChangelogGenerator() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogGenerator: &config.ChangelogGeneratorConfig{ @@ -461,9 +398,6 @@ func TestRegisterConfiguredPlugins_WithChangelogGenerator(t *testing.T) { } func TestRegisterConfiguredPlugins_ChangelogGeneratorDisabled(t *testing.T) { - changeloggenerator.ResetChangelogGenerator() - defer changeloggenerator.ResetChangelogGenerator() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogGenerator: &config.ChangelogGeneratorConfig{ @@ -481,9 +415,6 @@ func TestRegisterConfiguredPlugins_ChangelogGeneratorDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_ChangelogGeneratorNil(t *testing.T) { - changeloggenerator.ResetChangelogGenerator() - defer changeloggenerator.ResetChangelogGenerator() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ChangelogGenerator: nil, @@ -499,9 +430,6 @@ func TestRegisterConfiguredPlugins_ChangelogGeneratorNil(t *testing.T) { } func TestRegisterConfiguredPlugins_WithReleaseGate(t *testing.T) { - releasegate.Unregister() - defer releasegate.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ReleaseGate: &config.ReleaseGateConfig{ @@ -526,9 +454,6 @@ func TestRegisterConfiguredPlugins_WithReleaseGate(t *testing.T) { } func TestRegisterConfiguredPlugins_ReleaseGateDisabled(t *testing.T) { - releasegate.Unregister() - defer releasegate.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ReleaseGate: &config.ReleaseGateConfig{ @@ -546,9 +471,6 @@ func TestRegisterConfiguredPlugins_ReleaseGateDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_ReleaseGateNil(t *testing.T) { - releasegate.Unregister() - defer releasegate.Unregister() - cfg := &config.Config{ Plugins: &config.PluginConfig{ ReleaseGate: nil, @@ -564,9 +486,6 @@ func TestRegisterConfiguredPlugins_ReleaseGateNil(t *testing.T) { } func TestRegisterConfiguredPlugins_WithAuditLog(t *testing.T) { - auditlog.ResetAuditLog() - defer auditlog.ResetAuditLog() - cfg := &config.Config{ Plugins: &config.PluginConfig{ AuditLog: &config.AuditLogConfig{ @@ -591,9 +510,6 @@ func TestRegisterConfiguredPlugins_WithAuditLog(t *testing.T) { } func TestRegisterConfiguredPlugins_AuditLogDisabled(t *testing.T) { - auditlog.ResetAuditLog() - defer auditlog.ResetAuditLog() - cfg := &config.Config{ Plugins: &config.PluginConfig{ AuditLog: &config.AuditLogConfig{ @@ -611,9 +527,6 @@ func TestRegisterConfiguredPlugins_AuditLogDisabled(t *testing.T) { } func TestRegisterConfiguredPlugins_AuditLogNil(t *testing.T) { - auditlog.ResetAuditLog() - defer auditlog.ResetAuditLog() - cfg := &config.Config{ Plugins: &config.PluginConfig{ AuditLog: nil, diff --git a/internal/plugins/releasegate/registry.go b/internal/plugins/releasegate/registry.go deleted file mode 100644 index 1c0141a8..00000000 --- a/internal/plugins/releasegate/registry.go +++ /dev/null @@ -1,24 +0,0 @@ -package releasegate - -// defaultReleaseGate is the singleton instance of ReleaseGate. -var defaultReleaseGate ReleaseGate - -// RegisterReleaseGateFn allows overriding the registration function for testing. -var RegisterReleaseGateFn = func(rg ReleaseGate) { - defaultReleaseGate = rg -} - -// GetReleaseGateFn allows overriding the getter function for testing. -var GetReleaseGateFn = func() ReleaseGate { - return defaultReleaseGate -} - -// Register initializes the release gate plugin with the given configuration. -func Register(cfg *Config) { - RegisterReleaseGateFn(NewReleaseGate(cfg)) -} - -// Unregister removes the release gate plugin. -func Unregister() { - defaultReleaseGate = nil -} diff --git a/internal/plugins/releasegate/registry_test.go b/internal/plugins/releasegate/registry_test.go deleted file mode 100644 index fd4859db..00000000 --- a/internal/plugins/releasegate/registry_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package releasegate - -import "testing" - -func TestRegisterAndGet(t *testing.T) { - Unregister() - defer Unregister() - - cfg := &Config{Enabled: true} - Register(cfg) - - rg := GetReleaseGateFn() - if rg == nil { - t.Fatal("expected release gate to be registered") - } - - plugin, ok := rg.(*ReleaseGatePlugin) - if !ok { - t.Fatal("expected ReleaseGatePlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } -} - -func TestGetReturnsNilWhenNotRegistered(t *testing.T) { - Unregister() - defer Unregister() - - rg := GetReleaseGateFn() - if rg != nil { - t.Error("expected nil when not registered") - } -} - -func TestRegistryUnregister(t *testing.T) { - Unregister() - defer Unregister() - - Register(&Config{Enabled: true}) - Unregister() - - rg := GetReleaseGateFn() - if rg != nil { - t.Error("expected nil after unregister") - } -} diff --git a/internal/plugins/releasegate/releasegate_test.go b/internal/plugins/releasegate/releasegate_test.go index 9fded8d1..bb375b81 100644 --- a/internal/plugins/releasegate/releasegate_test.go +++ b/internal/plugins/releasegate/releasegate_test.go @@ -579,54 +579,6 @@ func TestMatchBranchPattern(t *testing.T) { } } -func TestRegistry(t *testing.T) { - // Save original state - origGetter := GetReleaseGateFn - origRegisterer := RegisterReleaseGateFn - defer func() { - GetReleaseGateFn = origGetter - RegisterReleaseGateFn = origRegisterer - Unregister() - }() - - // Reset to defaults - GetReleaseGateFn = func() ReleaseGate { - return defaultReleaseGate - } - RegisterReleaseGateFn = func(rg ReleaseGate) { - defaultReleaseGate = rg - } - - t.Run("Register", func(t *testing.T) { - cfg := &Config{Enabled: true} - Register(cfg) - - rg := GetReleaseGateFn() - if rg == nil { - t.Error("Register() did not set the default release gate") - } - - plugin, ok := rg.(*ReleaseGatePlugin) - if !ok { - t.Error("Register() did not set a ReleaseGatePlugin instance") - } - - if !plugin.IsEnabled() { - t.Error("Register() plugin is not enabled") - } - }) - - t.Run("Unregister", func(t *testing.T) { - Register(&Config{Enabled: true}) - Unregister() - - rg := GetReleaseGateFn() - if rg != nil { - t.Error("Unregister() did not clear the default release gate") - } - }) -} - func TestNewReleaseGateWithOps_NilGitOps(t *testing.T) { // When gitOps is nil, it should default to OSGitOperations plugin := NewReleaseGateWithOps(nil, nil) diff --git a/internal/plugins/tagmanager/plugin_test.go b/internal/plugins/tagmanager/plugin_test.go index ae7618c1..0a9b68e5 100644 --- a/internal/plugins/tagmanager/plugin_test.go +++ b/internal/plugins/tagmanager/plugin_test.go @@ -449,74 +449,6 @@ func TestTagManagerPlugin_CreateTag_PushError(t *testing.T) { } } -func TestRegister(t *testing.T) { - // Reset before and after test - ResetTagManager() - defer ResetTagManager() - - cfg := &Config{Enabled: true, Prefix: "v"} - Register(cfg) - - tm := GetTagManagerFn() - if tm == nil { - t.Fatal("Register() should register the tag manager") - } - if tm.Name() != "tag-manager" { - t.Errorf("Register() tm.Name() = %q, want %q", tm.Name(), "tag-manager") - } -} - -func TestGetTagManagerFn(t *testing.T) { - // Reset before and after test - ResetTagManager() - defer ResetTagManager() - - // Initially should be nil - if tm := GetTagManagerFn(); tm != nil { - t.Error("GetTagManagerFn() should return nil when no manager registered") - } - - // After registration should return the manager - Register(&Config{Enabled: true}) - if tm := GetTagManagerFn(); tm == nil { - t.Error("GetTagManagerFn() should return manager after registration") - } -} - -func TestResetTagManager(t *testing.T) { - Register(&Config{Enabled: true}) - - if tm := GetTagManagerFn(); tm == nil { - t.Fatal("Expected tag manager to be registered") - } - - ResetTagManager() - - if tm := GetTagManagerFn(); tm != nil { - t.Error("ResetTagManager() should clear the registered manager") - } -} - -func TestRegisterTagManager_DoubleRegistration(t *testing.T) { - // Reset before and after test - ResetTagManager() - defer ResetTagManager() - - // Register first manager - first := NewTagManager(&Config{Enabled: true, Prefix: "v"}) - registerTagManager(first) - - // Attempt to register second manager (should be ignored with warning) - second := NewTagManager(&Config{Enabled: true, Prefix: "release-"}) - registerTagManager(second) - - // Should still have the first manager - tm := GetTagManagerFn() - if tm != first { - t.Error("Double registration should keep the first manager") - } -} - func TestTagManagerPlugin_NilConfig(t *testing.T) { tm := NewTagManager(nil) diff --git a/internal/plugins/tagmanager/registry.go b/internal/plugins/tagmanager/registry.go deleted file mode 100644 index b7e5742c..00000000 --- a/internal/plugins/tagmanager/registry.go +++ /dev/null @@ -1,37 +0,0 @@ -package tagmanager - -import ( - "fmt" - "os" -) - -var ( - defaultTagManager TagManager - RegisterTagManagerFn = registerTagManager - GetTagManagerFn = getTagManager -) - -func registerTagManager(tm TagManager) { - if defaultTagManager != nil { - fmt.Fprintf(os.Stderr, - "WARNING: Ignoring tag manager %q: another manager (%q) is already registered.\n", - tm.Name(), defaultTagManager.Name(), - ) - return - } - defaultTagManager = tm -} - -func getTagManager() TagManager { - return defaultTagManager -} - -// ResetTagManager clears the registered tag manager (for testing). -func ResetTagManager() { - defaultTagManager = nil -} - -// Register registers the tag manager plugin with the given configuration. -func Register(cfg *Config) { - RegisterTagManagerFn(NewTagManager(cfg)) -} diff --git a/internal/plugins/versionvalidator/plugin_test.go b/internal/plugins/versionvalidator/plugin_test.go index cb561fe7..e0f9efcf 100644 --- a/internal/plugins/versionvalidator/plugin_test.go +++ b/internal/plugins/versionvalidator/plugin_test.go @@ -617,36 +617,6 @@ func TestVersionValidatorPlugin_GetConfig(t *testing.T) { } } -func TestRegister(t *testing.T) { - Unregister() - defer Unregister() - - cfg := &Config{Enabled: true} - Register(cfg) - - vv := GetVersionValidatorFn() - if vv == nil { - t.Fatal("Register() should register the version validator") - } - if vv.Name() != "version-validator" { - t.Errorf("Register() vv.Name() = %q, want %q", vv.Name(), "version-validator") - } -} - -func TestUnregister(t *testing.T) { - Register(&Config{Enabled: true}) - - if vv := GetVersionValidatorFn(); vv == nil { - t.Fatal("Expected version validator to be registered") - } - - Unregister() - - if vv := GetVersionValidatorFn(); vv != nil { - t.Error("Unregister() should clear the registered validator") - } -} - func TestVersionValidatorPlugin_InvalidRegexPattern(t *testing.T) { cfg := &Config{ Enabled: true, diff --git a/internal/plugins/versionvalidator/registry.go b/internal/plugins/versionvalidator/registry.go deleted file mode 100644 index 51850d1d..00000000 --- a/internal/plugins/versionvalidator/registry.go +++ /dev/null @@ -1,24 +0,0 @@ -package versionvalidator - -// defaultVersionValidator is the singleton instance of VersionValidator. -var defaultVersionValidator VersionValidator - -// RegisterVersionValidatorFn allows overriding the registration function for testing. -var RegisterVersionValidatorFn = func(v VersionValidator) { - defaultVersionValidator = v -} - -// GetVersionValidatorFn allows overriding the getter function for testing. -var GetVersionValidatorFn = func() VersionValidator { - return defaultVersionValidator -} - -// Register initializes the version validator plugin with the given configuration. -func Register(cfg *Config) { - RegisterVersionValidatorFn(NewVersionValidator(cfg)) -} - -// Unregister removes the version validator plugin. -func Unregister() { - defaultVersionValidator = nil -} diff --git a/internal/plugins/versionvalidator/registry_test.go b/internal/plugins/versionvalidator/registry_test.go deleted file mode 100644 index d2697bfe..00000000 --- a/internal/plugins/versionvalidator/registry_test.go +++ /dev/null @@ -1,48 +0,0 @@ -package versionvalidator - -import "testing" - -func TestRegisterAndGet(t *testing.T) { - Unregister() - defer Unregister() - - cfg := &Config{Enabled: true} - Register(cfg) - - vv := GetVersionValidatorFn() - if vv == nil { - t.Fatal("expected version validator to be registered") - } - - plugin, ok := vv.(*VersionValidatorPlugin) - if !ok { - t.Fatal("expected VersionValidatorPlugin type") - } - - if !plugin.IsEnabled() { - t.Error("expected plugin to be enabled") - } -} - -func TestGetReturnsNilWhenNotRegistered(t *testing.T) { - Unregister() - defer Unregister() - - vv := GetVersionValidatorFn() - if vv != nil { - t.Error("expected nil when not registered") - } -} - -func TestRegistryUnregister(t *testing.T) { - Unregister() - defer Unregister() - - Register(&Config{Enabled: true}) - Unregister() - - vv := GetVersionValidatorFn() - if vv != nil { - t.Error("expected nil after unregister") - } -} From 56bdd143d6b81fb2c4cc0f4d9df4fd1cd5b4f855 Mon Sep 17 00:00:00 2001 From: indaco Date: Wed, 11 Mar 2026 10:00:24 +0100 Subject: [PATCH 2/2] refactor(git): replace mutable function variables with direct function calls --- internal/git/git.go | 15 +++---- internal/git/git_error_recovery_test.go | 58 ++++++------------------- internal/git/git_test.go | 53 ++++++++++------------ 3 files changed, 43 insertions(+), 83 deletions(-) diff --git a/internal/git/git.go b/internal/git/git.go index 2472d75b..91c8aa0f 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -10,21 +10,16 @@ import ( "github.com/indaco/sley/internal/core" ) -// Function variables to allow mocking -var ( - CloneOrUpdate = DefaultCloneOrUpdate - UpdateRepo = DefaultUpdateRepo - CloneRepoFunc = CloneRepo -) - -func DefaultCloneOrUpdate(ctx context.Context, repoURL, repoPath string) error { +// CloneOrUpdate clones a repository if it doesn't exist, or updates it if it does. +func CloneOrUpdate(ctx context.Context, repoURL, repoPath string) error { if IsValidGitRepo(repoPath) { return UpdateRepo(ctx, repoPath) } - return CloneRepoFunc(ctx, repoURL, repoPath) + return CloneRepo(ctx, repoURL, repoPath) } -func DefaultUpdateRepo(ctx context.Context, repoPath string) error { +// UpdateRepo pulls the latest changes in an existing repository. +func UpdateRepo(ctx context.Context, repoPath string) error { // Apply default timeout if context has no deadline if _, hasDeadline := ctx.Deadline(); !hasDeadline { var cancel context.CancelFunc diff --git a/internal/git/git_error_recovery_test.go b/internal/git/git_error_recovery_test.go index 96cf7443..90181713 100644 --- a/internal/git/git_error_recovery_test.go +++ b/internal/git/git_error_recovery_test.go @@ -2,7 +2,6 @@ package git import ( "context" - "errors" "testing" "time" ) @@ -18,12 +17,6 @@ func TestCloneRepo_ContextCancellation(t *testing.T) { if err == nil { t.Fatal("expected error when context is cancelled, got nil") } - - // The error should be context.Canceled or contain relevant message - if !errors.Is(err, context.Canceled) && !errors.Is(ctx.Err(), context.Canceled) { - // Some implementations wrap the error differently - t.Logf("got error: %v (context.Err: %v)", err, ctx.Err()) - } } // TestCloneRepo_ContextTimeout tests that CloneRepo respects context deadline. @@ -61,13 +54,13 @@ func TestUpdateRepo_ContextCancellation(t *testing.T) { } } -// TestDefaultCloneOrUpdate_ContextCancellation tests context cancellation for clone or update. -func TestDefaultCloneOrUpdate_ContextCancellation(t *testing.T) { +// TestCloneOrUpdate_ContextCancellation tests context cancellation for clone or update. +func TestCloneOrUpdate_ContextCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // Cancel immediately tempDir := t.TempDir() - err := DefaultCloneOrUpdate(ctx, "https://github.com/octocat/Hello-World.git", tempDir) + err := CloneOrUpdate(ctx, "https://github.com/octocat/Hello-World.git", tempDir) if err == nil { t.Fatal("expected error when context is cancelled, got nil") @@ -123,51 +116,28 @@ func TestUpdateRepo_NonExistentRepo(t *testing.T) { } } -// TestDefaultCloneOrUpdate_UpdateError tests error propagation when update fails. -func TestDefaultCloneOrUpdate_UpdateError(t *testing.T) { +// TestCloneOrUpdate_UpdateError tests error propagation when update fails. +func TestCloneOrUpdate_UpdateError(t *testing.T) { + // Create a repo with no remote configured — git pull will fail sourceRepo := setupTestRepo(t) - // Save original and restore after test - originalUpdateRepo := UpdateRepo - defer func() { UpdateRepo = originalUpdateRepo }() - - expectedErr := errors.New("simulated update failure") - UpdateRepo = func(ctx context.Context, repoPath string) error { - return expectedErr - } - ctx := context.Background() - err := DefaultCloneOrUpdate(ctx, "https://github.com/test/repo.git", sourceRepo) + err := CloneOrUpdate(ctx, "https://github.com/test/repo.git", sourceRepo) if err == nil { - t.Fatal("expected error when UpdateRepo fails, got nil") - } - - if !errors.Is(err, expectedErr) { - t.Errorf("expected error %v, got %v", expectedErr, err) + t.Fatal("expected error when git pull fails on repo with no remote, got nil") } } -// TestDefaultCloneOrUpdate_CloneError tests error propagation when clone fails. -func TestDefaultCloneOrUpdate_CloneError(t *testing.T) { - // Save original and restore after test - originalCloneRepo := CloneRepoFunc - defer func() { CloneRepoFunc = originalCloneRepo }() - - expectedErr := errors.New("simulated clone failure") - CloneRepoFunc = func(ctx context.Context, repoURL, repoPath string) error { - return expectedErr - } - +// TestCloneOrUpdate_CloneError tests error propagation when clone fails. +func TestCloneOrUpdate_CloneError(t *testing.T) { tempDir := t.TempDir() + destPath := tempDir + "/new" + ctx := context.Background() - err := DefaultCloneOrUpdate(ctx, "https://github.com/test/repo.git", tempDir+"/new") + err := CloneOrUpdate(ctx, "https://invalid.repo.url/nonexistent.git", destPath) if err == nil { - t.Fatal("expected error when CloneRepoFunc fails, got nil") - } - - if !errors.Is(err, expectedErr) { - t.Errorf("expected error %v, got %v", expectedErr, err) + t.Fatal("expected error when clone fails with invalid URL, got nil") } } diff --git a/internal/git/git_test.go b/internal/git/git_test.go index 8469b9d8..cd8463c6 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -10,47 +10,42 @@ import ( "testing" ) -func TestDefaultCloneOrUpdate(t *testing.T) { +func TestCloneOrUpdate(t *testing.T) { t.Run("ExistingRepo", func(t *testing.T) { - tempDir := setupTestRepo(t) + // Create a source repo and clone it so dest has a remote + sourceRepo := setupTestRepo(t) + destRepo := filepath.Join(t.TempDir(), "cloned") - // Mock UpdateRepo to verify it's being called - originalUpdateRepo := UpdateRepo - defer func() { UpdateRepo = originalUpdateRepo }() - - UpdateRepo = func(ctx context.Context, repoPath string) error { - if repoPath != tempDir { - t.Errorf("UpdateRepo called with wrong path: got %s, want %s", repoPath, tempDir) - } - return nil + ctx := context.Background() + if err := CloneRepo(ctx, sourceRepo, destRepo); err != nil { + t.Fatalf("setup clone failed: %v", err) } - ctx := context.Background() - err := DefaultCloneOrUpdate(ctx, "https://github.com/octocat/Hello-World.git", tempDir) + // CloneOrUpdate on existing repo should call UpdateRepo (git pull) + err := CloneOrUpdate(ctx, sourceRepo, destRepo) if err != nil { - t.Fatalf("DefaultCloneOrUpdate failed: %v", err) + t.Fatalf("CloneOrUpdate failed on existing repo: %v", err) + } + + // Verify it's still a valid git repo + if !IsValidGitRepo(destRepo) { + t.Error("expected valid git repo after update") } }) t.Run("NonExistingRepo", func(t *testing.T) { - tempDir := t.TempDir() - destRepo := filepath.Join(tempDir, "new_repo") - - // Mock CloneRepoFunc to verify it's being called - originalCloneRepo := CloneRepoFunc - defer func() { CloneRepoFunc = originalCloneRepo }() - - CloneRepoFunc = func(ctx context.Context, repoURL, repoPath string) error { - if repoPath != destRepo { - t.Errorf("CloneRepoFunc called with wrong path: got %s, want %s", repoPath, destRepo) - } - return nil - } + sourceRepo := setupTestRepo(t) + destRepo := filepath.Join(t.TempDir(), "new_repo") ctx := context.Background() - err := DefaultCloneOrUpdate(ctx, "https://github.com/octocat/Hello-World.git", destRepo) + err := CloneOrUpdate(ctx, sourceRepo, destRepo) if err != nil { - t.Fatalf("DefaultCloneOrUpdate failed: %v", err) + t.Fatalf("CloneOrUpdate failed on new repo: %v", err) + } + + // Verify it cloned successfully + if !IsValidGitRepo(destRepo) { + t.Error("expected valid git repo after clone") } }) }