fix: Refactor config loading and add tests

Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>

internal/config/load.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -22,13 +23,13 @@ const (
 var hookKeyRegexp = regexp.MustCompile(`^(?P<hookName>[^.]+)\.(scripts|commands)`)
 
 // Loads configs from the given directory with extensions.
-func Load(fs afero.Fs, path string) (*Config, error) {
-	global, err := read(fs, path, "lefthook")
+func Load(fs afero.Fs, repo *git.Repository) (*Config, error) {
+	global, err := read(fs, repo.RootPath, "lefthook")
 	if err != nil {
 		return nil, err
 	}
 
-	extends, err := mergeAll(fs, path)
+	extends, err := mergeAll(fs, repo)
 	if err != nil {
 		return nil, err
 	}
@@ -66,22 +67,21 @@ func read(fs afero.Fs, path string, name string) (*viper.Viper, error) {
 }
 
 // mergeAll merges remotes and extends from .lefthook and .lefthook-local.
-func mergeAll(fs afero.Fs, path string) (*viper.Viper, error) {
-	extends, err := read(fs, path, "lefthook")
+func mergeAll(fs afero.Fs, repo *git.Repository) (*viper.Viper, error) {
+	extends, err := read(fs, repo.RootPath, "lefthook")
 	if err != nil {
 		return nil, err
 	}
 
-	if err := mergeRemote(fs, extends); err != nil {
+	if err := mergeRemote(fs, repo, extends); err != nil {
 		return nil, err
 	}
 
 	if err := extend(fs, extends); err != nil {
 		return nil, err
 	}
 
-	extends.SetConfigName("lefthook-local")
-	if err := extends.MergeInConfig(); err != nil {
+	if err := merge("lefthook-local", "", extends); err != nil {
 		if _, notFoundErr := err.(viper.ConfigFileNotFoundError); !notFoundErr {
 			return nil, err
 		}
@@ -95,7 +95,7 @@ func mergeAll(fs afero.Fs, path string) (*viper.Viper, error) {
 }
 
 // mergeRemote merges remote config to the current one.
-func mergeRemote(fs afero.Fs, v *viper.Viper) error {
+func mergeRemote(fs afero.Fs, repo *git.Repository, v *viper.Viper) error {
 	var remote Remote
 	err := v.UnmarshalKey("remote", &remote)
 	if err != nil {
@@ -106,11 +106,7 @@ func mergeRemote(fs afero.Fs, v *viper.Viper) error {
 		return nil
 	}
 
-	remotePath, err := git.RemoteFolder(remote.GitURL)
-	if err != nil {
-		return err
-	}
-
+	remotePath := repo.RemoteFolder(remote.GitURL)
 	configFile := DefaultConfigName
 	if len(remote.Config) > 0 {
 		configFile = remote.Config
@@ -124,36 +120,33 @@ func mergeRemote(fs afero.Fs, v *viper.Viper) error {
 		return nil
 	}
 
-	// TODO: Rewrite using common merge()
-	v.SetConfigName("remote")
-	v.SetConfigFile(configPath)
-	if err := v.MergeInConfig(); err != nil {
+	if err := merge("remote", configPath, v); err != nil {
 		return err
 	}
 
 	return nil
 }
 
+// extend merges all files listed in 'extends' option into the config.
 func extend(fs afero.Fs, v *viper.Viper) error {
-	for _, path := range v.GetStringSlice("extends") {
-		if err := merge(fs, path, v); err != nil {
+	for i, path := range v.GetStringSlice("extends") {
+		if err := merge(fmt.Sprintf("extend_%d", i), path, v); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-// FIXME: Use MergeInConfig because MergeConfigMap destroys scripts names.
-func merge(fs afero.Fs, path string, v *viper.Viper) error {
-	name := strings.TrimSuffix(filepath.Base(path), filepath.Ext(path))
-
-	another, err := read(fs, filepath.Dir(path), name)
-	if err != nil {
-		return err
+// merge merges the configuration using viper builtin MergeInConfig.
+func merge(name, path string, v *viper.Viper) error {
+	v.SetConfigName(name)
+	if len(path) > 0 {
+		v.SetConfigFile(path)
 	}
-	if err = v.MergeConfigMap(another.AllSettings()); err != nil {
+	if err := v.MergeInConfig(); err != nil {
 		return err
 	}
+
 	return nil
 }
 
@@ -167,7 +160,7 @@ func unmarshalConfigs(base, extra *viper.Viper, c *Config) error {
 	}
 
 	// For extra non-git hooks.
-	// This behavior will be deprecated in next versions.
+	// This behavior may be deprecated in next versions.
 	for _, maybeHook := range base.AllKeys() {
 		if !hookKeyRegexp.MatchString(maybeHook) {
 			continue

internal/config/load_test.go

@@ -8,6 +8,8 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/spf13/afero"
+
+	"github.com/evilmartians/lefthook/internal/git"
 )
 
 func TestLoad(t *testing.T) {
@@ -17,10 +19,12 @@ func TestLoad(t *testing.T) {
 	}
 
 	for i, tt := range [...]struct {
-		name   string
-		global []byte
-		local  []byte
-		result *Config
+		name             string
+		global           []byte
+		local            []byte
+		remote           []byte
+		remoteConfigPath string
+		result           *Config
 	}{
 		{
 			name: "simple",
@@ -157,7 +161,7 @@ lints:
 			result: &Config{
 				SourceDir:      DefaultSourceDir,
 				SourceDirLocal: DefaultSourceDirLocal,
-				Colors:         true, // defaults to true
+				Colors:         DefaultColorsEnabled,
 				Hooks: map[string]*Hook{
 					"tests": {
 						Parallel: false,
@@ -177,8 +181,108 @@ lints:
 				},
 			},
 		},
+		{
+			name: "with remote",
+			global: []byte(`
+remote:
+  git_url: git@github.com:evilmartians/lefthook
+`),
+			remote: []byte(`
+pre-commit:
+  commands:
+    lint:
+      run: yarn lint
+  scripts:
+    "test.sh":
+      runner: bash
+`),
+			remoteConfigPath: filepath.Join(root, ".git", "info", "remotes", "lefthook", "lefthook.yml"),
+			result: &Config{
+				SourceDir:      DefaultSourceDir,
+				SourceDirLocal: DefaultSourceDirLocal,
+				Colors:         DefaultColorsEnabled,
+				Remote: Remote{
+					GitURL: "git@github.com:evilmartians/lefthook",
+				},
+				Hooks: map[string]*Hook{
+					"pre-commit": {
+						Commands: map[string]*Command{
+							"lint": {
+								Run: "yarn lint",
+							},
+						},
+						Scripts: map[string]*Script{
+							"test.sh": {
+								Runner: "bash",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "with remote and custom config name",
+			global: []byte(`
+remote:
+  git_url: git@github.com:evilmartians/lefthook
+  ref: v1.0.0
+  config: examples/custom.yml
+
+pre-commit:
+  commands:
+    global:
+      run: echo 'Global!'
+    lint:
+      run: this will be overwritten
+`),
+			remote: []byte(`
+pre-commit:
+  commands:
+    lint:
+      run: yarn lint
+      skip: true
+  scripts:
+    "test.sh":
+      runner: bash
+`),
+			remoteConfigPath: filepath.Join(root, ".git", "info", "remotes", "lefthook", "examples", "custom.yml"),
+			result: &Config{
+				SourceDir:      DefaultSourceDir,
+				SourceDirLocal: DefaultSourceDirLocal,
+				Colors:         DefaultColorsEnabled,
+				Remote: Remote{
+					GitURL: "git@github.com:evilmartians/lefthook",
+					Ref:    "v1.0.0",
+					Config: "examples/custom.yml",
+				},
+				Hooks: map[string]*Hook{
+					"pre-commit": {
+						Commands: map[string]*Command{
+							"lint": {
+								Run:  "yarn lint",
+								Skip: true,
+							},
+							"global": {
+								Run: "echo 'Global!'",
+							},
+						},
+						Scripts: map[string]*Script{
+							"test.sh": {
+								Runner: "bash",
+							},
+						},
+					},
+				},
+			},
+		},
 	} {
 		fs := afero.Afero{Fs: afero.NewMemMapFs()}
+		repo := &git.Repository{
+			Fs:       fs,
+			RootPath: root,
+			InfoPath: filepath.Join(root, ".git", "info"),
+		}
+
 		t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) {
 			if err := fs.WriteFile(filepath.Join(root, "lefthook.yml"), tt.global, 0o644); err != nil {
 				t.Errorf("unexpected error: %s", err)
@@ -188,7 +292,17 @@ lints:
 				t.Errorf("unexpected error: %s", err)
 			}
 
-			checkConfig, err := Load(fs.Fs, root)
+			if len(tt.remoteConfigPath) > 0 {
+				if err := fs.MkdirAll(filepath.Base(tt.remoteConfigPath), 0o755); err != nil {
+					t.Errorf("unexpected error: %s", err)
+				}
+
+				if err := fs.WriteFile(tt.remoteConfigPath, tt.remote, 0o644); err != nil {
+					t.Errorf("unexpected error: %s", err)
+				}
+			}
+
+			checkConfig, err := Load(fs.Fs, repo)
 
 			if err != nil {
 				t.Errorf("should parse configs without errors: %s", err)

internal/git/remote.go

@@ -14,22 +14,6 @@ const (
 	remotesFolderMode = 0o755
 )
 
-func RemoteFolder(url string) (string, error) {
-	infoPath, err := execGit(cmdInfoPath)
-	if err != nil {
-		return "", err
-	}
-
-	remotesPath := filepath.Join(infoPath, remotesFolder)
-
-	return filepath.Join(
-		remotesPath,
-		filepath.Base(
-			strings.TrimSuffix(url, filepath.Ext(url)),
-		),
-	), nil
-}
-
 func (r *Repository) RemoteFolder(url string) string {
 	remotesPath := filepath.Join(r.InfoPath, remotesFolder)
 

internal/lefthook/add.go

@@ -61,7 +61,7 @@ func (l *Lefthook) getSourceDirs() (global, local string) {
 	global = config.DefaultSourceDir
 	local = config.DefaultSourceDirLocal
 
-	cfg, err := config.Load(l.Fs, l.repo.RootPath)
+	cfg, err := config.Load(l.Fs, l.repo)
 	if err == nil {
 		if len(cfg.SourceDir) > 0 {
 			global = cfg.SourceDir

internal/lefthook/install.go

@@ -74,7 +74,7 @@ func (l *Lefthook) readOrCreateConfig() (*config.Config, error) {
 		}
 	}
 
-	return config.Load(l.Fs, l.repo.RootPath)
+	return config.Load(l.Fs, l.repo)
 }
 
 func (l *Lefthook) configExists(path string) bool {

internal/lefthook/run.go

@@ -43,7 +43,7 @@ func (l *Lefthook) Run(hookName string, gitArgs []string) error {
 	}
 
 	// Load config
-	cfg, err := config.Load(l.Fs, l.repo.RootPath)
+	cfg, err := config.Load(l.Fs, l.repo)
 	if err != nil {
 		return err
 	}

internal/lefthook/runner/runner_test.go

@@ -189,7 +189,7 @@ func TestRunAll(t *testing.T) {
 		},
 		{
 			name:       "with simple scripts",
-			sourceDirs: []string{config.DefaultSourceDir},
+			sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)},
 			existingFiles: []string{
 				filepath.Join(root, config.DefaultSourceDir, hookName, "script.sh"),
 				filepath.Join(root, config.DefaultSourceDir, hookName, "failing.js"),