From d5b7e77f15fd9b835aa62a6fef066dd902324d16 Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Fri, 18 Oct 2024 15:24:06 +0300 Subject: [PATCH] feat: add skip option merge-commit (#850) * feat: add skip merge commit option * refactor: tests * docs: add skip option merge-commit * docs: give a list of possible skip options --- .golangci.yml | 1 - docs/configuration.md | 19 ++ internal/config/command.go | 4 +- internal/config/hook.go | 4 +- internal/config/script.go | 4 +- internal/config/skip_checker.go | 27 +-- internal/config/skip_checker_test.go | 46 +++-- internal/git/state.go | 61 ++++-- internal/lefthook/run_test.go | 40 +--- internal/lefthook/runner/prepare_command.go | 2 +- internal/lefthook/runner/prepare_script.go | 2 +- internal/lefthook/runner/runner.go | 2 +- internal/lefthook/runner/runner_test.go | 206 +++++++------------- testdata/skip_merge_commit.txt | 38 ++++ 14 files changed, 237 insertions(+), 219 deletions(-) create mode 100644 testdata/skip_merge_commit.txt diff --git a/.golangci.yml b/.golangci.yml index 3501e359..d215e910 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -44,7 +44,6 @@ linters: - errname - errorlint - exhaustive - - exportloopref - forbidigo - gci - gochecknoinits diff --git a/docs/configuration.md b/docs/configuration.md index a3fceca8..eed8f41c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -799,6 +799,13 @@ pre-commit: You can skip all or specific commands and scripts using `skip` option. You can also skip when merging, rebasing, or being on a specific branch. Globs are available for branches. +Possible skip values: +- `rebase` - when in rebase git state +- `merge` - when in merge git state +- `merge-commit` - when current HEAD commit is the merge commit +- `ref: main` - when on a `main` branch +- `run: test ${SKIP_ME} -eq 1` - when `test ${SKIP_ME} -eq 1` is successful (return code is 0) + **Example** Always skipping a command: @@ -839,6 +846,18 @@ pre-commit: run: yarn lint ``` +Skipping when your are on a merge commit: + +```yml +# lefthook.yml + +pre-push: + commands: + lint: + skip: merge-commit + run: yarn lint +``` + Skipping the whole hook on `main` branch: ```yml diff --git a/internal/config/command.go b/internal/config/command.go index ad2db62e..8a2ca4da 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -46,9 +46,9 @@ func (c Command) Validate() error { return nil } -func (c Command) DoSkip(gitState git.State) bool { +func (c Command) DoSkip(state func() git.State) bool { skipChecker := NewSkipChecker(system.Cmd) - return skipChecker.check(gitState, c.Skip, c.Only) + return skipChecker.check(state, c.Skip, c.Only) } func (c Command) ExecutionPriority() int { diff --git a/internal/config/hook.go b/internal/config/hook.go index f22290bc..4e970b22 100644 --- a/internal/config/hook.go +++ b/internal/config/hook.go @@ -43,9 +43,9 @@ func (h *Hook) Validate() error { return nil } -func (h *Hook) DoSkip(gitState git.State) bool { +func (h *Hook) DoSkip(state func() git.State) bool { skipChecker := NewSkipChecker(system.Cmd) - return skipChecker.check(gitState, h.Skip, h.Only) + return skipChecker.check(state, h.Skip, h.Only) } func unmarshalHooks(base, extra *viper.Viper) (*Hook, error) { diff --git a/internal/config/script.go b/internal/config/script.go index bd3e1f8d..a8c74635 100644 --- a/internal/config/script.go +++ b/internal/config/script.go @@ -29,9 +29,9 @@ type scriptRunnerReplace struct { Runner string `mapstructure:"runner"` } -func (s Script) DoSkip(gitState git.State) bool { +func (s Script) DoSkip(state func() git.State) bool { skipChecker := NewSkipChecker(system.Cmd) - return skipChecker.check(gitState, s.Skip, s.Only) + return skipChecker.check(state, s.Skip, s.Only) } func (s Script) ExecutionPriority() int { diff --git a/internal/config/skip_checker.go b/internal/config/skip_checker.go index 7b0d0907..5febc061 100644 --- a/internal/config/skip_checker.go +++ b/internal/config/skip_checker.go @@ -17,37 +17,41 @@ func NewSkipChecker(cmd system.Command) *skipChecker { } // check returns the result of applying a skip/only setting which can be a branch, git state, shell command, etc. -func (sc *skipChecker) check(gitState git.State, skip interface{}, only interface{}) bool { +func (sc *skipChecker) check(state func() git.State, skip interface{}, only interface{}) bool { + if skip == nil && only == nil { + return false + } + if skip != nil { - if sc.matches(gitState, skip) { + if sc.matches(state, skip) { return true } } if only != nil { - return !sc.matches(gitState, only) + return !sc.matches(state, only) } return false } -func (sc *skipChecker) matches(gitState git.State, value interface{}) bool { +func (sc *skipChecker) matches(state func() git.State, value interface{}) bool { switch typedValue := value.(type) { case bool: return typedValue case string: - return typedValue == gitState.Step + return typedValue == state().State case []interface{}: - return sc.matchesSlices(gitState, typedValue) + return sc.matchesSlices(state, typedValue) } return false } -func (sc *skipChecker) matchesSlices(gitState git.State, slice []interface{}) bool { +func (sc *skipChecker) matchesSlices(gitState func() git.State, slice []interface{}) bool { for _, state := range slice { switch typedState := state.(type) { case string: - if typedState == gitState.Step { + if typedState == gitState().State { return true } case map[string]interface{}: @@ -64,19 +68,20 @@ func (sc *skipChecker) matchesSlices(gitState git.State, slice []interface{}) bo return false } -func (sc *skipChecker) matchesRef(gitState git.State, typedState map[string]interface{}) bool { +func (sc *skipChecker) matchesRef(state func() git.State, typedState map[string]interface{}) bool { ref, ok := typedState["ref"].(string) if !ok { return false } - if ref == gitState.Branch { + branch := state().Branch + if ref == branch { return true } g := glob.MustCompile(ref) - return g.Match(gitState.Branch) + return g.Match(branch) } func (sc *skipChecker) matchesCommands(typedState map[string]interface{}) bool { diff --git a/internal/config/skip_checker_test.go b/internal/config/skip_checker_test.go index e5c263e8..a83e13f9 100644 --- a/internal/config/skip_checker_test.go +++ b/internal/config/skip_checker_test.go @@ -23,123 +23,129 @@ func TestDoSkip(t *testing.T) { for _, tt := range [...]struct { name string - state git.State + state func() git.State skip, only interface{} skipped bool }{ { name: "when true", - state: git.State{}, + state: func() git.State { return git.State{} }, skip: true, skipped: true, }, { name: "when false", - state: git.State{}, + state: func() git.State { return git.State{} }, skip: false, skipped: false, }, { name: "when merge", - state: git.State{Step: "merge"}, + state: func() git.State { return git.State{State: "merge"} }, skip: "merge", skipped: true, }, + { + name: "when merge-commit", + state: func() git.State { return git.State{State: "merge-commit"} }, + skip: "merge-commit", + skipped: true, + }, { name: "when rebase (but want merge)", - state: git.State{Step: "rebase"}, + state: func() git.State { return git.State{State: "rebase"} }, skip: "merge", skipped: false, }, { name: "when rebase", - state: git.State{Step: "rebase"}, + state: func() git.State { return git.State{State: "rebase"} }, skip: []interface{}{"rebase"}, skipped: true, }, { name: "when rebase (but want merge)", - state: git.State{Step: "rebase"}, + state: func() git.State { return git.State{State: "rebase"} }, skip: []interface{}{"merge"}, skipped: false, }, { name: "when branch", - state: git.State{Branch: "feat/skipme"}, + state: func() git.State { return git.State{Branch: "feat/skipme"} }, skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, skipped: true, }, { name: "when branch doesn't match", - state: git.State{Branch: "feat/important"}, + state: func() git.State { return git.State{Branch: "feat/important"} }, skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, skipped: false, }, { name: "when branch glob", - state: git.State{Branch: "feat/important"}, + state: func() git.State { return git.State{Branch: "feat/important"} }, skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, skipped: true, }, { name: "when branch glob doesn't match", - state: git.State{Branch: "feat"}, + state: func() git.State { return git.State{Branch: "feat"} }, skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, skipped: false, }, { name: "when only specified", - state: git.State{Branch: "feat"}, + state: func() git.State { return git.State{Branch: "feat"} }, only: []interface{}{map[string]interface{}{"ref": "feat"}}, skipped: false, }, { name: "when only branch doesn't match", - state: git.State{Branch: "dev"}, + state: func() git.State { return git.State{Branch: "dev"} }, only: []interface{}{map[string]interface{}{"ref": "feat"}}, skipped: true, }, { name: "when only branch with glob", - state: git.State{Branch: "feat/important"}, + state: func() git.State { return git.State{Branch: "feat/important"} }, only: []interface{}{map[string]interface{}{"ref": "feat/*"}}, skipped: false, }, { name: "when only merge", - state: git.State{Step: "merge"}, + state: func() git.State { return git.State{State: "merge"} }, only: []interface{}{"merge"}, skipped: false, }, { name: "when only and skip", - state: git.State{Step: "rebase"}, + state: func() git.State { return git.State{State: "rebase"} }, skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, only: "rebase", skipped: false, }, { name: "when only and skip applies skip", - state: git.State{Step: "rebase"}, + state: func() git.State { return git.State{State: "rebase"} }, skip: []interface{}{"rebase"}, only: "rebase", skipped: true, }, { name: "when skip with run command", - state: git.State{}, + state: func() git.State { return git.State{} }, skip: []interface{}{map[string]interface{}{"run": "success"}}, skipped: true, }, { name: "when skip with multi-run command", - state: git.State{Branch: "feat"}, + state: func() git.State { return git.State{Branch: "feat"} }, skip: []interface{}{map[string]interface{}{"run": "success", "ref": "feat"}}, skipped: true, }, { name: "when only with run command", - state: git.State{}, + state: func() git.State { return git.State{} }, only: []interface{}{map[string]interface{}{"run": "fail"}}, skipped: true, }, diff --git a/internal/git/state.go b/internal/git/state.go index dda14139..940e06c7 100644 --- a/internal/git/state.go +++ b/internal/git/state.go @@ -5,38 +5,68 @@ import ( "os" "path/filepath" "regexp" + "strings" ) type State struct { - Branch, Step string + Branch, State string } const ( - NilStep string = "" - MergeStep string = "merge" - RebaseStep string = "rebase" + Nil string = "" + Merge string = "merge" + MergeCommit string = "merge-commit" + Rebase string = "rebase" ) -var refBranchRegexp = regexp.MustCompile(`^ref:\s*refs/heads/(.+)$`) +var ( + refBranchRegexp = regexp.MustCompile(`^ref:\s*refs/heads/(.+)$`) + cmdParentCommits = []string{"git", "show", "--no-patch", `--format="%P"`} +) + +var ( + state State + stateInitialized bool +) + +func ResetState() { + stateInitialized = false +} func (r *Repository) State() State { + if stateInitialized { + return state + } + + stateInitialized = true branch := r.Branch() if r.inMergeState() { - return State{ + state = State{ Branch: branch, - Step: MergeStep, + State: Merge, } + return state } if r.inRebaseState() { - return State{ + state = State{ Branch: branch, - Step: RebaseStep, + State: Rebase, } + return state } - return State{ + if r.inMergeCommitState() { + state = State{ + Branch: branch, + State: MergeCommit, + } + return state + } + + state = State{ Branch: branch, - Step: NilStep, + State: Nil, } + return state } func (r *Repository) Branch() string { @@ -81,3 +111,12 @@ func (r *Repository) inRebaseState() bool { return true } + +func (r *Repository) inMergeCommitState() bool { + parents, err := r.Git.Cmd(cmdParentCommits) + if err != nil { + return false + } + + return strings.Contains(parents, " ") +} diff --git a/internal/lefthook/run_test.go b/internal/lefthook/run_test.go index 01a7c800..aa9c6334 100644 --- a/internal/lefthook/run_test.go +++ b/internal/lefthook/run_test.go @@ -4,10 +4,10 @@ import ( "fmt" "io" "path/filepath" - "slices" "testing" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" "github.com/evilmartians/lefthook/internal/git" ) @@ -153,6 +153,7 @@ post-commit: }, } { t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { + assert := assert.New(t) fs := afero.NewMemMapFs() lefthook := &Lefthook{ Options: &Options{Fs: fs}, @@ -167,48 +168,27 @@ post-commit: // Create files that should exist for _, path := range tt.existingDirs { - if err := fs.MkdirAll(path, 0o755); err != nil { - t.Errorf("unexpected error: %s", err) - } - } - - err := afero.WriteFile(fs, configPath, []byte(tt.config), 0o644) - if err != nil { - t.Errorf("unexpected error: %s", err) + assert.NoError(fs.MkdirAll(path, 0o755)) } + assert.NoError(afero.WriteFile(fs, configPath, []byte(tt.config), 0o644)) for env, value := range tt.envs { t.Setenv(env, value) } + git.ResetState() err = lefthook.Run(tt.hook, RunArgs{}, tt.gitArgs) - if err != nil { - if !tt.error { - t.Errorf("unexpected error: %s", err) - } + if tt.error { + assert.Error(err) } else { - if tt.error { - t.Errorf("expected an error") - } + assert.NoError(err) } hookNameCompletions := lefthook.configHookCompletions() - if tt.hookNameCompletions != nil { - if !slices.Equal(tt.hookNameCompletions, hookNameCompletions) { - t.Errorf("expected hook name completions %v, got %v", tt.hookNameCompletions, hookNameCompletions) - } - } else if len(hookNameCompletions) != 0 { - t.Errorf("expected no hook name completions, got %v", lefthook.configHookCompletions()) - } + assert.ElementsMatch(tt.hookNameCompletions, hookNameCompletions) hookCommandCompletions := lefthook.configHookCommandCompletions(tt.hook) - if tt.hookCommandCompletions != nil { - if !slices.Equal(tt.hookCommandCompletions, hookCommandCompletions) { - t.Errorf("expected hook command completions %v, got %v", tt.hookCommandCompletions, hookCommandCompletions) - } - } else if len(hookCommandCompletions) != 0 { - t.Errorf("expected no hook command completions, got %v", hookCommandCompletions) - } + assert.ElementsMatch(tt.hookCommandCompletions, hookCommandCompletions) }) } } diff --git a/internal/lefthook/runner/prepare_command.go b/internal/lefthook/runner/prepare_command.go index e4e11378..9913f0d4 100644 --- a/internal/lefthook/runner/prepare_command.go +++ b/internal/lefthook/runner/prepare_command.go @@ -26,7 +26,7 @@ type template struct { } func (r *Runner) prepareCommand(name string, command *config.Command) (*run, error) { - if command.DoSkip(r.Repo.State()) { + if command.DoSkip(r.Repo.State) { return nil, &skipError{"settings"} } diff --git a/internal/lefthook/runner/prepare_script.go b/internal/lefthook/runner/prepare_script.go index 059817eb..b210de25 100644 --- a/internal/lefthook/runner/prepare_script.go +++ b/internal/lefthook/runner/prepare_script.go @@ -11,7 +11,7 @@ import ( ) func (r *Runner) prepareScript(script *config.Script, path string, file os.FileInfo) (string, error) { - if script.DoSkip(r.Repo.State()) { + if script.DoSkip(r.Repo.State) { return "", &skipError{"settings"} } diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index db3dce95..4aa2bba1 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -90,7 +90,7 @@ type executable interface { func (r *Runner) RunAll(ctx context.Context, sourceDirs []string) ([]Result, error) { results := make([]Result, 0, len(r.Hook.Commands)+len(r.Hook.Scripts)) - if r.Hook.DoSkip(r.Repo.State()) { + if r.Hook.DoSkip(r.Repo.State) { r.logSkip(r.HookName, "hook setting") return results, nil } diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/runner/runner_test.go index 2af044e6..d4330aa1 100644 --- a/internal/lefthook/runner/runner_test.go +++ b/internal/lefthook/runner/runner_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" "github.com/evilmartians/lefthook/internal/config" "github.com/evilmartians/lefthook/internal/git" @@ -71,9 +72,7 @@ func (g *gitCmd) reset() { func TestRunAll(t *testing.T) { root, err := filepath.Abs("src") - if err != nil { - t.Errorf("unexpected error: %s", err) - } + assert.NoError(t, err) gitExec := &gitCmd{} gitPath := filepath.Join(root, ".git") @@ -85,18 +84,17 @@ func TestRunAll(t *testing.T) { InfoPath: filepath.Join(gitPath, "info"), } - for i, tt := range [...]struct { - name, branch, hookName string - args []string - sourceDirs []string - existingFiles []string - hook *config.Hook - success, fail []Result - gitCommands []string - force bool + for name, tt := range map[string]struct { + branch, hookName string + args []string + sourceDirs []string + existingFiles []string + hook *config.Hook + success, fail []Result + gitCommands []string + force bool }{ - { - name: "empty hook", + "empty hook": { hookName: "post-commit", hook: &config.Hook{ Commands: map[string]*config.Command{}, @@ -104,8 +102,7 @@ func TestRunAll(t *testing.T) { Piped: true, }, }, - { - name: "with simple command", + "with simple command": { hookName: "post-commit", hook: &config.Hook{ Commands: map[string]*config.Command{ @@ -117,8 +114,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("test")}, }, - { - name: "with simple command in follow mode", + "with simple command in follow mode": { hookName: "post-commit", hook: &config.Hook{ Follow: true, @@ -131,8 +127,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("test")}, }, - { - name: "with multiple commands ran in parallel", + "with multiple commands ran in parallel": { hookName: "post-commit", hook: &config.Hook{ Parallel: true, @@ -155,8 +150,7 @@ func TestRunAll(t *testing.T) { }, fail: []Result{failed("type-check", "")}, }, - { - name: "with exclude tags", + "with exclude tags": { hookName: "post-commit", hook: &config.Hook{ ExcludeTags: []string{"tests", "formatter"}, @@ -177,8 +171,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("lint")}, }, - { - name: "with skip boolean option", + "with skip=true": { hookName: "post-commit", hook: &config.Hook{ Commands: map[string]*config.Command{ @@ -194,8 +187,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("lint")}, }, - { - name: "with skip merge", + "with skip=merge": { hookName: "post-commit", existingFiles: []string{ filepath.Join(gitPath, "MERGE_HEAD"), @@ -214,8 +206,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("lint")}, }, - { - name: "with only on merge", + "with only=merge match": { hookName: "post-commit", existingFiles: []string{ filepath.Join(gitPath, "MERGE_HEAD"), @@ -237,8 +228,7 @@ func TestRunAll(t *testing.T) { succeeded("test"), }, }, - { - name: "with only on merge", + "with only=merge no match": { hookName: "post-commit", hook: &config.Hook{ Commands: map[string]*config.Command{ @@ -252,10 +242,10 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, - success: []Result{succeeded("lint")}, + gitCommands: []string{`git show --no-patch --format="%P"`}, + success: []Result{succeeded("lint")}, }, - { - name: "with global skip merge", + "with hook's skip=merge match": { hookName: "post-commit", existingFiles: []string{ filepath.Join(gitPath, "MERGE_HEAD"), @@ -274,8 +264,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{}, }, - { - name: "with global only on merge", + "with hook's skip=merge no match": { hookName: "post-commit", hook: &config.Hook{ Only: "merge", @@ -289,10 +278,10 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, - success: []Result{}, + gitCommands: []string{`git show --no-patch --format="%P"`}, + success: []Result{}, }, - { - name: "with global only on merge", + "with hook's only=merge match": { hookName: "post-commit", existingFiles: []string{ filepath.Join(gitPath, "MERGE_HEAD"), @@ -314,8 +303,7 @@ func TestRunAll(t *testing.T) { succeeded("test"), }, }, - { - name: "with skip rebase and merge in an array", + "with skip=[merge, rebase] match rebase": { hookName: "post-commit", existingFiles: []string{ filepath.Join(gitPath, "rebase-merge"), @@ -335,8 +323,7 @@ func TestRunAll(t *testing.T) { }, success: []Result{succeeded("lint")}, }, - { - name: "with global skip on ref", + "with skip=ref match": { branch: "main", existingFiles: []string{ filepath.Join(gitPath, "HEAD"), @@ -354,10 +341,10 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, - success: []Result{}, + gitCommands: []string{`git show --no-patch --format="%P"`}, + success: []Result{}, }, - { - name: "with global only on ref", + "with hook's only=ref match": { branch: "main", existingFiles: []string{ filepath.Join(gitPath, "HEAD"), @@ -375,13 +362,13 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, + gitCommands: []string{`git show --no-patch --format="%P"`}, success: []Result{ succeeded("lint"), succeeded("test"), }, }, - { - name: "with global only on ref", + "with hook's only=ref no match": { branch: "develop", existingFiles: []string{ filepath.Join(gitPath, "HEAD"), @@ -399,10 +386,10 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, - success: []Result{}, + gitCommands: []string{`git show --no-patch --format="%P"`}, + success: []Result{}, }, - { - name: "with global skip on another ref", + "with hook's skip=ref no match": { branch: "fix", existingFiles: []string{ filepath.Join(gitPath, "HEAD"), @@ -420,13 +407,13 @@ func TestRunAll(t *testing.T) { }, Scripts: map[string]*config.Script{}, }, + gitCommands: []string{`git show --no-patch --format="%P"`}, success: []Result{ succeeded("test"), succeeded("lint"), }, }, - { - name: "with fail test", + "with fail": { hookName: "post-commit", hook: &config.Hook{ Commands: map[string]*config.Command{ @@ -439,8 +426,7 @@ func TestRunAll(t *testing.T) { }, fail: []Result{failed("test", "try 'success'")}, }, - { - name: "with simple scripts", + "with simple scripts": { sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), @@ -462,8 +448,7 @@ func TestRunAll(t *testing.T) { success: []Result{succeeded("script.sh")}, fail: []Result{failed("failing.js", "install node")}, }, - { - name: "with simple scripts and only option", + "with simple scripts and only=merge match": { sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), @@ -488,8 +473,7 @@ func TestRunAll(t *testing.T) { success: []Result{succeeded("script.sh")}, fail: []Result{failed("failing.js", "install node")}, }, - { - name: "with simple scripts and only option", + "with simple scripts and only=merge no match": { sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), @@ -510,11 +494,11 @@ func TestRunAll(t *testing.T) { }, }, }, - success: []Result{}, - fail: []Result{}, + gitCommands: []string{`git show --no-patch --format="%P"`}, + success: []Result{}, + fail: []Result{}, }, - { - name: "with interactive and parallel", + "with interactive=true, parallel=true": { sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), @@ -545,8 +529,7 @@ func TestRunAll(t *testing.T) { success: []Result{}, // script.sh and ok are skipped because of non-interactive cmd failure fail: []Result{failed("failing.js", ""), failed("fail", "")}, }, - { - name: "with stage_fixed in true", + "with stage_fixed=true": { sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ filepath.Join(root, config.DefaultSourceDir, "post-commit", "success.sh"), @@ -578,8 +561,7 @@ func TestRunAll(t *testing.T) { success: []Result{succeeded("ok"), succeeded("success.sh")}, fail: []Result{failed("fail", ""), failed("failing.js", "")}, }, - { - name: "pre-commit hook simple", + "with simple pre-commit": { hookName: "pre-commit", sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, existingFiles: []string{ @@ -624,8 +606,7 @@ func TestRunAll(t *testing.T) { "git stash list", }, }, - { - name: "pre-commit hook with implicit skip", + "with pre-commit skip": { hookName: "pre-commit", existingFiles: []string{ filepath.Join(root, "README.md"), @@ -655,8 +636,7 @@ func TestRunAll(t *testing.T) { "git stash list", }, }, - { - name: "skippable pre-commit hook with force", + "with pre-commit skip but forced": { hookName: "pre-commit", existingFiles: []string{ filepath.Join(root, "README.md"), @@ -686,8 +666,7 @@ func TestRunAll(t *testing.T) { "git stash list", }, }, - { - name: "pre-commit hook with stage_fixed under root", + "with pre-commit and stage_fixed=true under root": { hookName: "pre-commit", existingFiles: []string{ filepath.Join(root, "scripts", "script.sh"), @@ -712,8 +691,7 @@ func TestRunAll(t *testing.T) { "git stash list", }, }, - { - name: "pre-push hook with implicit skip", + "with pre-push skip": { hookName: "pre-push", existingFiles: []string{ filepath.Join(root, "README.md"), @@ -756,25 +734,19 @@ func TestRunAll(t *testing.T) { gitExec.reset() for _, file := range tt.existingFiles { - if err := fs.MkdirAll(filepath.Dir(file), 0o755); err != nil { - t.Errorf("unexpected error: %s", err) - } - if err := afero.WriteFile(fs, file, []byte{}, 0o755); err != nil { - t.Errorf("unexpected error: %s", err) - } + assert.NoError(t, fs.MkdirAll(filepath.Dir(file), 0o755)) + assert.NoError(t, afero.WriteFile(fs, file, []byte{}, 0o755)) } if len(tt.branch) > 0 { - if err := afero.WriteFile(fs, filepath.Join(gitPath, "HEAD"), []byte("ref: refs/heads/"+tt.branch), 0o644); err != nil { - t.Errorf("unexpected error: %s", err) - } + assert.NoError(t, afero.WriteFile(fs, filepath.Join(gitPath, "HEAD"), []byte("ref: refs/heads/"+tt.branch), 0o644)) } - t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + git.ResetState() results, err := runner.RunAll(context.Background(), tt.sourceDirs) - if err != nil { - t.Errorf("unexpected error %s", err) - } + assert.NoError(err) var success, fail []Result for _, result := range results { @@ -785,50 +757,20 @@ func TestRunAll(t *testing.T) { } } - if !resultsMatch(tt.success, success) { - t.Errorf("success results are not matching\n Need: %v\n Was: %v", tt.success, success) - } + assert.ElementsMatch(success, tt.success) + assert.ElementsMatch(fail, tt.fail) - if !resultsMatch(tt.fail, fail) { - t.Errorf("fail results are not matching:\n Need: %v\n Was: %v", tt.fail, fail) - } - - if len(gitExec.commands) != len(tt.gitCommands) { - t.Errorf("wrong git commands\nExpected: %#v\nWas: %#v", tt.gitCommands, gitExec.commands) - } else { - for i, command := range gitExec.commands { - gitCommandRe := regexp.MustCompile(tt.gitCommands[i]) - if !gitCommandRe.MatchString(command) { - t.Errorf("wrong git command regexp #%d\nExpected: %s\nWas: %s", i, tt.gitCommands[i], command) - } + assert.Len(gitExec.commands, len(tt.gitCommands)) + for i, command := range gitExec.commands { + gitCommandRe := regexp.MustCompile(tt.gitCommands[i]) + if !gitCommandRe.MatchString(command) { + t.Errorf("wrong git command regexp #%d\nExpected: %s\nWas: %s", i, tt.gitCommands[i], command) } } }) } } -func resultsMatch(a, b []Result) bool { - if len(a) != len(b) { - return false - } - - matches := make(map[string]struct{}) - - for _, item := range a { - str := fmt.Sprintf("%v", item) - matches[str] = struct{}{} - } - - for _, item := range b { - str := fmt.Sprintf("%v", item) - if _, ok := matches[str]; !ok { - return false - } - } - - return true -} - func TestReplaceQuoted(t *testing.T) { for i, tt := range [...]struct { name, source, substitution string @@ -894,9 +836,7 @@ func TestReplaceQuoted(t *testing.T) { } { t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) { result := replaceQuoted(tt.source, tt.substitution, tt.files) - if result != tt.result { - t.Errorf("Expected `%s` to eq `%s`", result, tt.result) - } + assert.Equal(t, result, tt.result) }) } } @@ -928,11 +868,7 @@ func TestSortByPriorityCommands(t *testing.T) { } { t.Run(fmt.Sprintf("%d: %s", i+1, tt.name), func(t *testing.T) { sortByPriority(tt.names, tt.commands) - for i, name := range tt.result { - if tt.names[i] != name { - t.Errorf("Not matching on index %d: %s != %s", i, name, tt.names[i]) - } - } + assert.Equal(t, tt.result, tt.names) }) } } @@ -964,11 +900,7 @@ func TestSortByPriorityScripts(t *testing.T) { } { t.Run(fmt.Sprintf("%d: %s", i+1, tt.name), func(t *testing.T) { sortByPriority(tt.names, tt.scripts) - for i, name := range tt.result { - if tt.names[i] != name { - t.Errorf("Not matching on index %d: %s != %s", i, name, tt.names[i]) - } - } + assert.Equal(t, tt.result, tt.names) }) } } diff --git a/testdata/skip_merge_commit.txt b/testdata/skip_merge_commit.txt new file mode 100644 index 00000000..a3ef2e11 --- /dev/null +++ b/testdata/skip_merge_commit.txt @@ -0,0 +1,38 @@ +exec git init +exec git config user.email "you@example.com" +exec git config user.name "Your Name" +exec git add -A +exec git commit -m 'commit 1' +exec lefthook install + +exec git checkout -b merge-me +exec touch file.A +exec git add -A +exec git commit -m 'commit A' + +exec lefthook run pre-commit --force +stdout 'skip-merge-commit' + +exec git checkout - +exec touch file.B +exec git add -A +exec git commit -m 'commit B' + +exec git merge merge-me + +exec lefthook run pre-commit --force +! stdout 'skip-merge-commit' + +-- lefthook.yml -- +skip_output: + - skips + - meta + - summary + - execution_info + +pre-commit: + commands: + skip-merge-commit: + skip: + - merge-commit + run: echo 'skip-merge-commit'