chore: various small tests & optimizations

Signed-off-by: Jakob Möller <jmoller@redhat.com>
jakobmoellerdev · Jul 29, 2024 · 8298e9e · 8298e9e
1 parent ba4f9ac
commit 8298e9e
Show file tree

Hide file tree

Showing 14 changed files with 86 additions and 38 deletions.
diff --git a/client_test.go b/client_test.go
@@ -10,7 +10,7 @@ import (
 )
 
 func TestLVs(t *testing.T) {
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 	ctx := WithCustomEnvironment(context.Background(), map[string]string{})
 	slog.SetDefault(slog.New(NewContextPropagatingSlogHandler(NewTestingHandler(t))))
 	slog.SetLogLoggerLevel(slog.LevelDebug)
@@ -63,7 +63,7 @@ func TestLVs(t *testing.T) {
 		},
 	} {
 		t.Run(fmt.Sprintf("[%v]%s", i, tc.String()), func(t *testing.T) {
-			FailTestIfNotRoot(t)
+			SkipOrFailTestIfNotRoot(t)
 			ctx, cancel := context.WithCancel(ctx)
 			defer cancel()
 			clnt := GetTestClient(ctx)

diff --git a/config_test.go b/config_test.go
@@ -10,7 +10,7 @@ import (
 
 func Test_RawConfig(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 	slog.SetDefault(slog.New(NewContextPropagatingSlogHandler(NewTestingHandler(t))))
 	slog.SetLogLoggerLevel(slog.LevelDebug)
 	ctx := context.Background()

diff --git a/lvextend_test.go b/lvextend_test.go
@@ -9,7 +9,7 @@ import (
 
 func TestLVExtend(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	clnt := NewClient()
 	ctx := context.Background()

diff --git a/lvmdevices_test.go b/lvmdevices_test.go
@@ -13,7 +13,7 @@ import (
 
 func TestLVMDevices(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	_, err := exec.LookPath("lvmdevices")
 	if err != nil {

diff --git a/lvrename_test.go b/lvrename_test.go
@@ -10,7 +10,7 @@ import (
 
 func TestLVRename(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	clnt := NewClient()
 	ctx := context.Background()

diff --git a/run_lvm.go b/run_lvm.go
@@ -21,8 +21,6 @@ func (c *client) RunLVM(ctx context.Context, args ...string) error {
 func (c *client) RunLVMInto(ctx context.Context, into any, args ...string) error {
 	cmd := CommandContext(ctx, GetLVMPath(), args...)
 
-	slog.DebugContext(ctx, "running command", slog.String("command", strings.Join(cmd.Args, " ")))
-
 	output, err := StreamedCommand(ctx, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to execute command: %v", err)
@@ -76,8 +74,6 @@ func (c *client) RunRaw(ctx context.Context, process RawOutputProcessor, args ..
 	}
 	cmd := CommandContext(ctx, args[0], args[1:]...)
 
-	slog.DebugContext(ctx, "running command", slog.String("command", strings.Join(cmd.Args, " ")))
-
 	output, err := StreamedCommand(ctx, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to execute command: %v", err)

diff --git a/slog_handler.go b/slog_handler.go
@@ -40,7 +40,7 @@ func copySyncMap(m *sync.Map) *sync.Map {
 	return &cp
 }
 
-var _ slog.Handler = SlogHandler{}
+var _ slog.Handler = &ContextPropagatingSlogHandler{}
 
 type TestingHandler struct {
 	tb         testing.TB
@@ -57,7 +57,7 @@ func NewTestingHandler(tb testing.TB) slog.Handler {
 	}
 }
 
-func (h TestingHandler) Enabled(_ context.Context, l slog.Level) bool {
+func (h TestingHandler) Enabled(_ context.Context, _ slog.Level) bool {
 	return !h.tb.Skipped()
 }
 
@@ -112,21 +112,23 @@ func (h TestingHandler) printStack() bool {
 	return h.tb.Failed()
 }
 
-type SlogHandler struct {
+type ContextPropagatingSlogHandler struct {
 	handler slog.Handler
 }
 
+// NewContextPropagatingSlogHandler returns a new slog.Handler that propagates context values as slog attributes.
+// The handler is a wrapper around the provided handler.
 func NewContextPropagatingSlogHandler(handler slog.Handler) slog.Handler {
-	return SlogHandler{
+	return &ContextPropagatingSlogHandler{
 		handler: handler,
 	}
 }
 
-func (h SlogHandler) Enabled(ctx context.Context, level slog.Level) bool {
+func (h *ContextPropagatingSlogHandler) Enabled(ctx context.Context, level slog.Level) bool {
 	return h.handler.Enabled(ctx, level)
 }
 
-func (h SlogHandler) Handle(ctx context.Context, record slog.Record) error {
+func (h *ContextPropagatingSlogHandler) Handle(ctx context.Context, record slog.Record) error {
 	if v, ok := ctx.Value(fields).(*sync.Map); ok {
 		v.Range(func(key, val any) bool {
 			if keyString, ok := key.(string); ok {
@@ -142,10 +144,10 @@ func (h SlogHandler) Handle(ctx context.Context, record slog.Record) error {
 	return h.handler.Handle(ctx, record)
 }
 
-func (h SlogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
-	return SlogHandler{h.handler.WithAttrs(attrs)}
+func (h *ContextPropagatingSlogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
+	return &ContextPropagatingSlogHandler{h.handler.WithAttrs(attrs)}
 }
 
-func (h SlogHandler) WithGroup(name string) slog.Handler {
-	return h.handler.WithGroup(name)
+func (h *ContextPropagatingSlogHandler) WithGroup(name string) slog.Handler {
+	return &ContextPropagatingSlogHandler{h.handler.WithGroup(name)}
 }
diff --git a/slog_handler_test.go b/slog_handler_test.go
@@ -0,0 +1,57 @@
+package lvm2go_test
+
+import (
+	"bufio"
+	"bytes"
+	"context"
+	"encoding/json"
+	"log/slog"
+	"testing"
+
+	. "github.com/jakobmoellerdev/lvm2go"
+)
+
+func TestNewContextPropagatingSlogHandler(t *testing.T) {
+	SkipOrFailTestIfNotRoot(t)
+	stdout := &bytes.Buffer{}
+	var loggingHandler slog.Handler
+	loggingHandler = slog.NewJSONHandler(stdout, &slog.HandlerOptions{
+		Level:     slog.LevelDebug,
+		AddSource: true,
+	})
+	loggingHandler = NewContextPropagatingSlogHandler(loggingHandler)
+	slog.SetDefault(slog.New(loggingHandler))
+	ctx := context.Background()
+
+	lvm := NewClient()
+	if _, err := lvm.Version(ctx); err != nil {
+		t.Errorf("Error: %v", err)
+	}
+
+	lineReader := bufio.NewScanner(stdout)
+	var lines []map[string]any
+	for lineReader.Scan() {
+		line := make(map[string]any)
+		if err := json.NewDecoder(bytes.NewReader(lineReader.Bytes())).Decode(&line); err != nil {
+			t.Errorf("Error: %v", err)
+		}
+		lines = append(lines, line)
+	}
+	if err := lineReader.Err(); err != nil {
+		t.Errorf("Error: %v", err)
+	}
+	if len(lines) == 0 {
+		t.Errorf("Expected output in logger, got nothing")
+	}
+
+	foundcommand := false
+	for _, line := range lines {
+		if line["command"] != nil && line["msg"] != nil {
+			foundcommand = true
+			break
+		}
+	}
+	if !foundcommand {
+		t.Errorf("Expected command in logger output, got nothing")
+	}
+}
diff --git a/streamed_command.go b/streamed_command.go
@@ -2,9 +2,11 @@ package lvm2go
 
 import (
 	"context"
+	"errors"
 	"io"
 	"log/slog"
 	"os/exec"
+	"strings"
 )
 
 // StreamedCommand runs the command and returns the stdout as a ReadCloser that also Waits for the command to finish.
@@ -17,27 +19,18 @@ func StreamedCommand(ctx context.Context, cmd *exec.Cmd) (io.ReadCloser, error)
 	}
 	stderr, err := cmd.StderrPipe()
 	if err != nil {
-		_ = stdout.Close()
-		return nil, err
+		return nil, errors.Join(err, stdout.Close())
 	}
 
-	slog.DebugContext(ctx, "invoking command", "args", cmd.Args, "env", cmd.Env, "pwd", cmd.Dir)
+	slog.DebugContext(ctx, "running command", slog.String("command", strings.Join(cmd.Args, " ")))
 
 	cmd.Cancel = func() error {
 		slog.WarnContext(ctx, "killing streamed command process due to ctx cancel")
-		if err := stdout.Close(); err != nil {
-			return err
-		}
-		if err := stderr.Close(); err != nil {
-			return err
-		}
-		return cmd.Process.Kill()
+		return errors.Join(cmd.Process.Kill(), stdout.Close(), stderr.Close())
 	}
 
 	if err := cmd.Start(); err != nil {
-		_ = stdout.Close()
-		_ = stderr.Close()
-		return nil, err
+		return nil, errors.Join(err, stdout.Close(), stderr.Close())
 	}
 	// Return a read closer that will wait for the command to finish when closed to release all resources.
 	return commandReadCloser{cmd: cmd, ReadCloser: stdout, stderr: stderr}, nil

diff --git a/util_test.go b/util_test.go
@@ -30,7 +30,7 @@ var sharedTestClientKey = struct{}{}
 
 var skipRootfulTests = flag.Bool("skip-rootful-tests", false, "Name of location to greet")
 
-func FailTestIfNotRoot(t *testing.T) {
+func SkipOrFailTestIfNotRoot(t *testing.T) {
 	if os.Geteuid() != 0 {
 		if *skipRootfulTests {
 			t.Skip("Skipping test because it requires root privileges to setup its environment.")

diff --git a/version_test.go b/version_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func Test_Version(t *testing.T) {
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 	slog.SetDefault(slog.New(NewContextPropagatingSlogHandler(NewTestingHandler(t))))
 	slog.SetLogLoggerLevel(slog.LevelDebug)
 	ctx := context.Background()

diff --git a/vgchange_test.go b/vgchange_test.go
@@ -9,7 +9,7 @@ import (
 )
 
 func TestVGChange(t *testing.T) {
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	clnt := NewClient()
 	ctx := context.Background()

diff --git a/vgextend_test.go b/vgextend_test.go
@@ -9,7 +9,7 @@ import (
 
 func TestVGExtend(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	clnt := NewClient()
 	ctx := context.Background()

diff --git a/vgrename_test.go b/vgrename_test.go
@@ -9,7 +9,7 @@ import (
 
 func TestVGRename(t *testing.T) {
 	t.Parallel()
-	FailTestIfNotRoot(t)
+	SkipOrFailTestIfNotRoot(t)
 
 	clnt := NewClient()
 	ctx := context.Background()