From 9e82e37ab8823a2c68b1faf684233f6470a8cfd4 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 7 Sep 2021 22:41:34 +0200 Subject: [PATCH] refactor(netxlite/iox): group tests and avoid races (#484) Part of https://github.com/ooni/probe/issues/1591 --- internal/netxlite/iox/iox_test.go | 284 ++++++++++++++++++------------ 1 file changed, 168 insertions(+), 116 deletions(-) diff --git a/internal/netxlite/iox/iox_test.go b/internal/netxlite/iox/iox_test.go index 5418fc9..eae09f0 100644 --- a/internal/netxlite/iox/iox_test.go +++ b/internal/netxlite/iox/iox_test.go @@ -5,130 +5,182 @@ import ( "errors" "io" "strings" + "sync" "testing" - "time" "github.com/ooni/probe-cli/v3/internal/netxlite/mocks" ) -func TestReadAllContextCommonCase(t *testing.T) { - r := strings.NewReader("deadbeef") - ctx := context.Background() - out, err := ReadAllContext(ctx, r) - if err != nil { - t.Fatal(err) - } - if len(out) != 8 { - t.Fatal("not the expected number of bytes") - } +func TestReadAllContext(t *testing.T) { + t.Run("with success and background context", func(t *testing.T) { + r := strings.NewReader("deadbeef") + ctx := context.Background() + out, err := ReadAllContext(ctx, r) + if err != nil { + t.Fatal(err) + } + if len(out) != 8 { + t.Fatal("not the expected number of bytes") + } + }) + + t.Run("with failure and background context", func(t *testing.T) { + expected := errors.New("mocked error") + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + return 0, expected + }, + } + ctx := context.Background() + out, err := ReadAllContext(ctx, r) + if !errors.Is(err, expected) { + t.Fatal("not the error we expected", err) + } + if len(out) != 0 { + t.Fatal("not the expected number of bytes") + } + }) + + t.Run("with success and cancelled context", func(t *testing.T) { + wg := &sync.WaitGroup{} + wg.Add(1) + sigch := make(chan interface{}) + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + defer wg.Done() + <-sigch + // "When Read encounters an error or end-of-file condition + // after successfully reading n > 0 bytes, it returns + // the number of bytes read. It may return the (non-nil) + // error from the same call or return the error (and n == 0) + // from a subsequent call."" + // + // See https://pkg.go.dev/io#Reader + return len(b), io.EOF + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // fail immediately + out, err := ReadAllContext(ctx, r) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if len(out) != 0 { + t.Fatal("not the expected number of bytes") + } + close(sigch) + wg.Wait() + }) + + t.Run("with failure and cancelled context", func(t *testing.T) { + wg := &sync.WaitGroup{} + wg.Add(1) + sigch := make(chan interface{}) + expected := errors.New("mocked error") + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + defer wg.Done() + <-sigch + return 0, expected + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // fail immediately + out, err := ReadAllContext(ctx, r) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if len(out) != 0 { + t.Fatal("not the expected number of bytes") + } + close(sigch) + wg.Wait() + }) } -func TestReadAllContextWithError(t *testing.T) { - expected := errors.New("mocked error") - r := &mocks.Reader{ - MockRead: func(b []byte) (int, error) { - return 0, expected - }, - } - ctx := context.Background() - out, err := ReadAllContext(ctx, r) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected", err) - } - if len(out) != 0 { - t.Fatal("not the expected number of bytes") - } -} +func TestCopyContext(t *testing.T) { + t.Run("with success and background context", func(t *testing.T) { + r := strings.NewReader("deadbeef") + ctx := context.Background() + out, err := CopyContext(ctx, io.Discard, r) + if err != nil { + t.Fatal(err) + } + if out != 8 { + t.Fatal("not the expected number of bytes") + } + }) -func TestReadAllContextWithCancelledContext(t *testing.T) { - r := strings.NewReader("deadbeef") - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - out, err := ReadAllContext(ctx, r) - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected", err) - } - if len(out) != 0 { - t.Fatal("not the expected number of bytes") - } -} + t.Run("with failure and background context", func(t *testing.T) { + expected := errors.New("mocked error") + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + return 0, expected + }, + } + ctx := context.Background() + out, err := CopyContext(ctx, io.Discard, r) + if !errors.Is(err, expected) { + t.Fatal("not the error we expected", err) + } + if out != 0 { + t.Fatal("not the expected number of bytes") + } + }) -func TestReadAllContextWithErrorAndCancelledContext(t *testing.T) { - expected := errors.New("mocked error") - r := &mocks.Reader{ - MockRead: func(b []byte) (int, error) { - time.Sleep(time.Millisecond) - return 0, expected - }, - } - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - out, err := ReadAllContext(ctx, r) - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected", err) - } - if len(out) != 0 { - t.Fatal("not the expected number of bytes") - } -} + t.Run("with success and cancelled context", func(t *testing.T) { + wg := &sync.WaitGroup{} + wg.Add(1) + sigch := make(chan interface{}) + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + defer wg.Done() + <-sigch + // "When Read encounters an error or end-of-file condition + // after successfully reading n > 0 bytes, it returns + // the number of bytes read. It may return the (non-nil) + // error from the same call or return the error (and n == 0) + // from a subsequent call."" + // + // See https://pkg.go.dev/io#Reader + return len(b), io.EOF + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // fail immediately + out, err := CopyContext(ctx, io.Discard, r) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if out != 0 { + t.Fatal("not the expected number of bytes") + } + close(sigch) + wg.Wait() + }) -func TestCopyContextCommonCase(t *testing.T) { - r := strings.NewReader("deadbeef") - ctx := context.Background() - out, err := CopyContext(ctx, io.Discard, r) - if err != nil { - t.Fatal(err) - } - if out != 8 { - t.Fatal("not the expected number of bytes") - } -} - -func TestCopyContextWithError(t *testing.T) { - expected := errors.New("mocked error") - r := &mocks.Reader{ - MockRead: func(b []byte) (int, error) { - return 0, expected - }, - } - ctx := context.Background() - out, err := CopyContext(ctx, io.Discard, r) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected", err) - } - if out != 0 { - t.Fatal("not the expected number of bytes") - } -} - -func TestCopyContextWithCancelledContext(t *testing.T) { - r := strings.NewReader("deadbeef") - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - out, err := CopyContext(ctx, io.Discard, r) - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected", err) - } - if out != 0 { - t.Fatal("not the expected number of bytes") - } -} - -func TestCopyContextWithErrorAndCancelledContext(t *testing.T) { - expected := errors.New("mocked error") - r := &mocks.Reader{ - MockRead: func(b []byte) (int, error) { - time.Sleep(time.Millisecond) - return 0, expected - }, - } - ctx, cancel := context.WithCancel(context.Background()) - cancel() // fail immediately - out, err := CopyContext(ctx, io.Discard, r) - if !errors.Is(err, context.Canceled) { - t.Fatal("not the error we expected", err) - } - if out != 0 { - t.Fatal("not the expected number of bytes") - } + t.Run("with failure and cancelled context", func(t *testing.T) { + wg := &sync.WaitGroup{} + wg.Add(1) + sigch := make(chan interface{}) + expected := errors.New("mocked error") + r := &mocks.Reader{ + MockRead: func(b []byte) (int, error) { + defer wg.Done() + <-sigch + return 0, expected + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // fail immediately + out, err := CopyContext(ctx, io.Discard, r) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if out != 0 { + t.Fatal("not the expected number of bytes") + } + close(sigch) + wg.Wait() + }) }