Included tests for internal/signalio/csv

- Included tests for the functions `maybeWriteHeader()` and `marshalValue` in `internal/signalio/csv`.
- Included additional comments for `maybeWriteHeader()`.

Signed-off-by: nathannaveen <[email protected]>

Author: nathannaveen

internal/signalio/csv.go

@@ -50,13 +50,40 @@ func (w *csvWriter) WriteSignals(signals []signal.Set, extra ...Field) error {
 }
 
 func (w *csvWriter) maybeWriteHeader() error {
-	// Check headerWritten without the lock to avoid holding the lock if the
-	// header has already been written.
+	/*
+		The variable w.headerWritten is checked twice to avoid what is known as a "race condition".
+		A race condition can occur when two or more goroutines try to access a shared resource
+		(in this case, the csvWriter instance) concurrently, and the outcome of the program depends on
+		the interleaving of their execution.
+
+		Imagine the following scenario:
+
+		1. Goroutine A reads the value of w.headerWritten as false.
+		2. Goroutine B reads the value of w.headerWritten as false.
+		3. Goroutine A acquires the mutex lock and sets w.headerWritten to true.
+		4. Goroutine B acquires the mutex lock and sets w.headerWritten to true.
+
+		If this happens, the header will be written twice, which is not the desired behavior.
+		By checking w.headerWritten twice, once before acquiring the mutex lock and once after acquiring the lock,
+		the function can ensure that only one goroutine enters the critical section and writes the header.
+
+		Here's how the function works:
+
+		1. Goroutine A reads the value of w.headerWritten as false.
+		2. Goroutine A acquires the mutex lock.
+		3. Goroutine A re-checks the value of w.headerWritten and finds it to be false.
+		4. Goroutine A sets w.headerWritten to true and writes the header.
+		5. Goroutine A releases the mutex lock.
+
+		If Goroutine B tries to enter the critical section at any point after step 2,
+		it will have to wait until Goroutine A releases the lock in step 5. Once the lock is released,
+		Goroutine B will re-check the value of w.headerWritten and find it to be true,
+		so it will not write the header again.
+	*/
+
 	if w.headerWritten {
 		return nil
 	}
-	// Grab the lock and re-check headerWritten just in case another goroutine
-	// entered the same critical section. Also prevent concurrent writes to w.
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	if w.headerWritten {

internal/signalio/csv_test.go

@@ -0,0 +1,86 @@
+package signalio
+
+import (
+	"encoding/csv"
+	"sync"
+	"testing"
+	"time"
+)
+
+func TestMarshalValue(t *testing.T) {
+	tests := []struct {
+		value    any
+		expected string
+		wantErr  bool
+	}{
+		{value: true, expected: "true", wantErr: false},
+		{value: 1, expected: "1", wantErr: false},
+		{value: int16(2), expected: "2", wantErr: false},
+		{value: int32(3), expected: "3", wantErr: false},
+		{value: int64(4), expected: "4", wantErr: false},
+		{value: uint(5), expected: "5", wantErr: false},
+		{value: uint16(6), expected: "6", wantErr: false},
+		{value: uint32(7), expected: "7", wantErr: false},
+		{value: uint64(8), expected: "8", wantErr: false},
+		{value: byte(9), expected: "9", wantErr: false},
+		{value: float32(10.1), expected: "10.1", wantErr: false},
+		{value: 11.1, expected: "11.1", wantErr: false}, // float64
+		{value: "test", expected: "test", wantErr: false},
+		{value: time.Now(), expected: time.Now().Format(time.RFC3339), wantErr: false},
+		{value: nil, expected: "", wantErr: false},
+		{value: []int{1, 2, 3}, expected: "", wantErr: true},
+		{value: map[string]string{"key": "value"}, expected: "", wantErr: true},
+		{value: struct{}{}, expected: "", wantErr: true},
+	}
+	for _, test := range tests {
+		res, err := marshalValue(test.value)
+		if (err != nil) != test.wantErr {
+			t.Errorf("Unexpected error for value %v: wantErr %v, got %v", test.value, test.wantErr, err)
+		}
+		if res != test.expected {
+			t.Errorf("Unexpected result for value %v: expected %v, got %v", test.value, test.expected, res)
+		}
+	}
+}
+
+func Test_csvWriter_maybeWriteHeader(t *testing.T) {
+	type fields struct {
+		w             *csv.Writer
+		header        []string
+		headerWritten bool
+	}
+	tests := []struct {
+		name   string
+		fields fields
+	}{
+		{
+			name: "write header",
+			fields: fields{
+				w:             csv.NewWriter(nil),
+				header:        []string{},
+				headerWritten: false,
+			},
+		},
+		{
+			name: "header already written",
+			fields: fields{
+				w:             csv.NewWriter(nil),
+				header:        []string{"a", "b", "c"},
+				headerWritten: true,
+			},
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			w := &csvWriter{
+				w:             test.fields.w,
+				header:        test.fields.header,
+				headerWritten: test.fields.headerWritten,
+				mu:            sync.Mutex{},
+			}
+			if err := w.maybeWriteHeader(); err != nil { // never want an error
+				t.Errorf("maybeWriteHeader() error = %v", err)
+			}
+		})
+	}
+}