From 1bd921f26c372dff99b8c37d0ccb47bf798f0b08 Mon Sep 17 00:00:00 2001 From: Valentin Kiselev Date: Thu, 11 May 2023 11:22:19 +0300 Subject: [PATCH] feat: add only option (#478) --- docs/configuration.md | 46 +++++ internal/config/command.go | 6 +- internal/config/hook.go | 6 +- internal/config/load_test.go | 18 +- internal/config/script.go | 6 +- internal/config/skip.go | 16 +- internal/config/skip_test.go | 68 ++++++-- 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 | 176 ++++++++++++++++++++ 11 files changed, 310 insertions(+), 38 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index a774ca9e..febbca87 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -15,6 +15,7 @@ - [`config`](#config) - [Hook](#git-hook) - [`skip`](#skip) + - [`only`](#only) - [`files`](#files-global) - [`parallel`](#parallel) - [`piped`](#piped) @@ -25,6 +26,7 @@ - [Command](#command) - [`run`](#run) - [`skip`](#skip) + - [`only`](#only) - [`tags`](#tags) - [`glob`](#glob) - [`files`](#files) @@ -37,6 +39,7 @@ - [Script](#script) - [`runner`](#runner) - [`skip`](#skip) + - [`only`](#only) - [`tags`](#tags) - [`env`](#env) - [`fail_text`](#fail_text) @@ -718,6 +721,49 @@ pre-commit: skip: true ``` +### `only` + +You can force a command, script, or the whole hook to execute only in certain conditions. This option acts like the opposite of [`skip`](#skip). It accepts the same values but skips execution only if the condition is not satisfied. + +**Note** + +`skip` option takes precedence over `only` option, so if you have conflicting conditions the execution will be skipped. + +**Example** + +Execute a hook only for `dev/*` branches. + +```yml +# lefthook.yml + +pre-commit: + only: + - ref: dev/* + commands: + lint: + run: yarn lint + test: + run: yarn test +``` + +When rebasing execute quick linter but skip usual linter and tests. + +```yml +# lefthook.yml + +pre-commit: + commands: + lint: + skip: rebase + run: yarn lint + test: + skip: rebase + run: yarn test + lint-on-rebase: + only: rebase + run: yarn lint-quickly +``` + ### `tags` You can specify tags for commands and scripts. This is useful for [excluding](#exclude_tags). You can specify more than one tag using comma or space. diff --git a/internal/config/command.go b/internal/config/command.go index 9abe0919..49d9c4b4 100644 --- a/internal/config/command.go +++ b/internal/config/command.go @@ -15,6 +15,7 @@ type Command struct { Run string `mapstructure:"run"` Skip interface{} `mapstructure:"skip"` + Only interface{} `mapstructure:"only"` Tags []string `mapstructure:"tags"` Glob string `mapstructure:"glob"` Files string `mapstructure:"files"` @@ -37,10 +38,7 @@ func (c Command) Validate() error { } func (c Command) DoSkip(gitState git.State) bool { - if value := c.Skip; value != nil { - return isSkip(gitState, value) - } - return false + return doSkip(gitState, c.Skip, c.Only) } type commandRunReplace struct { diff --git a/internal/config/hook.go b/internal/config/hook.go index bae52107..2b538c14 100644 --- a/internal/config/hook.go +++ b/internal/config/hook.go @@ -30,6 +30,7 @@ type Hook struct { Piped bool `mapstructure:"piped"` ExcludeTags []string `mapstructure:"exclude_tags"` Skip interface{} `mapstructure:"skip"` + Only interface{} `mapstructure:"only"` Follow bool `mapstructure:"follow"` } @@ -42,10 +43,7 @@ func (h *Hook) Validate() error { } func (h *Hook) DoSkip(gitState git.State) bool { - if value := h.Skip; value != nil { - return isSkip(gitState, value) - } - return false + return doSkip(gitState, h.Skip, h.Only) } func unmarshalHooks(base, extra *viper.Viper) (*Hook, error) { diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 241a7779..e712d9cf 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -97,7 +97,7 @@ pre-commit: run: docker exec -it ruby:2.7 {cmd} scripts: "format.sh": - skip: true + only: true pre-push: commands: @@ -128,7 +128,7 @@ pre-push: }, Scripts: map[string]*Script{ "format.sh": { - Skip: true, + Only: true, Runner: "bash", }, }, @@ -265,7 +265,7 @@ remote: config: examples/custom.yml pre-commit: - skip: + only: - ref: main commands: global: @@ -277,11 +277,14 @@ pre-commit: pre-commit: commands: lint: - run: yarn lint - skip: + only: - merge + - rebase + run: yarn lint scripts: "test.sh": + skip: + - merge runner: bash `, remoteConfigPath: filepath.Join(root, ".git", "info", "lefthook-remotes", "lefthook", "examples", "custom.yml"), @@ -296,11 +299,11 @@ pre-commit: }, Hooks: map[string]*Hook{ "pre-commit": { - Skip: []interface{}{map[string]interface{}{"ref": "main"}}, + Only: []interface{}{map[string]interface{}{"ref": "main"}}, Commands: map[string]*Command{ "lint": { Run: "yarn lint", - Skip: []interface{}{"merge"}, + Only: []interface{}{"merge", "rebase"}, }, "global": { Run: "echo 'Global!'", @@ -309,6 +312,7 @@ pre-commit: Scripts: map[string]*Script{ "test.sh": { Runner: "bash", + Skip: []interface{}{"merge"}, }, }, }, diff --git a/internal/config/script.go b/internal/config/script.go index d2041fb6..41c0bc52 100644 --- a/internal/config/script.go +++ b/internal/config/script.go @@ -13,6 +13,7 @@ type Script struct { Runner string `mapstructure:"runner"` Skip interface{} `mapstructure:"skip"` + Only interface{} `mapstructure:"only"` Tags []string `mapstructure:"tags"` Env map[string]string `mapstructure:"env"` @@ -22,10 +23,7 @@ type Script struct { } func (s Script) DoSkip(gitState git.State) bool { - if value := s.Skip; value != nil { - return isSkip(gitState, value) - } - return false + return doSkip(gitState, s.Skip, s.Only) } type scriptRunnerReplace struct { diff --git a/internal/config/skip.go b/internal/config/skip.go index 556cc55d..9b52f3bd 100644 --- a/internal/config/skip.go +++ b/internal/config/skip.go @@ -6,7 +6,21 @@ import ( "github.com/evilmartians/lefthook/internal/git" ) -func isSkip(gitState git.State, value interface{}) bool { +func doSkip(gitState git.State, skip, only interface{}) bool { + if skip != nil { + if matches(gitState, skip) { + return true + } + } + + if only != nil { + return !matches(gitState, only) + } + + return false +} + +func matches(gitState git.State, value interface{}) bool { switch typedValue := value.(type) { case bool: return typedValue diff --git a/internal/config/skip_test.go b/internal/config/skip_test.go index 2956a33c..d4d3c6f3 100644 --- a/internal/config/skip_test.go +++ b/internal/config/skip_test.go @@ -6,76 +6,114 @@ import ( "github.com/evilmartians/lefthook/internal/git" ) -func TestIsSkip(t *testing.T) { +func TestDoSkip(t *testing.T) { for _, tt := range [...]struct { - name string - skip interface{} - state git.State - skipped bool + name string + state git.State + skip, only interface{} + skipped bool }{ { name: "when true", - skip: true, state: git.State{}, + skip: true, skipped: true, }, { name: "when false", - skip: false, state: git.State{}, + skip: false, skipped: false, }, { name: "when merge", - skip: "merge", state: git.State{Step: "merge"}, + skip: "merge", skipped: true, }, { name: "when rebase (but want merge)", - skip: "merge", state: git.State{Step: "rebase"}, + skip: "merge", skipped: false, }, { name: "when rebase", - skip: []interface{}{"rebase"}, state: git.State{Step: "rebase"}, + skip: []interface{}{"rebase"}, skipped: true, }, { name: "when rebase (but want merge)", - skip: []interface{}{"merge"}, state: git.State{Step: "rebase"}, + skip: []interface{}{"merge"}, skipped: false, }, { name: "when branch", - skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, state: git.State{Branch: "feat/skipme"}, + skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, skipped: true, }, { name: "when branch doesn't match", - skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, state: git.State{Branch: "feat/important"}, + skip: []interface{}{map[string]interface{}{"ref": "feat/skipme"}}, skipped: false, }, { name: "when branch glob", - skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, state: 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"}, skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, + skipped: false, + }, + { + name: "when only specified", state: 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"}, + only: []interface{}{map[string]interface{}{"ref": "feat"}}, + skipped: true, + }, + { + name: "when only branch with glob", + state: git.State{Branch: "feat/important"}, + only: []interface{}{map[string]interface{}{"ref": "feat/*"}}, + skipped: false, + }, + { + name: "when only merge", + state: git.State{Step: "merge"}, + only: []interface{}{"merge"}, + skipped: false, + }, + { + name: "when only and skip", + state: git.State{Step: "rebase"}, + skip: []interface{}{map[string]interface{}{"ref": "feat/*"}}, + only: "rebase", + skipped: false, + }, + { + name: "when only and skip applies skip", + state: git.State{Step: "rebase"}, + skip: []interface{}{"rebase"}, + only: "rebase", + skipped: true, + }, } { t.Run(tt.name, func(t *testing.T) { - if isSkip(tt.state, tt.skip) != tt.skipped { + if doSkip(tt.state, tt.skip, tt.only) != tt.skipped { t.Errorf("Expected: %v, Was %v", tt.skipped, !tt.skipped) } }) diff --git a/internal/lefthook/runner/prepare_command.go b/internal/lefthook/runner/prepare_command.go index 00064d9d..aab1c231 100644 --- a/internal/lefthook/runner/prepare_command.go +++ b/internal/lefthook/runner/prepare_command.go @@ -17,7 +17,7 @@ type commandArgs struct { } func (r *Runner) prepareCommand(name string, command *config.Command) (*commandArgs, error) { - if command.Skip != nil && command.DoSkip(r.Repo.State()) { + if command.DoSkip(r.Repo.State()) { return nil, errors.New("settings") } diff --git a/internal/lefthook/runner/prepare_script.go b/internal/lefthook/runner/prepare_script.go index eeb2e174..c275e14e 100644 --- a/internal/lefthook/runner/prepare_script.go +++ b/internal/lefthook/runner/prepare_script.go @@ -10,7 +10,7 @@ import ( ) func (r *Runner) prepareScript(script *config.Script, path string, file os.FileInfo) ([]string, error) { - if script.Skip != nil && script.DoSkip(r.Repo.State()) { + if script.DoSkip(r.Repo.State()) { return nil, errors.New("settings") } diff --git a/internal/lefthook/runner/runner.go b/internal/lefthook/runner/runner.go index 06c7209f..689f65b2 100644 --- a/internal/lefthook/runner/runner.go +++ b/internal/lefthook/runner/runner.go @@ -63,7 +63,7 @@ func (r *Runner) RunAll(sourceDirs []string) { log.Error(err) } - if r.Hook.Skip != nil && r.Hook.DoSkip(r.Repo.State()) { + if r.Hook.DoSkip(r.Repo.State()) { r.logSkip(r.HookName, "hook setting") return } diff --git a/internal/lefthook/runner/runner_test.go b/internal/lefthook/runner/runner_test.go index 92d1ef1b..61479d84 100644 --- a/internal/lefthook/runner/runner_test.go +++ b/internal/lefthook/runner/runner_test.go @@ -228,6 +228,46 @@ func TestRunAll(t *testing.T) { }, success: []Result{{Name: "lint", Status: StatusOk}}, }, + { + name: "with only on merge", + hookName: "post-commit", + existingFiles: []string{ + filepath.Join(gitPath, "MERGE_HEAD"), + }, + hook: &config.Hook{ + Commands: map[string]*config.Command{ + "test": { + Run: "success", + Only: "merge", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{ + {Name: "lint", Status: StatusOk}, + {Name: "test", Status: StatusOk}, + }, + }, + { + name: "with only on merge", + hookName: "post-commit", + hook: &config.Hook{ + Commands: map[string]*config.Command{ + "test": { + Run: "success", + Only: "merge", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{{Name: "lint", Status: StatusOk}}, + }, { name: "with global skip merge", hookName: "post-commit", @@ -248,6 +288,46 @@ func TestRunAll(t *testing.T) { }, success: []Result{}, }, + { + name: "with global only on merge", + hookName: "post-commit", + hook: &config.Hook{ + Only: "merge", + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{}, + }, + { + name: "with global only on merge", + hookName: "post-commit", + existingFiles: []string{ + filepath.Join(gitPath, "MERGE_HEAD"), + }, + hook: &config.Hook{ + Only: "merge", + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{ + {Name: "lint", Status: StatusOk}, + {Name: "test", Status: StatusOk}, + }, + }, { name: "with skip rebase and merge in an array", hookName: "post-commit", @@ -290,6 +370,51 @@ func TestRunAll(t *testing.T) { }, success: []Result{}, }, + { + name: "with global only on ref", + branch: "main", + existingFiles: []string{ + filepath.Join(gitPath, "HEAD"), + }, + hookName: "post-commit", + hook: &config.Hook{ + Only: []interface{}{"merge", map[string]interface{}{"ref": "main"}}, + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{ + {Name: "lint", Status: StatusOk}, + {Name: "test", Status: StatusOk}, + }, + }, + { + name: "with global only on ref", + branch: "develop", + existingFiles: []string{ + filepath.Join(gitPath, "HEAD"), + }, + hookName: "post-commit", + hook: &config.Hook{ + Only: []interface{}{"merge", map[string]interface{}{"ref": "main"}}, + Commands: map[string]*config.Command{ + "test": { + Run: "success", + }, + "lint": { + Run: "success", + }, + }, + Scripts: map[string]*config.Script{}, + }, + success: []Result{}, + }, { name: "with global skip on another ref", branch: "fix", @@ -351,6 +476,57 @@ func TestRunAll(t *testing.T) { success: []Result{{Name: "script.sh", Status: StatusOk}}, fail: []Result{{Name: "failing.js", Status: StatusErr, Text: "install node"}}, }, + { + name: "with simple scripts and only option", + sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, + existingFiles: []string{ + filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), + filepath.Join(root, config.DefaultSourceDir, "post-commit", "failing.js"), + filepath.Join(gitPath, "MERGE_HEAD"), + }, + hookName: "post-commit", + hook: &config.Hook{ + Commands: map[string]*config.Command{}, + Scripts: map[string]*config.Script{ + "script.sh": { + Runner: "success", + Only: "merge", + }, + "failing.js": { + Only: "merge", + Runner: "fail", + FailText: "install node", + }, + }, + }, + success: []Result{{Name: "script.sh", Status: StatusOk}}, + fail: []Result{{Name: "failing.js", Status: StatusErr, Text: "install node"}}, + }, + { + name: "with simple scripts and only option", + sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)}, + existingFiles: []string{ + filepath.Join(root, config.DefaultSourceDir, "post-commit", "script.sh"), + filepath.Join(root, config.DefaultSourceDir, "post-commit", "failing.js"), + }, + hookName: "post-commit", + hook: &config.Hook{ + Commands: map[string]*config.Command{}, + Scripts: map[string]*config.Script{ + "script.sh": { + Only: "merge", + Runner: "success", + }, + "failing.js": { + Only: "merge", + Runner: "fail", + FailText: "install node", + }, + }, + }, + success: []Result{}, + fail: []Result{}, + }, { name: "with interactive and parallel", sourceDirs: []string{filepath.Join(root, config.DefaultSourceDir)},