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

chore: refactor commands interfaces #735

Merged
merged 4 commits into from
May 31, 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
2 changes: 1 addition & 1 deletion .goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ changelog:
- '^spec:'
- '^tmp:'
- '^context:'
- '^\d\.\d\.\d:'
- '^\d+\.\d+\.\d+:'

snapcrafts:
-
Expand Down
2 changes: 1 addition & 1 deletion internal/config/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (c Command) Validate() error {
}

func (c Command) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(system.Executor{})
skipChecker := NewSkipChecker(system.Cmd)
return skipChecker.check(gitState, c.Skip, c.Only)
}

Expand Down
14 changes: 6 additions & 8 deletions internal/config/command_executor.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
package config

import (
"io"
"runtime"
)

// Executor is a general execution interface for implicit commands.
type Executor interface {
Execute(args []string, root string) (string, error)
}
"github.com/evilmartians/lefthook/internal/system"
)

// commandExecutor implements execution of a skip checks passed in a `run` option.
type commandExecutor struct {
exec Executor
cmd system.Command
}

// cmd runs plain string command in a subshell returning the success of it.
func (c *commandExecutor) cmd(commandLine string) bool {
func (c *commandExecutor) execute(commandLine string) bool {
if commandLine == "" {
return false
}
Expand All @@ -27,7 +25,7 @@ func (c *commandExecutor) cmd(commandLine string) bool {
args = []string{"sh", "-c", commandLine}
}

_, err := c.exec.Execute(args, "")
err := c.cmd.Run(args, "", system.NullReader, io.Discard)

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

func (h *Hook) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(system.Executor{})
skipChecker := NewSkipChecker(system.Cmd)
return skipChecker.check(gitState, h.Skip, h.Only)
}

Expand Down
2 changes: 1 addition & 1 deletion internal/config/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type scriptRunnerReplace struct {
}

func (s Script) DoSkip(gitState git.State) bool {
skipChecker := NewSkipChecker(system.Executor{})
skipChecker := NewSkipChecker(system.Cmd)
return skipChecker.check(gitState, s.Skip, s.Only)
}

Expand Down
7 changes: 4 additions & 3 deletions internal/config/skip_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import (

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

type skipChecker struct {
exec *commandExecutor
}

func NewSkipChecker(executor Executor) *skipChecker {
return &skipChecker{&commandExecutor{executor}}
func NewSkipChecker(cmd system.Command) *skipChecker {
return &skipChecker{&commandExecutor{cmd}}
}

// check returns the result of applying a skip/only setting which can be a branch, git state, shell command, etc.
Expand Down Expand Up @@ -84,7 +85,7 @@ func (sc *skipChecker) matchesCommands(typedState map[string]interface{}) bool {
return false
}

result := sc.exec.cmd(commandLine)
result := sc.exec.execute(commandLine)

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

Expand Down
13 changes: 7 additions & 6 deletions internal/config/skip_checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,24 @@ package config

import (
"errors"
"io"
"testing"

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

type mockExecutor struct{}
type mockCmd struct{}

func (mc mockExecutor) Execute(args []string, _root string) (string, error) {
if len(args) == 3 && args[2] == "success" {
return "", nil
func (mc mockCmd) Run(cmd []string, _root string, _in io.Reader, _out io.Writer) error {
if len(cmd) == 3 && cmd[2] == "success" {
return nil
} else {
return "", errors.New("failure")
return errors.New("failure")
}
}

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

for _, tt := range [...]struct {
name string
Expand Down
32 changes: 19 additions & 13 deletions internal/git/command_executor.go
Original file line number Diff line number Diff line change
@@ -1,33 +1,29 @@
package git

import (
"bytes"
"fmt"
"path/filepath"
"strings"

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

// Executor is a general execution interface for implicit commands.
// Added here mostly for mockable tests.
type Executor interface {
Execute(args []string, root string) (string, error)
}

// CommandExecutor provides some methods that take some effect on execution and/or result data.
type CommandExecutor struct {
exec Executor
cmd system.Command
root string
}

// NewExecutor returns an object that executes given commands in the OS.
func NewExecutor(exec Executor) *CommandExecutor {
return &CommandExecutor{exec: exec}
func NewExecutor(cmd system.Command) *CommandExecutor {
return &CommandExecutor{cmd: cmd}
}

// Cmd runs plain string command. Trims spaces around output.
func (c CommandExecutor) Cmd(args []string) (string, error) {
out, err := c.exec.Execute(args, c.root)
func (c CommandExecutor) Cmd(cmd []string) (string, error) {
out, err := c.execute(cmd, c.root)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -55,7 +51,7 @@ func (c CommandExecutor) BatchedCmd(cmd []string, args []string) (string, error)

// CmdLines runs plain string command, returns its output split by newline.
func (c CommandExecutor) CmdLines(cmd []string) ([]string, error) {
out, err := c.exec.Execute(cmd, c.root)
out, err := c.execute(cmd, c.root)
if err != nil {
return nil, err
}
Expand All @@ -66,14 +62,24 @@ func (c CommandExecutor) CmdLines(cmd []string) ([]string, error) {
// CmdLines runs plain string command, returns its output split by newline.
func (c CommandExecutor) CmdLinesWithinFolder(cmd []string, folder string) ([]string, error) {
root := filepath.Join(c.root, folder)
out, err := c.exec.Execute(cmd, root)
out, err := c.execute(cmd, root)
if err != nil {
return nil, err
}

return strings.Split(strings.TrimSpace(out), "\n"), nil
}

func (c CommandExecutor) execute(cmd []string, root string) (string, error) {
out := bytes.NewBuffer(make([]byte, 0))
err := c.cmd.Run(cmd, root, system.NullReader, out)
strOut := out.String()

log.Debug("[lefthook] out: ", strOut)

return strOut, err
}

func batchByLength(s []string, length int) [][]string {
batches := make([][]string, 0)

Expand Down
16 changes: 11 additions & 5 deletions internal/git/repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,27 @@ package git
import (
"errors"
"fmt"
"io"
"strings"
"testing"
)

type GitMock struct {
type gitCmd struct {
cases map[string]string
}

func (g GitMock) Execute(cmd []string, _root string) (string, error) {
func (g gitCmd) Run(cmd []string, _root string, _in io.Reader, out io.Writer) error {
res, ok := g.cases[(strings.Join(cmd, " "))]
if !ok {
return "", errors.New("doesn't exist")
return errors.New("doesn't exist")
}

return strings.TrimSpace(res), nil
_, err := out.Write([]byte(strings.TrimSpace(res)))
if err != nil {
return err
}

return nil
}

func TestPartiallyStagedFiles(t *testing.T) {
Expand All @@ -37,7 +43,7 @@ MM staged but changed
t.Run(fmt.Sprintf("%d: %s", i, tt.name), func(t *testing.T) {
repository := &Repository{
Git: &CommandExecutor{
exec: GitMock{
cmd: gitCmd{
cases: map[string]string{
"git status --short --porcelain": tt.gitOut,
},
Expand Down
2 changes: 1 addition & 1 deletion internal/lefthook/lefthook.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func initialize(opts *Options) (*Lefthook, error) {

log.SetColors(!opts.NoColors)

repo, err := git.NewRepository(opts.Fs, git.NewExecutor(system.Executor{}))
repo, err := git.NewRepository(opts.Fs, git.NewExecutor(system.Cmd))
if err != nil {
return nil, err
}
Expand Down
9 changes: 5 additions & 4 deletions internal/lefthook/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package lefthook

import (
"fmt"
"io"
"path/filepath"
"slices"
"testing"
Expand All @@ -11,10 +12,10 @@ import (
"github.com/evilmartians/lefthook/internal/git"
)

type GitMock struct{}
type gitCmd struct{}

func (g GitMock) Execute(_cmd []string, root string) (string, error) {
return "", nil
func (g gitCmd) Run([]string, string, io.Reader, io.Writer) error {
return nil
}

func TestRun(t *testing.T) {
Expand Down Expand Up @@ -157,7 +158,7 @@ post-commit:
Options: &Options{Fs: fs},
repo: &git.Repository{
Fs: fs,
Git: git.NewExecutor(GitMock{}),
Git: git.NewExecutor(gitCmd{}),
HooksPath: hooksPath,
RootPath: root,
GitPath: gitPath,
Expand Down
10 changes: 0 additions & 10 deletions internal/lefthook/runner/exec/execute_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,6 @@ func (e CommandExecutor) Execute(ctx context.Context, opts Options, in io.Reader
return nil
}

func (e CommandExecutor) RawExecute(ctx context.Context, command []string, in io.Reader, out io.Writer) error {
cmd := exec.CommandContext(ctx, command[0], command[1:]...)

cmd.Stdin = in
cmd.Stdout = out
cmd.Stderr = os.Stderr

return cmd.Run()
}

func (e CommandExecutor) execute(ctx context.Context, cmdstr string, args *executeArgs) error {
command := exec.CommandContext(ctx, "sh", "-c", cmdstr)
command.Dir = args.root
Expand Down
10 changes: 0 additions & 10 deletions internal/lefthook/runner/exec/execute_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,6 @@ func (e CommandExecutor) Execute(ctx context.Context, opts Options, in io.Reader
return nil
}

func (e CommandExecutor) RawExecute(ctx context.Context, command []string, in io.Reader, out io.Writer) error {
cmd := exec.CommandContext(ctx, command[0], command[1:]...)

cmd.Stdin = in
cmd.Stdout = out
cmd.Stderr = os.Stderr

return cmd.Run()
}

func (e CommandExecutor) execute(cmdstr string, args *executeArgs) error {
cmdargs := strings.Split(cmdstr, " ")
command := exec.Command(cmdargs[0])
Expand Down
3 changes: 1 addition & 2 deletions internal/lefthook/runner/exec/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,5 @@ type Options struct {
// Executor provides an interface for command execution.
// It is used here for testing purpose mostly.
type Executor interface {
Execute(ctx context.Context, opts Options, in io.Reader, out io.Writer) error
RawExecute(ctx context.Context, command []string, in io.Reader, out io.Writer) error
Execute(context.Context, Options, io.Reader, io.Writer) error
}
8 changes: 6 additions & 2 deletions internal/lefthook/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/evilmartians/lefthook/internal/lefthook/runner/exec"
"github.com/evilmartians/lefthook/internal/lefthook/runner/filters"
"github.com/evilmartians/lefthook/internal/log"
"github.com/evilmartians/lefthook/internal/system"
)

const (
Expand Down Expand Up @@ -55,6 +56,7 @@ type Runner struct {
partiallyStagedFiles []string
failed atomic.Bool
executor exec.Executor
cmd system.CommandWithContext
}

func New(opts Options) *Runner {
Expand All @@ -65,6 +67,7 @@ func New(opts Options) *Runner {
// and scripts access the same Git data STDIN is cached via cachedReader.
stdin: NewCachedReader(os.Stdin),
executor: exec.CommandExecutor{},
cmd: system.Cmd,
}
}

Expand Down Expand Up @@ -143,12 +146,13 @@ func (r *Runner) runLFSHook(ctx context.Context) error {
"[git-lfs] executing hook: git lfs %s %s", r.HookName, strings.Join(r.GitArgs, " "),
)
out := bytes.NewBuffer(make([]byte, 0))
err := r.executor.RawExecute(
err := r.cmd.RunWithContext(
ctx,
append(
[]string{"git", "lfs", r.HookName},
r.GitArgs...,
),
"",
r.stdin,
out,
)
Expand Down Expand Up @@ -497,7 +501,7 @@ func (r *Runner) run(ctx context.Context, opts exec.Options, follow bool) bool {
defer log.UnsetName(opts.Name)

// If the command does not explicitly `use_stdin` no input will be provided.
var in io.Reader = NewNullReader()
var in io.Reader = system.NullReader
if opts.UseStdin {
in = r.stdin
}
Expand Down
Loading
Loading