From 0fb9e401912f89d25ff96b0dd36aabf43b4fab8f Mon Sep 17 00:00:00 2001
From: Ophestra Umiker <cat@ophivana.moe>
Date: Mon, 7 Oct 2024 13:33:18 +0900
Subject: [PATCH] helper/args: MustNewCheckedArgs for cleaner hardcoded args

Signed-off-by: Ophestra Umiker <cat@ophivana.moe>
---
 helper/args.go        | 11 +++++++++++
 helper/args_test.go   | 15 +++++++++++++--
 helper/helper_test.go | 18 ++----------------
 helper/pipe_test.go   | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+), 18 deletions(-)
 create mode 100644 helper/pipe_test.go

diff --git a/helper/args.go b/helper/args.go
index c841fd3..f85d57e 100644
--- a/helper/args.go
+++ b/helper/args.go
@@ -53,3 +53,14 @@ func NewCheckedArgs(args []string) (io.WriterTo, error) {
 	a := argsWt(args)
 	return a, a.check()
 }
+
+// MustNewCheckedArgs returns a checked argument writer for args and panics if check fails.
+// Callers must not retain any references to args.
+func MustNewCheckedArgs(args []string) io.WriterTo {
+	a, err := NewCheckedArgs(args)
+	if err != nil {
+		panic(err.Error())
+	}
+
+	return a
+}
diff --git a/helper/args_test.go b/helper/args_test.go
index 378c6dc..5675a87 100644
--- a/helper/args_test.go
+++ b/helper/args_test.go
@@ -10,8 +10,6 @@ import (
 )
 
 func Test_argsFD_String(t *testing.T) {
-	argsOnce.Do(prepareArgs)
-
 	wantString := strings.Join(want, " ")
 	if got := argsWt.(fmt.Stringer).String(); got != wantString {
 		t.Errorf("String(): got %v; want %v",
@@ -26,4 +24,17 @@ func TestNewCheckedArgs(t *testing.T) {
 			args,
 			err, helper.ErrContainsNull)
 	}
+
+	t.Run("must panic", func(t *testing.T) {
+		badPayload := []string{"\x00"}
+		defer func() {
+			wantPanic := "argument contains null character"
+			if r := recover(); r != wantPanic {
+				t.Errorf("MustNewCheckedArgs(%q) panic = %v, wantPanic %v",
+					badPayload,
+					r, wantPanic)
+			}
+		}()
+		helper.MustNewCheckedArgs(badPayload)
+	})
 }
diff --git a/helper/helper_test.go b/helper/helper_test.go
index 835f358..0bbc569 100644
--- a/helper/helper_test.go
+++ b/helper/helper_test.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
@@ -23,20 +22,9 @@ var (
 		"--talk=org.freedesktop.UPower",
 	}
 
-	wantPayload string
-	argsWt      io.WriterTo
-	argsOnce    sync.Once
-)
-
-func prepareArgs() {
 	wantPayload = strings.Join(want, "\x00") + "\x00"
-
-	if a, err := helper.NewCheckedArgs(want); err != nil {
-		panic(err.Error())
-	} else {
-		argsWt = a
-	}
-}
+	argsWt      = helper.MustNewCheckedArgs(want)
+)
 
 func argF(argsFD int, _ int) []string {
 	return []string{"--args", strconv.Itoa(argsFD)}
@@ -48,7 +36,6 @@ func argFStatus(argsFD int, statFD int) []string {
 
 func TestHelper_StartNotify_Close_Wait(t *testing.T) {
 	helper.InternalReplaceExecCommand(t)
-	argsOnce.Do(prepareArgs)
 
 	t.Run("start non-existent helper path", func(t *testing.T) {
 		h := helper.New(argsWt, "/nonexistent", argF)
@@ -143,7 +130,6 @@ func TestHelper_StartNotify_Close_Wait(t *testing.T) {
 }
 func TestHelper_Start_Close_Wait(t *testing.T) {
 	helper.InternalReplaceExecCommand(t)
-	argsOnce.Do(prepareArgs)
 
 	var wt io.WriterTo
 	if a, err := helper.NewCheckedArgs(want); err != nil {
diff --git a/helper/pipe_test.go b/helper/pipe_test.go
new file mode 100644
index 0000000..e9c8d06
--- /dev/null
+++ b/helper/pipe_test.go
@@ -0,0 +1,42 @@
+package helper
+
+import (
+	"testing"
+)
+
+func Test_pipes_pipe_mustClosePipes(t *testing.T) {
+	p := new(pipes)
+
+	t.Run("pipe without args", func(t *testing.T) {
+		defer func() {
+			wantPanic := "attempted to pipe without args"
+			if r := recover(); r != wantPanic {
+				t.Errorf("pipe() panic = %v, wantPanic %v",
+					r, wantPanic)
+			}
+		}()
+		_ = p.pipe()
+	})
+
+	p.args = MustNewCheckedArgs(make([]string, 0))
+	t.Run("obtain pipes", func(t *testing.T) {
+		if err := p.pipe(); err != nil {
+			t.Errorf("pipe() error = %v",
+				err)
+			return
+		}
+	})
+
+	t.Run("pipe twice", func(t *testing.T) {
+		defer func() {
+			wantPanic := "attempted to pipe twice"
+			if r := recover(); r != wantPanic {
+				t.Errorf("pipe() panic = %v, wantPanic %v",
+					r, wantPanic)
+			}
+		}()
+		_ = p.pipe()
+	})
+
+	p.mustClosePipes()
+}