Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle "run" command in the skip/only setting #634

Merged
merged 10 commits into from
Mar 13, 2024
15 changes: 15 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,21 @@ pre-commit:
run: yarn test
```

Skipping hook by running a command:

```yml
# lefthook.yml

pre-commit:
skip:
- run: test "${NO_HOOK}" -eq 1
commands:
lint:
run: yarn lint
text:
run: yarn test
```

**Notes**

Always skipping is useful when you have a `lefthook-local.yml` config and you don't want to run some commands locally. So you just overwrite the `skip` option for them to be `true`.
Expand Down
3 changes: 2 additions & 1 deletion internal/config/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func (c Command) Validate() error {
}

func (c Command) DoSkip(gitState git.State) bool {
return doSkip(gitState, c.Skip, c.Only)
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, c.Skip, c.Only)
}

type commandRunReplace struct {
Expand Down
35 changes: 35 additions & 0 deletions internal/config/exec.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package config

import (
"os/exec"
"runtime"
)

type Exec interface {
Cmd(commandLine string) bool
}

type osExec struct{}

// NewOsExec returns an object that executes given commands in the OS.
func NewOsExec() Exec {
return &osExec{}
}

// Cmd runs plain string command. It checks only exit code and returns bool value.
func (o *osExec) Cmd(commandLine string) bool {
if commandLine == "" {
return false
}

var cmd *exec.Cmd
if runtime.GOOS == "windows" {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrexox I added calling the command via sh -c but as you said, it may not work with Windows if it does not include WSL, Cygwin, and so on.

So I added checking the OS, but I couldn't check it because we don't have a running test-integrity for windows here

I think there was a reason why it was not added to ci-cd for windows :)
But what do you think about checking the windows and adding test integrity for Windows to ci-cd?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try it out!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrexox, Unfortunately, the `[windows] skip' command was necessary to skip some of the Windows test integrations.

Also, I use PowerShell for Windows commands because it supports more complex commands (e.g. with if).

We can avoid skipping some test-integrities, but it requires adapting those tests, and it is difficult without a Windows machine.

The example of skipping test here

Copy link
Contributor Author

@prog-supdex prog-supdex Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we checked the skip run for Windows, and it's working.
I added the separated file skip_run_windows for it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I think this is acceptable for now. Thank you for digging into testscript, I appreciate it. I will try to figure out how to make it work for Windows when I have a chance to work on a Windows machine.

cmd = exec.Command("cmd", "/C", commandLine)
} else {
cmd = exec.Command("sh", "-c", commandLine)
}

err := cmd.Run()

return err == nil
}
3 changes: 2 additions & 1 deletion internal/config/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func (h *Hook) Validate() error {
}

func (h *Hook) DoSkip(gitState git.State) bool {
return doSkip(gitState, h.Skip, h.Only)
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, h.Skip, h.Only)
}

func unmarshalHooks(base, extra *viper.Viper) (*Hook, error) {
Expand Down
3 changes: 2 additions & 1 deletion internal/config/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ type Script struct {
}

func (s Script) DoSkip(gitState git.State) bool {
return doSkip(gitState, s.Skip, s.Only)
skipChecker := NewSkipChecker(NewOsExec())
return skipChecker.Check(gitState, s.Skip, s.Only)
}

type scriptRunnerReplace struct {
Expand Down
50 changes: 0 additions & 50 deletions internal/config/skip.go
mrexox marked this conversation as resolved.
Outdated
Show resolved Hide resolved

This file was deleted.

95 changes: 95 additions & 0 deletions internal/config/skip_checker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package config

import (
"github.com/gobwas/glob"

"github.com/evilmartians/lefthook/internal/git"
"github.com/evilmartians/lefthook/internal/log"
)

type SkipChecker struct {
Executor Exec
}

func NewSkipChecker(executor Exec) *SkipChecker {
if executor == nil {
executor = NewOsExec()
}

return &SkipChecker{Executor: executor}
}

func (sc *SkipChecker) Check(gitState git.State, skip interface{}, only interface{}) bool {
if skip != nil {
if sc.matches(gitState, skip) {
return true
}
}

if only != nil {
return !sc.matches(gitState, only)
}

return false
}

func (sc *SkipChecker) matches(gitState git.State, value interface{}) bool {
switch typedValue := value.(type) {
case bool:
return typedValue
case string:
return typedValue == gitState.Step
case []interface{}:
return sc.matchesSlices(gitState, typedValue)
}
return false
}

func (sc *SkipChecker) matchesSlices(gitState git.State, slice []interface{}) bool {
for _, state := range slice {
switch typedState := state.(type) {
case string:
if typedState == gitState.Step {
return true
}
case map[string]interface{}:
if sc.matchesRef(gitState, typedState) {
return true
}

if sc.matchesCommands(typedState) {
return true
}
}
}

return false
}

func (sc *SkipChecker) matchesRef(gitState git.State, typedState map[string]interface{}) bool {
ref, ok := typedState["ref"].(string)
if !ok {
return false
}

if ref == gitState.Branch {
return true
}

g := glob.MustCompile(ref)

return g.Match(gitState.Branch)
}

func (sc *SkipChecker) matchesCommands(typedState map[string]interface{}) bool {
commandLine, ok := typedState["run"].(string)
if !ok {
return false
}

result := sc.Executor.Cmd(commandLine)

log.Debugf("[lefthook] skip/only cmd: %s, result: %t", commandLine, result)

return result
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@ import (
"github.com/evilmartians/lefthook/internal/git"
)

type mockExecutor struct{}

func (mc mockExecutor) Cmd(cmd string) bool {
return cmd == "success"
}

func TestDoSkip(t *testing.T) {
skipChecker := NewSkipChecker(mockExecutor{})

for _, tt := range [...]struct {
name string
state git.State
Expand Down Expand Up @@ -111,9 +119,27 @@ func TestDoSkip(t *testing.T) {
only: "rebase",
skipped: true,
},
{
name: "when skip with run command",
state: git.State{},
skip: []interface{}{map[string]interface{}{"run": "success"}},
skipped: true,
},
{
name: "when skip with multi-run command",
state: git.State{Branch: "feat"},
skip: []interface{}{map[string]interface{}{"run": "success", "ref": "feat"}},
skipped: true,
},
{
name: "when only with run command",
state: git.State{},
only: []interface{}{map[string]interface{}{"run": "fail"}},
skipped: true,
},
} {
t.Run(tt.name, func(t *testing.T) {
if doSkip(tt.state, tt.skip, tt.only) != tt.skipped {
if skipChecker.Check(tt.state, tt.skip, tt.only) != tt.skipped {
t.Errorf("Expected: %v, Was %v", tt.skipped, !tt.skipped)
}
})
Expand Down
38 changes: 38 additions & 0 deletions testdata/skip_run.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
exec git init
exec git add -A
exec lefthook run skip
! stdout 'Ha-ha!'
exec lefthook run no-skip
stdout 'Ha-ha!'
exec lefthook run skip-var
! stdout 'Ha-ha!'
env VAR=1
exec lefthook run skip-var
stdout 'Ha-ha!'

-- lefthook.yml --
skip_output:
- skips
- meta
- summary
- execution_info
skip:
skip:
- run: test 1 -eq 1
commands:
echo:
run: echo 'Ha-ha!'

no-skip:
skip:
- run: "[ 1 -eq 0 ]"
commands:
echo:
run: echo 'Ha-ha!'

skip-var:
skip:
- run: test -z $VAR
commands:
echo:
run: echo 'Ha-ha!'
Loading