Merge pull request #32 from AlphaOne1/redaction

Improve error handling, add nil writer check, and enhance test coverage.

Author: AlphaOne1

CHANGELOG.md

@@ -1,3 +1,12 @@
+Release 0.1.2
+=============
+
+- introduced ErrWriterNil to catch nil writers
+- better parameter validation
+- better error messages
+- improved tests with configurable broken IO
+- added parallel annotation
+
 Release 0.1.1
 =============
 

geany.go

@@ -14,7 +14,10 @@ import (
 	"text/template"
 )
 
-// geanyData contains the data that geany provies on itself.
+// ErrWriterNil indicates that the provided io.Writer is nil and cannot be used.
+var ErrWriterNil = errors.New("writer is nil")
+
+// geanyData contains the data that geany provides on itself.
 type geanyData struct {
 	VcsRevision string
 	VcsTime     string
@@ -24,12 +27,15 @@ type geanyData struct {
 
 // logoData is the structure given as data to the templating engine.
 // It contains the geany provided build information and the user provided data.
+// The user is not required to pass data, in that case nil should be passed. In any case,
+// the data accessed in the logo template and provided data must match.
 type logoData struct {
-	Geany  geanyData
-	Values any
+	Geany  geanyData // build information
+	Values any       // user provided data
 }
 
-// getBuildInfo is a global variable to mock it easily in tests.
+// getBuildInfo is a global variable to mock it easily in tests. Take into account that
+// this variable can produce race conditions in parallel testing.
 var getBuildInfo = debug.ReadBuildInfo //nolint:gochecknoglobals
 
 // prepareLogoData collects the build information and the user-provided data
@@ -45,7 +51,7 @@ func prepareLogoData(values any) logoData {
 		Values: values,
 	}
 
-	if buildInfo, ok := getBuildInfo(); ok {
+	if buildInfo, ok := getBuildInfo(); ok && buildInfo != nil {
 		result.Geany.GoVersion = buildInfo.GoVersion
 
 		for _, s := range buildInfo.Settings {
@@ -68,25 +74,29 @@ func prepareLogoData(values any) logoData {
 // PrintSimpleWriter outputs just the name of the program, the build information
 // and, in case, the user given data to a user provided io.Writer.
 func PrintSimpleWriter(writer io.Writer, values any) error {
+	if writer == nil {
+		return ErrWriterNil
+	}
+
 	revData := prepareLogoData(values)
 
 	// normally we have the program's name given as the first argument
-	if len(os.Args) > 0 {
-		fmt.Printf("%s\n", os.Args[0])
+	if len(os.Args) > 0 && os.Args[0] != "" {
+		if _, err := fmt.Fprintf(writer, "%s\n", os.Args[0]); err != nil {
+			return fmt.Errorf("could not write program name: %w", err)
+		}
 	}
 
 	encoder := json.NewEncoder(writer)
 	encoder.SetIndent("", "    ")
 
 	// we suppress the linter here, as we cannot guarantee for users data.
-	err := encoder.Encode(revData) //nolint:musttag
-
-	if err == nil {
-		_, err = fmt.Fprintln(writer)
+	if err := encoder.Encode(revData); err != nil { //nolint:musttag
+		return fmt.Errorf("could not encode user data: %w", err)
 	}
 
-	if err != nil {
-		return fmt.Errorf("could not write: %w", err)
+	if _, err := fmt.Fprintln(writer); err != nil {
+		return fmt.Errorf("could not write final newline: %w", err)
 	}
 
 	return nil
@@ -105,23 +115,32 @@ func PrintSimple(values any) error {
 //   - VcsModified
 //   - GoVersion
 //
-// these can be referenced in the template, e.g., using {{ .VcsRevision }}.
+// these can be referenced in the template, e.g., using `{{ .Geany.VcsRevision }}`.
 // An additional custom value can be accessed via the Values field. Its type must match the way
-// that it is accessed in the logo.
+// that it is accessed in the logo and is accessed using e.g. `{{ .Values.Foo }}`.
+//
+// The template is parsed and executed. In case of an error, the program's name and user given
+// data are printed as JSON as fallback. The original error and, in case, the error of the fallback
+// are returned.
 func PrintLogoWriter(writer io.Writer, tmpl string, values any) error {
+	if writer == nil {
+		return ErrWriterNil
+	}
+
 	revData := prepareLogoData(values)
 
 	logo := template.New("logo")
-	template.Must(logo.Parse(tmpl))
+
+	if _, err := logo.Parse(tmpl); err != nil {
+		return fmt.Errorf("could not parse template: %w", err)
+	}
 
 	if err := logo.Execute(writer, revData); err != nil {
 		return errors.Join(err, PrintSimpleWriter(writer, values))
 	}
 
-	_, err := fmt.Fprintln(writer)
-
-	if err != nil {
-		return fmt.Errorf("could not write: %w", err)
+	if _, err := fmt.Fprintln(writer); err != nil {
+		return fmt.Errorf("could not write final newline: %w", err)
 	}
 
 	return nil

geany_internal_test.go

@@ -20,6 +20,8 @@ func Must[T any](t T, err error) T {
 }
 
 func TestPrepareLogoDataNormal(t *testing.T) {
+	t.Parallel()
+
 	logoData := prepareLogoData(nil)
 
 	buildInfo, ok := debug.ReadBuildInfo()
@@ -34,6 +36,8 @@ func TestPrepareLogoDataNormal(t *testing.T) {
 }
 
 func TestPrepareLogoDataMocked(t *testing.T) {
+	t.Parallel()
+
 	old := getBuildInfo
 	getBuildInfo = func() (*debug.BuildInfo, bool) {
 		result := debug.BuildInfo{

geany_test.go

@@ -104,6 +104,8 @@ func (b *BrokenNIO) Write(in []byte) (n int, err error) {
 }
 
 func TestBrokenLogoWriter(t *testing.T) {
+	t.Parallel()
+
 	target := BrokenNIO{}
 
 	err := geany.PrintLogoWriter(
@@ -112,10 +114,15 @@ func TestBrokenLogoWriter(t *testing.T) {
 		nil)
 
 	require.Error(t, err, "simple writer printing error")
-	require.Equal(t, "broken writer\ncould not write: broken writer", err.Error(), "not two broken writer errors")
+	require.Equal(t,
+		"broken writer\ncould not write program name: broken writer",
+		err.Error(),
+		"not two broken writer errors")
 }
 
 func TestBrokenLogoWriterFallback(t *testing.T) {
+	t.Parallel()
+
 	target := BrokenNIO{To: 1}
 
 	err := geany.PrintLogoWriter(
@@ -128,6 +135,8 @@ func TestBrokenLogoWriterFallback(t *testing.T) {
 }
 
 func TestBrokenLogoWriterAtEnd(t *testing.T) {
+	t.Parallel()
+
 	target := BrokenNIO{From: 2}
 
 	err := geany.PrintLogoWriter(
@@ -136,10 +145,12 @@ func TestBrokenLogoWriterAtEnd(t *testing.T) {
 		nil)
 
 	require.Error(t, err, "simple writer printing error")
-	assert.Equal(t, "could not write: broken writer", err.Error(), "just one broken writer error")
+	assert.Equal(t, "could not write final newline: broken writer", err.Error(), "just one broken writer error")
 }
 
 func TestBrokenSimpleWriter(t *testing.T) {
+	t.Parallel()
+
 	target := BrokenNIO{}
 
 	require.Error(t,
@@ -149,10 +160,48 @@ func TestBrokenSimpleWriter(t *testing.T) {
 		"simple writer no printing error")
 }
 
+func TestBrokenSimpleWriterUserData(t *testing.T) {
+	t.Parallel()
+
+	target := BrokenNIO{From: 1}
+
+	require.Error(t,
+		geany.PrintSimpleWriter(
+			&target,
+			nil),
+		"simple writer no printing error")
+}
+
+func TestBrokenSimpleWriterFinal(t *testing.T) {
+	t.Parallel()
+
+	target := BrokenNIO{From: 2}
+
+	require.Error(t,
+		geany.PrintSimpleWriter(
+			&target,
+			nil),
+		"simple writer no printing error")
+}
+
 func TestBrokenLogo(t *testing.T) {
-	assert.Panics(t,
-		func() {
-			_ = geany.PrintLogo("{{ .Geany }", nil)
-		},
-		"broken logo does not panic")
+	t.Parallel()
+
+	require.Error(t,
+		geany.PrintLogo("{{ .Geany }", nil),
+		"broken logo does produce error")
+}
+
+func TestLogoWriterNil(t *testing.T) {
+	t.Parallel()
+
+	require.ErrorIs(t,
+		geany.PrintLogoWriter(nil, "", nil),
+		geany.ErrWriterNil,
+		"nil writer does not produce error")
+
+	require.ErrorIs(t,
+		geany.PrintSimpleWriter(nil, nil),
+		geany.ErrWriterNil,
+		"nil writer does not produce error")
 }