Skip to content

Commit

Permalink
Included tests for internal/signalio/csv
Browse files Browse the repository at this point in the history
- Included tests for the functions `maybeWriteHeader()` and `marshalValue` in `internal/signalio/csv`.
- Included additional comments for `maybeWriteHeader()`.

Signed-off-by: nathannaveen <[email protected]>
  • Loading branch information
nathannaveen committed Jan 8, 2023
1 parent 27ad7d2 commit f81b586
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 4 deletions.
35 changes: 31 additions & 4 deletions internal/signalio/csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
89 changes: 89 additions & 0 deletions internal/signalio/csv_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
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
mu sync.Mutex
}
tests := []struct {
name string
fields fields
}{
{
name: "write header",
fields: fields{
w: csv.NewWriter(nil),
header: []string{},
headerWritten: false,
mu: sync.Mutex{},
},
},
{
name: "header already written",
fields: fields{
w: csv.NewWriter(nil),
header: []string{"a", "b", "c"},
headerWritten: true,
mu: sync.Mutex{},
},
},
}
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: test.fields.mu,
}
if err := w.maybeWriteHeader(); err != nil { // never want an error
t.Errorf("maybeWriteHeader() error = %v", err)
}
})
}
}

0 comments on commit f81b586

Please sign in to comment.