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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ jobs:
strategy:
matrix:
go-version: [1.21.x]
os: [ubuntu-latest, macos-latest]
os: [ubuntu-latest, macos-latest, windows-latest]
runs-on: ${{ matrix.os }}
env:
GOCOVERDIR: ${{ github.workspace }}/_icoverdir_
Expand Down Expand Up @@ -91,5 +91,5 @@ jobs:
steps:
- uses: coverallsapp/github-action@v2
with:
carryforward: "integration-1.21.x ubuntu-latest,integration-1.21.x macos-latest,1.21.x ubuntu-latest,1.21.x macos-latest,1.21.x windows-latest"
carryforward: "integration-1.21.x ubuntu-latest,integration-1.21.x macos-latest,integration-1.21.x windows-latest,1.21.x ubuntu-latest,1.21.x macos-latest,1.21.x windows-latest"
parallel-finished: true
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("powershell", "-Command", 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

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
2 changes: 2 additions & 0 deletions testdata/add.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

exec git init
exec lefthook add pre-commit
! stderr .
Expand Down
2 changes: 2 additions & 0 deletions testdata/dump.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

exec git init
exec lefthook dump
cmp stdout lefthook-dumped.yml
Expand Down
2 changes: 2 additions & 0 deletions testdata/hide_unstaged.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

exec git init
exec lefthook install
exec git config user.email "[email protected]"
Expand Down
2 changes: 2 additions & 0 deletions testdata/remote.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

exec git init
exec lefthook install

Expand Down
2 changes: 2 additions & 0 deletions testdata/remotes.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

exec git init
exec lefthook install

Expand Down
2 changes: 2 additions & 0 deletions testdata/run_interrupt.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
[windows] skip

chmod 0700 hook.sh
chmod 0700 commit-with-interrupt.sh
exec git init
Expand Down
3 changes: 3 additions & 0 deletions testdata/sh_syntax_in_files.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
[windows] skip

exec git init
exec lefthook install
exec git config user.email "[email protected]"
exec git config user.name "Your Name"

exec lefthook run echo_files

stdout '1.txt 10.txt'

-- lefthook.yml --
Expand Down
42 changes: 42 additions & 0 deletions testdata/skip_run.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[windows] skip

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
Loading