Skip to content

Commit b91e1f4

Browse files
authored
refactor(plugins): remove per-package registry singletons and legacy function variables (#228)
* refactor(plugins): remove per-package registry singletons and legacy global func vars. Replace mutable var XxxFn globals with direct function calls * refactor(git): replace mutable function variables with direct function calls
1 parent 94fcca8 commit b91e1f4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+248
-1771
lines changed

cmd/sley/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func main() {
1919
}
2020

2121
func runCLI(args []string) error {
22-
cfg, err := config.LoadConfigFn()
22+
cfg, err := config.LoadConfig()
2323
if err != nil {
2424
return err
2525
}
@@ -37,7 +37,7 @@ func runCLI(args []string) error {
3737
registry := plugins.NewPluginRegistry()
3838
plugins.RegisterBuiltinPlugins(cfg, registry)
3939

40-
if err := hooks.LoadPreReleaseHooksFromConfigFn(cfg); err != nil {
40+
if err := hooks.LoadPreReleaseHooksFromConfig(cfg); err != nil {
4141
return fmt.Errorf("failed to load pre-release hooks: %w", err)
4242
}
4343

cmd/sley/main_test.go

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,10 @@ package main
22

33
import (
44
"bytes"
5-
"fmt"
65
"os"
76
"path/filepath"
87
"strings"
98
"testing"
10-
11-
"github.com/indaco/sley/internal/config"
12-
"github.com/indaco/sley/internal/hooks"
139
)
1410

1511
func TestRunMain_ShowVersion(t *testing.T) {
@@ -128,42 +124,3 @@ func TestRunMain_LoadConfigError(t *testing.T) {
128124
t.Errorf("unexpected error: %v", err)
129125
}
130126
}
131-
132-
func TestRunMain_LoadPreReleaseHooksError(t *testing.T) {
133-
tmp := t.TempDir()
134-
135-
versionPath := filepath.Join(tmp, ".version")
136-
if err := os.WriteFile(versionPath, []byte("1.2.3\n"), 0600); err != nil {
137-
t.Fatal(err)
138-
}
139-
140-
origDir, err := os.Getwd()
141-
if err != nil {
142-
t.Fatal(err)
143-
}
144-
if err := os.Chdir(tmp); err != nil {
145-
t.Fatal(err)
146-
}
147-
t.Cleanup(func() {
148-
if err := os.Chdir(origDir); err != nil {
149-
t.Fatalf("failed to restore working directory: %v", err)
150-
}
151-
})
152-
153-
// Backup and override hooks.LoadPreReleaseHooksFromConfig
154-
originalFn := hooks.LoadPreReleaseHooksFromConfigFn
155-
hooks.LoadPreReleaseHooksFromConfigFn = func(cfg *config.Config) error {
156-
return fmt.Errorf("mock pre-release hook load error")
157-
}
158-
t.Cleanup(func() {
159-
hooks.LoadPreReleaseHooksFromConfigFn = originalFn
160-
})
161-
162-
err = runCLI([]string{"sley", "bump", "patch"})
163-
if err == nil {
164-
t.Fatal("expected error from LoadPreReleaseHooksFromConfig, got nil")
165-
}
166-
if !strings.Contains(err.Error(), "failed to load pre-release hooks") {
167-
t.Errorf("unexpected error: %v", err)
168-
}
169-
}

internal/clix/clix.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,9 @@ import (
1616
"github.com/urfave/cli/v3"
1717
)
1818

19-
var FromCommandFn = fromCommand
20-
21-
// fromCommand extracts the --path and --strict flags from a cli.Command,
19+
// FromCommand extracts the --path and --strict flags from a cli.Command,
2220
// and passes them to GetOrInitVersionFile.
23-
func fromCommand(cmd *cli.Command) (bool, error) {
21+
func FromCommand(cmd *cli.Command) (bool, error) {
2422
return GetOrInitVersionFile(cmd.String("path"), cmd.Bool("strict"))
2523
}
2624

internal/clix/clix_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestFromCommand(t *testing.T) {
9090
cfg := &config.Config{Path: tmpFile}
9191
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{})
9292

93-
created, err := FromCommandFn(appCli)
93+
created, err := FromCommand(appCli)
9494
if err != nil {
9595
t.Fatalf("unexpected error: %v", err)
9696
}
@@ -106,7 +106,7 @@ func TestFromCommand(t *testing.T) {
106106
cfg := &config.Config{Path: targetPath}
107107
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{})
108108

109-
created, err := FromCommandFn(appCli)
109+
created, err := FromCommand(appCli)
110110
if err != nil {
111111
t.Fatalf("unexpected error: %v", err)
112112
}

internal/commands/bump/auto.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func runBumpAuto(ctx context.Context, cfg *config.Config, registry *plugins.Plug
9191
disableInfer := isNoInferFlag || (cfg != nil && cfg.Plugins != nil && !cfg.Plugins.CommitParser)
9292

9393
// Run pre-release hooks first (before any version operations)
94-
if err := hooks.RunPreReleaseHooksFn(ctx, isSkipHooks); err != nil {
94+
if err := hooks.RunPreReleaseHooks(ctx, isSkipHooks); err != nil {
9595
return err
9696
}
9797

@@ -152,7 +152,7 @@ func determineBumpType(registry *plugins.PluginRegistry, label string, disableIn
152152

153153
// runSingleModuleAuto handles the single-module auto bump operation.
154154
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 {
155-
if _, err := clix.FromCommandFn(cmd); err != nil {
155+
if _, err := clix.FromCommand(cmd); err != nil {
156156
return err
157157
}
158158

internal/commands/bump/bump_auto_test.go

Lines changed: 26 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"github.com/indaco/sley/internal/config"
1313
"github.com/indaco/sley/internal/plugins"
14-
"github.com/indaco/sley/internal/plugins/commitparser"
1514
"github.com/indaco/sley/internal/plugins/commitparser/gitlog"
1615
"github.com/indaco/sley/internal/plugins/tagmanager"
1716
"github.com/indaco/sley/internal/semver"
@@ -308,21 +307,20 @@ func TestTryInferBumpTypeFromCommitParserPlugin_GetCommitsError(t *testing.T) {
308307
testutils.WithMock(func() {
309308
// Mock GetCommits to fail
310309
originalGetCommits := gitlog.GetCommitsFn
311-
originalParser := commitparser.GetCommitParserFn
312310

313311
gitlog.GetCommitsFn = func(since, until string) ([]string, error) {
314312
return nil, fmt.Errorf("simulated gitlog error")
315313
}
316-
commitparser.GetCommitParserFn = func() commitparser.CommitParser {
317-
return testutils.MockCommitParser{} // Return any parser
318-
}
319314

320315
t.Cleanup(func() {
321316
gitlog.GetCommitsFn = originalGetCommits
322-
commitparser.GetCommitParserFn = originalParser
323317
})
324318
}, func() {
325319
registry := plugins.NewPluginRegistry()
320+
parser := testutils.MockCommitParser{}
321+
if err := registry.RegisterCommitParser(&parser); err != nil {
322+
t.Fatalf("failed to register parser: %v", err)
323+
}
326324
label := tryInferBumpTypeFromCommitParserPlugin(registry, "", "")
327325
if label != "" {
328326
t.Errorf("expected empty label on gitlog error, got %q", label)
@@ -337,12 +335,13 @@ func TestTryInferBumpTypeFromCommitParserPlugin_ParserError(t *testing.T) {
337335
gitlog.GetCommitsFn = func(since, until string) ([]string, error) {
338336
return []string{"fix: something"}, nil
339337
}
340-
commitparser.GetCommitParserFn = func() commitparser.CommitParser {
341-
return testutils.MockCommitParser{Err: fmt.Errorf("parser error")}
342-
}
343338
},
344339
func() {
345340
registry := plugins.NewPluginRegistry()
341+
parser := testutils.MockCommitParser{Err: fmt.Errorf("parser error")}
342+
if err := registry.RegisterCommitParser(&parser); err != nil {
343+
t.Fatalf("failed to register parser: %v", err)
344+
}
346345
label := tryInferBumpTypeFromCommitParserPlugin(registry, "", "")
347346
if label != "" {
348347
t.Errorf("expected empty label on parser error, got %q", label)
@@ -695,56 +694,36 @@ func TestGetNextVersion(t *testing.T) {
695694
/* ------------------------------------------------------------------------- */
696695

697696
func TestBumpAuto_CallsCreateTagAfterBump_WithEnabledTagManager(t *testing.T) {
698-
// This test verifies that createTagAfterBump is called when tag manager is enabled
699-
// by testing the function directly with the "auto" bump type parameter
700697
version := semver.SemVersion{Major: 99, Minor: 88, Patch: 77}
701698

702-
// Save original function and restore after test
703-
origGetTagManagerFn := tagmanager.GetTagManagerFn
704-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
705-
706699
// Create an enabled tag manager plugin
707700
plugin := tagmanager.NewTagManager(&tagmanager.Config{
708701
Enabled: true,
709702
AutoCreate: true,
710703
Prefix: "v",
711704
Annotate: true,
712705
})
713-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
714706

715707
registry := plugins.NewPluginRegistry()
708+
if err := registry.RegisterTagManager(plugin); err != nil {
709+
t.Fatalf("failed to register tag manager: %v", err)
710+
}
716711

717-
// Call createTagAfterBump directly with "auto" bump type
718-
// This is the call that runSingleModuleAuto makes at the end
719712
err := createTagAfterBump(registry, version, "auto")
720713

721-
// The test environment may have various outcomes depending on git state
722-
// The important thing is that the function is called and attempts tag creation
723714
if err != nil {
724715
errStr := err.Error()
725-
// These are acceptable errors when running in a test environment
726-
if !strings.Contains(errStr, "failed to create tag") && !strings.Contains(errStr, "already exists") {
727-
t.Fatalf("expected tag creation error, tag exists error, or no error, got: %v", err)
716+
if !strings.Contains(errStr, "failed to create tag") && !strings.Contains(errStr, "already exists") && !strings.Contains(errStr, "failed to commit") {
717+
t.Fatalf("expected tag creation error, tag exists error, commit error, or no error, got: %v", err)
728718
}
729719
}
730720
}
731721

732722
func TestBumpAuto_EndToEnd_WithMockTagManager(t *testing.T) {
733-
// This test verifies the full bump auto flow with a mock tag manager
734-
// that tracks whether CreateTag is called
735723
tmpDir := t.TempDir()
736724
versionPath := filepath.Join(tmpDir, ".version")
737725
testutils.WriteTempVersionFile(t, tmpDir, "1.2.3")
738726

739-
// Save original function and restore after test
740-
origGetTagManagerFn := tagmanager.GetTagManagerFn
741-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
742-
743-
// Create a mock that tracks calls - note: createTagAfterBump uses type assertion
744-
// to *tagmanager.TagManagerPlugin, so mocks will be treated as disabled
745-
// We use this to verify the flow works when tag manager returns nil for registry
746-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil }
747-
748727
cfg := &config.Config{Path: versionPath}
749728
registry := plugins.NewPluginRegistry()
750729
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)})
@@ -757,7 +736,6 @@ func TestBumpAuto_EndToEnd_WithMockTagManager(t *testing.T) {
757736
t.Fatalf("expected no error, got: %v", err)
758737
}
759738

760-
// Verify the version was bumped
761739
got := testutils.ReadTempVersionFile(t, tmpDir)
762740
want := "1.2.4"
763741
if got != want {
@@ -770,16 +748,10 @@ func TestBumpAuto_SkipsTagCreation_WhenTagManagerDisabled(t *testing.T) {
770748
versionPath := filepath.Join(tmpDir, ".version")
771749
testutils.WriteTempVersionFile(t, tmpDir, "1.2.3")
772750

773-
// Save original function and restore after test
774-
origGetTagManagerFn := tagmanager.GetTagManagerFn
775-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
776-
777-
// Create a disabled tag manager plugin
778751
plugin := tagmanager.NewTagManager(&tagmanager.Config{
779752
Enabled: false,
780753
AutoCreate: false,
781754
})
782-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
783755

784756
cfg := &config.Config{Path: versionPath}
785757
registry := plugins.NewPluginRegistry()
@@ -808,13 +780,6 @@ func TestBumpAuto_SkipsTagCreation_WhenNoTagManagerRegistered(t *testing.T) {
808780
versionPath := filepath.Join(tmpDir, ".version")
809781
testutils.WriteTempVersionFile(t, tmpDir, "1.2.3-alpha.1")
810782

811-
// Save original function and restore after test
812-
origGetTagManagerFn := tagmanager.GetTagManagerFn
813-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
814-
815-
// No tag manager registered
816-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil }
817-
818783
cfg := &config.Config{Path: versionPath}
819784
registry := plugins.NewPluginRegistry()
820785
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)})
@@ -838,35 +803,24 @@ func TestBumpAuto_TagCreatedWithCorrectParameters(t *testing.T) {
838803
version := semver.SemVersion{Major: 1, Minor: 2, Patch: 4}
839804

840805
t.Run("calls createTagAfterBump with auto bump type", func(t *testing.T) {
841-
// Save original and restore after test
842-
origGetTagManagerFn := tagmanager.GetTagManagerFn
843-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
844-
845-
// Create enabled tag manager
846806
plugin := tagmanager.NewTagManager(&tagmanager.Config{
847807
Enabled: true,
848808
AutoCreate: true,
849809
Prefix: "v",
850810
})
851-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
852811

853812
registry := plugins.NewPluginRegistry()
854-
// Note: createTagAfterBump checks for *TagManagerPlugin type assertion
855-
// When using a real plugin, it will try to create a tag (which fails without git)
813+
if err := registry.RegisterTagManager(plugin); err != nil {
814+
t.Fatalf("failed to register tag manager: %v", err)
815+
}
856816
err := createTagAfterBump(registry, version, "auto")
857817

858-
// In test environment without git, we expect a tag creation error
859-
if err != nil && !strings.Contains(err.Error(), "failed to create tag") {
818+
if err != nil && !strings.Contains(err.Error(), "failed to create tag") && !strings.Contains(err.Error(), "failed to commit") {
860819
t.Errorf("unexpected error type: %v", err)
861820
}
862821
})
863822

864823
t.Run("returns nil when tag manager is nil", func(t *testing.T) {
865-
origGetTagManagerFn := tagmanager.GetTagManagerFn
866-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
867-
868-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return nil }
869-
870824
registry := plugins.NewPluginRegistry()
871825
err := createTagAfterBump(registry, version, "auto")
872826
if err != nil {
@@ -875,16 +829,15 @@ func TestBumpAuto_TagCreatedWithCorrectParameters(t *testing.T) {
875829
})
876830

877831
t.Run("returns nil when tag manager is disabled", func(t *testing.T) {
878-
origGetTagManagerFn := tagmanager.GetTagManagerFn
879-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
880-
881832
plugin := tagmanager.NewTagManager(&tagmanager.Config{
882833
Enabled: false,
883834
AutoCreate: false,
884835
})
885-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
886836

887837
registry := plugins.NewPluginRegistry()
838+
if err := registry.RegisterTagManager(plugin); err != nil {
839+
t.Fatalf("failed to register tag manager: %v", err)
840+
}
888841
err := createTagAfterBump(registry, version, "auto")
889842
if err != nil {
890843
t.Errorf("expected nil error when tag manager is disabled, got: %v", err)
@@ -897,19 +850,16 @@ func TestBumpAuto_TagCreation_OnPreReleasePromotion(t *testing.T) {
897850
versionPath := filepath.Join(tmpDir, ".version")
898851
testutils.WriteTempVersionFile(t, tmpDir, "2.0.0-rc.1")
899852

900-
// Save original function and restore after test
901-
origGetTagManagerFn := tagmanager.GetTagManagerFn
902-
defer func() { tagmanager.GetTagManagerFn = origGetTagManagerFn }()
903-
904-
// Create a disabled tag manager to verify bump succeeds without tag creation
905853
plugin := tagmanager.NewTagManager(&tagmanager.Config{
906854
Enabled: false,
907855
AutoCreate: false,
908856
})
909-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
910857

911858
cfg := &config.Config{Path: versionPath}
912859
registry := plugins.NewPluginRegistry()
860+
if err := registry.RegisterTagManager(plugin); err != nil {
861+
t.Fatalf("failed to register tag manager: %v", err)
862+
}
913863
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)})
914864

915865
err := appCli.Run(context.Background(), []string{
@@ -932,28 +882,25 @@ func TestBumpAuto_InferredMinorBump_WithTagManager(t *testing.T) {
932882
versionPath := filepath.Join(tmpDir, ".version")
933883
testutils.WriteTempVersionFile(t, tmpDir, "1.0.0")
934884

935-
// Save and restore original functions
936-
origGetTagManagerFn := tagmanager.GetTagManagerFn
937885
originalInfer := tryInferBumpTypeFromCommitParserPluginFn
938886
defer func() {
939-
tagmanager.GetTagManagerFn = origGetTagManagerFn
940887
tryInferBumpTypeFromCommitParserPluginFn = originalInfer
941888
}()
942889

943-
// Mock inference to return "minor"
944890
tryInferBumpTypeFromCommitParserPluginFn = func(registry *plugins.PluginRegistry, since, until string) string {
945891
return "minor"
946892
}
947893

948-
// Create a disabled tag manager (to avoid git dependency in tests)
949894
plugin := tagmanager.NewTagManager(&tagmanager.Config{
950895
Enabled: false,
951896
AutoCreate: false,
952897
})
953-
tagmanager.GetTagManagerFn = func() tagmanager.TagManager { return plugin }
954898

955899
cfg := &config.Config{Path: versionPath, Plugins: &config.PluginConfig{CommitParser: true}}
956900
registry := plugins.NewPluginRegistry()
901+
if err := registry.RegisterTagManager(plugin); err != nil {
902+
t.Fatalf("failed to register tag manager: %v", err)
903+
}
957904
appCli := testutils.BuildCLIForTests(cfg.Path, []*cli.Command{Run(cfg, registry)})
958905

959906
err := appCli.Run(context.Background(), []string{

0 commit comments

Comments
 (0)