From 2bf7cb24493b31504404c41b1d78ea3204400954 Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Sat, 14 May 2022 01:18:29 +0200 Subject: [PATCH 1/6] Use server-side measurements during upload test. --- .../internal/emitter/humanreadable.go | 31 +++++++++++++------ cmd/ndt7-client/main.go | 16 +++++----- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/cmd/ndt7-client/internal/emitter/humanreadable.go b/cmd/ndt7-client/internal/emitter/humanreadable.go index 6e537f8..a1131a8 100644 --- a/cmd/ndt7-client/internal/emitter/humanreadable.go +++ b/cmd/ndt7-client/internal/emitter/humanreadable.go @@ -58,16 +58,29 @@ func (h HumanReadable) onSpeedEvent(m *spec.Measurement) error { // The specification recommends that we show application level // measurements. Let's just do that in interactive mode. To this // end, we ignore any measurement coming from the server. - if m.Origin != spec.OriginClient { - return nil + switch m.Test { + case spec.TestDownload: + if m.Origin == spec.OriginClient { + if m.AppInfo == nil || m.AppInfo.ElapsedTime <= 0 { + return errors.New("missing AppInfo or invalid ElapsedTime") + } + elapsed := float64(m.AppInfo.ElapsedTime) + v := 8.0 * float64(m.AppInfo.NumBytes) / elapsed + _, err := fmt.Fprintf(h.out, "\rAvg. speed : %7.1f Mbit/s", v) + return err + } + case spec.TestUpload: + if m.Origin == spec.OriginServer { + if m.TCPInfo == nil || m.TCPInfo.ElapsedTime <= 0 { + return errors.New("missing TCPInfo or invalid ElapsedTime") + } + elapsed := float64(m.TCPInfo.ElapsedTime) + v := 8.0 * float64(m.TCPInfo.BytesReceived) / elapsed + _, err := fmt.Fprintf(h.out, "\rAvg. speed : %7.1f Mbit/s", v) + return err + } } - if m.AppInfo == nil || m.AppInfo.ElapsedTime <= 0 { - return errors.New("Missing m.AppInfo or invalid m.AppInfo.ElapsedTime") - } - elapsed := float64(m.AppInfo.ElapsedTime) / 1e06 - v := (8.0 * float64(m.AppInfo.NumBytes)) / elapsed / (1000.0 * 1000.0) - _, err := fmt.Fprintf(h.out, "\rAvg. speed : %7.1f Mbit/s", v) - return err + return nil } // OnComplete handles the complete event diff --git a/cmd/ndt7-client/main.go b/cmd/ndt7-client/main.go index 1bc0c8a..70c8cf5 100644 --- a/cmd/ndt7-client/main.go +++ b/cmd/ndt7-client/main.go @@ -115,9 +115,9 @@ const ( ) var ( - ClientName = "ndt7-client-go-cmd" - ClientVersion = "0.6.1" - flagProfile = flag.String("profile", "", + ClientName = "ndt7-client-go-cmd" + ClientVersion = "0.6.1" + flagProfile = flag.String("profile", "", "file where to store pprof profile (see https://blog.golang.org/pprof)") flagScheme = flagx.Enum{ @@ -273,12 +273,14 @@ func makeSummary(FQDN string, results map[spec.TestKind]*ndt7.LatestMeasurements } } } - // Upload comes from the client-side Measurement during the upload test. + // The upload rate comes from the receiver (the server). Currently + // ndt-server only provides network-level throughput via TCPInfo. + // TODO: Use AppInfo for application-level measurements when available. if ul, ok := results[spec.TestUpload]; ok { - if ul.Client.AppInfo != nil && ul.Client.AppInfo.ElapsedTime > 0 { - elapsed := float64(ul.Client.AppInfo.ElapsedTime) / 1e06 + if ul.Server.TCPInfo != nil && ul.Server.TCPInfo.BytesReceived > 0 { + elapsed := float64(ul.Server.TCPInfo.ElapsedTime) / 1e06 s.Upload = emitter.ValueUnitPair{ - Value: (8.0 * float64(ul.Client.AppInfo.NumBytes)) / + Value: (8.0 * float64(ul.Server.TCPInfo.BytesReceived)) / elapsed / (1000.0 * 1000.0), Unit: "Mbit/s", } From 3521123abd758234a242b268c4b590153b995d07 Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Tue, 17 May 2022 03:04:33 +0200 Subject: [PATCH 2/6] Update makeSummary test in main_test.go --- cmd/ndt7-client/main_test.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cmd/ndt7-client/main_test.go b/cmd/ndt7-client/main_test.go index 8a3d60a..159b980 100644 --- a/cmd/ndt7-client/main_test.go +++ b/cmd/ndt7-client/main_test.go @@ -387,6 +387,9 @@ func TestMakeSummary(t *testing.T) { tcpInfo.BytesSent = 100 tcpInfo.BytesRetrans = 1 tcpInfo.MinRTT = 10000 + // Simulate a 8Mb/s upload rate. + tcpInfo.BytesReceived = 10000000 + tcpInfo.ElapsedTime = 10000000 results := map[spec.TestKind]*ndt7.LatestMeasurements{ spec.TestDownload: { @@ -406,11 +409,8 @@ func TestMakeSummary(t *testing.T) { }, }, spec.TestUpload: { - Client: spec.Measurement{ - AppInfo: &spec.AppInfo{ - NumBytes: 100, - ElapsedTime: 1, - }, + Server: spec.Measurement{ + TCPInfo: tcpInfo, }, }, } @@ -425,7 +425,7 @@ func TestMakeSummary(t *testing.T) { Unit: "Mbit/s", }, Upload: emitter.ValueUnitPair{ - Value: 800.0, + Value: 8.0, Unit: "Mbit/s", }, DownloadRetrans: emitter.ValueUnitPair{ @@ -441,6 +441,7 @@ func TestMakeSummary(t *testing.T) { generated := makeSummary("test", results) if !reflect.DeepEqual(generated, expected) { + t.Errorf("expected %+v; got %+v", expected, generated) t.Fatal("makeSummary(): unexpected summary data") } } From d279b463d90b34b6f024bc10b793026231bc2667 Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Tue, 17 May 2022 03:15:15 +0200 Subject: [PATCH 3/6] Update failing test --- cmd/ndt7-client/internal/emitter/humanreadable_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/ndt7-client/internal/emitter/humanreadable_test.go b/cmd/ndt7-client/internal/emitter/humanreadable_test.go index 86a186a..8c76569 100644 --- a/cmd/ndt7-client/internal/emitter/humanreadable_test.go +++ b/cmd/ndt7-client/internal/emitter/humanreadable_test.go @@ -110,9 +110,10 @@ func TestHumanReadableOnDownloadEventFailure(t *testing.T) { ElapsedTime: 1234, }, Origin: spec.OriginClient, + Test: spec.TestDownload, }) if err != mocks.ErrMocked { - t.Fatal("Not the error we expected") + t.Fatalf("Not the error we expected: %v", err) } } From adbb33b34e35e8f3e7952d6ee7247e4306d8bd1d Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Tue, 17 May 2022 03:51:25 +0200 Subject: [PATCH 4/6] Fix tests. --- .../internal/emitter/humanreadable_test.go | 34 +++++---- internal/download/download_test.go | 73 +++++++++++++------ internal/upload/upload_test.go | 63 ++++++++++------ 3 files changed, 111 insertions(+), 59 deletions(-) diff --git a/cmd/ndt7-client/internal/emitter/humanreadable_test.go b/cmd/ndt7-client/internal/emitter/humanreadable_test.go index 8c76569..d61640b 100644 --- a/cmd/ndt7-client/internal/emitter/humanreadable_test.go +++ b/cmd/ndt7-client/internal/emitter/humanreadable_test.go @@ -88,6 +88,7 @@ func TestHumanReadableOnDownloadEvent(t *testing.T) { NumBytes: 100000000, }, Origin: spec.OriginClient, + Test: spec.TestDownload, }) if err != nil { t.Fatal(err) @@ -134,12 +135,13 @@ func TestHumanReadableIgnoresServerData(t *testing.T) { func TestHumanReadableOnUploadEvent(t *testing.T) { sw := &mocks.SavingWriter{} hr := HumanReadable{sw} + tcpInfo := &spec.TCPInfo{} + tcpInfo.BytesReceived = 10000000 + tcpInfo.ElapsedTime = 10000000 err := hr.OnUploadEvent(&spec.Measurement{ - AppInfo: &spec.AppInfo{ - ElapsedTime: 3000000, - NumBytes: 100000000, - }, - Origin: spec.OriginClient, + TCPInfo: tcpInfo, + Origin: spec.OriginServer, + Test: spec.TestUpload, }) if err != nil { t.Fatal(err) @@ -149,7 +151,7 @@ func TestHumanReadableOnUploadEvent(t *testing.T) { } if !reflect.DeepEqual( sw.Data[0], - []byte("\rAvg. speed : 266.7 Mbit/s"), + []byte("\rAvg. speed : 8.0 Mbit/s"), ) { t.Fatal("unexpected output") } @@ -159,22 +161,24 @@ func TestHumanReadableOnUploadEventSafetyCheck(t *testing.T) { sw := &mocks.SavingWriter{} hr := HumanReadable{sw} err := hr.OnUploadEvent(&spec.Measurement{ - Origin: spec.OriginClient, + Origin: spec.OriginServer, + Test: spec.TestUpload, }) if err == nil { t.Fatal("We did expect an error here") } if len(sw.Data) != 0 { - t.Fatal("Some data was written and it shouldn't have") + t.Fatal("Some data was written and it shouldn't have been") } } -func TestHumanReadableOnUploadEventDivideByZero(t *testing.T) { +func TestHumanReadableOnUploadMissingTCPInfo(t *testing.T) { sw := &mocks.SavingWriter{} hr := HumanReadable{sw} err := hr.OnUploadEvent(&spec.Measurement{ AppInfo: &spec.AppInfo{}, - Origin: spec.OriginClient, + Origin: spec.OriginServer, + Test: spec.TestUpload, }) if err == nil { t.Fatal("We did expect an error here") @@ -186,11 +190,13 @@ func TestHumanReadableOnUploadEventDivideByZero(t *testing.T) { func TestHumanReadableOnUploadEventFailure(t *testing.T) { hr := HumanReadable{&mocks.FailingWriter{}} + tcpInfo := &spec.TCPInfo{} + tcpInfo.BytesReceived = 10000000 + tcpInfo.ElapsedTime = 10000000 err := hr.OnUploadEvent(&spec.Measurement{ - AppInfo: &spec.AppInfo{ - ElapsedTime: 1234, - }, - Origin: spec.OriginClient, + TCPInfo: tcpInfo, + Origin: spec.OriginServer, + Test: spec.TestUpload, }) if err != mocks.ErrMocked { t.Fatal("Not the error we expected") diff --git a/internal/download/download_test.go b/internal/download/download_test.go index 8c6b7f1..ccb5f10 100644 --- a/internal/download/download_test.go +++ b/internal/download/download_test.go @@ -92,12 +92,10 @@ func TestReadBinary(t *testing.T) { NextReaderMessageType: websocket.BinaryMessage, MessageByteArray: []byte("12345678"), } - go func() { - err := Run(ctx, &conn, outch) - if err != nil { - t.Fatal(err) - } - }() + errch := make(chan error, 1) + go func(errch chan<- error) { + errch <- Run(ctx, &conn, outch) + }(errch) prev := spec.Measurement{ AppInfo: &spec.AppInfo{}, } @@ -119,6 +117,9 @@ func TestReadBinary(t *testing.T) { } prev = m } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestSetReadDeadlineError(t *testing.T) { @@ -133,15 +134,21 @@ func TestSetReadDeadlineError(t *testing.T) { MessageByteArray: []byte("{}"), SetReadDeadlineResult: mockedErr, } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("We didn't expect measurements here") + errch <- errors.New("We didn't expect measurements here") + return } - }() + errch <- nil + }(errch) err := Run(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error that we were expecting") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestReadMessageError(t *testing.T) { @@ -156,15 +163,21 @@ func TestReadMessageError(t *testing.T) { MessageByteArray: []byte("{}"), NextReaderResult: mockedErr, } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("We didn't expect measurements here") + errch <- errors.New("We didn't expect measurements here") + return } - }() + errch <- nil + }(errch) err := Run(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error that we were expecting") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestReaderError(t *testing.T) { @@ -178,15 +191,21 @@ func TestReaderError(t *testing.T) { NextReaderMessageType: websocket.TextMessage, NextReaderMustFail: true, } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("We didn't expect measurements here") + errch <- errors.New("We didn't expect measurements here") + return } - }() + errch <- nil + }(errch) err := Run(ctx, &conn, outch) if err != mocks.ErrReadFailed { t.Fatal("Not the error that we were expecting") } + if err := <-errch; err != nil { + t.Fatal(err) + } // Test when type is websocket.BinaryMessage outch = make(chan spec.Measurement) ctx, cancel = context.WithTimeout( @@ -197,15 +216,21 @@ func TestReaderError(t *testing.T) { NextReaderMessageType: websocket.BinaryMessage, NextReaderMustFail: true, } - go func() { + errch = make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("We didn't expect measurements here") + errch <- errors.New("We didn't expect measurements here") + return } - }() + errch <- nil + }(errch) err = Run(ctx, &conn, outch) if err != mocks.ErrReadFailed { t.Fatal("Not the error that we were expecting") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestReadInvalidJSON(t *testing.T) { @@ -218,13 +243,19 @@ func TestReadInvalidJSON(t *testing.T) { NextReaderMessageType: websocket.TextMessage, MessageByteArray: []byte("{"), } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("We didn't expect measurements here") + errch <- errors.New("We didn't expect measurements here") + return } - }() + errch <- nil + }(errch) err := Run(ctx, &conn, outch) if err == nil { t.Fatal("We expected to have an error here") } + if err := <-errch; err != nil { + t.Fatal(err) + } } diff --git a/internal/upload/upload_test.go b/internal/upload/upload_test.go index 544c02e..46319de 100644 --- a/internal/upload/upload_test.go +++ b/internal/upload/upload_test.go @@ -22,20 +22,21 @@ func TestNormal(t *testing.T) { MessageByteArray: []byte("{}"), ReadMessageType: websocket.TextMessage, } - go func() { - err := Run(ctx, &conn, outch) - if err != nil { - t.Fatal(err) - } - }() + errch := make(chan error) + go func(errch chan<- error) { + errch <- Run(ctx, &conn, outch) + }(errch) tot := 0 // Drain the channel and count the number of Measurements read. - for _ = range outch { + for range outch { tot++ } if tot <= 0 { t.Fatal("Expected at least one message") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestSetReadDeadlineError(t *testing.T) { @@ -46,8 +47,7 @@ func TestSetReadDeadlineError(t *testing.T) { ch := make(chan spec.Measurement, 128) errCh := make(chan error) go readcounterflow(context.Background(), &conn, ch, errCh) - err := <-errCh - if err != mockedErr { + if err := <-errCh; err != mockedErr { t.Fatal("Not the error we expected") } } @@ -61,8 +61,7 @@ func TestReadMessageError(t *testing.T) { errCh := make(chan error) defer close(errCh) go readcounterflow(context.Background(), &conn, ch, errCh) - err := <-errCh - if err != mockedErr { + if err := <-errCh; err != mockedErr { t.Fatal("Not the error we expected") } } @@ -76,8 +75,7 @@ func TestReadNonTextMessageError(t *testing.T) { errCh := make(chan error) defer close(errCh) go readcounterflow(context.Background(), &conn, ch, errCh) - err := <-errCh - if err != errNonTextMessage { + if err := <-errCh; err != errNonTextMessage { t.Fatal("Not the error we expected") } } @@ -115,8 +113,7 @@ func TestReadGoodMessage(t *testing.T) { errCh := make(chan error) defer close(errCh) go readcounterflow(ctx, &conn, ch, errCh) - err := <-errCh - if err != nil { + if err := <-errCh; err != nil { t.Fatal(err) } } @@ -133,16 +130,22 @@ func TestMakePreparedMessageError(t *testing.T) { return nil, mockedErr } conn := mocks.Conn{} - go func() { + errch := make(chan error) + go func(errch chan<- error) { for range outch { - t.Fatal("Did not expect messages here") + errch <- errors.New("Did not expect messages here") + return } - }() + errch <- nil + }(errch) err := upload(ctx, &conn, outch) makePreparedMessage = savedFunc if err != mockedErr { t.Fatal("Not the error we expected") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestSetWriteDeadlineError(t *testing.T) { @@ -155,15 +158,21 @@ func TestSetWriteDeadlineError(t *testing.T) { conn := mocks.Conn{ SetWriteDeadlineResult: mockedErr, } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("Did not expect messages here") + errch <- errors.New("Did not expect messages here") + return } - }() + errch <- nil + }(errch) err := upload(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error we expected") } + if err := <-errch; err != nil { + t.Fatal(err) + } } func TestWritePreparedMessageError(t *testing.T) { @@ -176,13 +185,19 @@ func TestWritePreparedMessageError(t *testing.T) { conn := mocks.Conn{ WritePreparedMessageResult: mockedErr, } - go func() { + errch := make(chan error, 1) + go func(errch chan<- error) { for range outch { - t.Fatal("Did not expect messages here") + errch <- errors.New("Did not expect messages here") + return } - }() + errch <- nil + }(errch) err := upload(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error we expected") } + if err := <-errch; err != nil { + t.Fatal(err) + } } From 7aa29068ff0762095419762bdc115bb68d42d475 Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Tue, 17 May 2022 04:04:50 +0200 Subject: [PATCH 5/6] Simplify error handling in goroutines. Just use t.Error() --- internal/download/download_test.go | 71 +++++++++--------------------- internal/upload/upload_test.go | 49 +++++++-------------- 2 files changed, 36 insertions(+), 84 deletions(-) diff --git a/internal/download/download_test.go b/internal/download/download_test.go index ccb5f10..ad006d0 100644 --- a/internal/download/download_test.go +++ b/internal/download/download_test.go @@ -92,10 +92,12 @@ func TestReadBinary(t *testing.T) { NextReaderMessageType: websocket.BinaryMessage, MessageByteArray: []byte("12345678"), } - errch := make(chan error, 1) - go func(errch chan<- error) { - errch <- Run(ctx, &conn, outch) - }(errch) + go func() { + err := Run(ctx, &conn, outch) + if err != nil { + t.Errorf("error: %v", err) + } + }() prev := spec.Measurement{ AppInfo: &spec.AppInfo{}, } @@ -117,9 +119,6 @@ func TestReadBinary(t *testing.T) { } prev = m } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestSetReadDeadlineError(t *testing.T) { @@ -134,21 +133,15 @@ func TestSetReadDeadlineError(t *testing.T) { MessageByteArray: []byte("{}"), SetReadDeadlineResult: mockedErr, } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("We didn't expect measurements here") - return + t.Error("We didn't expect measurements here") } - errch <- nil - }(errch) + }() err := Run(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error that we were expecting") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestReadMessageError(t *testing.T) { @@ -163,21 +156,15 @@ func TestReadMessageError(t *testing.T) { MessageByteArray: []byte("{}"), NextReaderResult: mockedErr, } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("We didn't expect measurements here") - return + t.Error("We didn't expect measurements here") } - errch <- nil - }(errch) + }() err := Run(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error that we were expecting") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestReaderError(t *testing.T) { @@ -191,21 +178,16 @@ func TestReaderError(t *testing.T) { NextReaderMessageType: websocket.TextMessage, NextReaderMustFail: true, } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("We didn't expect measurements here") + t.Error("We didn't expect measurements here") return } - errch <- nil - }(errch) + }() err := Run(ctx, &conn, outch) if err != mocks.ErrReadFailed { t.Fatal("Not the error that we were expecting") } - if err := <-errch; err != nil { - t.Fatal(err) - } // Test when type is websocket.BinaryMessage outch = make(chan spec.Measurement) ctx, cancel = context.WithTimeout( @@ -216,21 +198,15 @@ func TestReaderError(t *testing.T) { NextReaderMessageType: websocket.BinaryMessage, NextReaderMustFail: true, } - errch = make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("We didn't expect measurements here") - return + t.Error("We didn't expect measurements here") } - errch <- nil - }(errch) + }() err = Run(ctx, &conn, outch) if err != mocks.ErrReadFailed { t.Fatal("Not the error that we were expecting") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestReadInvalidJSON(t *testing.T) { @@ -243,19 +219,14 @@ func TestReadInvalidJSON(t *testing.T) { NextReaderMessageType: websocket.TextMessage, MessageByteArray: []byte("{"), } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("We didn't expect measurements here") + t.Error("We didn't expect measurements here") return } - errch <- nil - }(errch) + }() err := Run(ctx, &conn, outch) if err == nil { t.Fatal("We expected to have an error here") } - if err := <-errch; err != nil { - t.Fatal(err) - } } diff --git a/internal/upload/upload_test.go b/internal/upload/upload_test.go index 46319de..d07e14e 100644 --- a/internal/upload/upload_test.go +++ b/internal/upload/upload_test.go @@ -22,10 +22,12 @@ func TestNormal(t *testing.T) { MessageByteArray: []byte("{}"), ReadMessageType: websocket.TextMessage, } - errch := make(chan error) - go func(errch chan<- error) { - errch <- Run(ctx, &conn, outch) - }(errch) + go func() { + err := Run(ctx, &conn, outch) + if err != nil { + t.Errorf("error: %v", err) + } + }() tot := 0 // Drain the channel and count the number of Measurements read. for range outch { @@ -34,9 +36,6 @@ func TestNormal(t *testing.T) { if tot <= 0 { t.Fatal("Expected at least one message") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestSetReadDeadlineError(t *testing.T) { @@ -130,22 +129,16 @@ func TestMakePreparedMessageError(t *testing.T) { return nil, mockedErr } conn := mocks.Conn{} - errch := make(chan error) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("Did not expect messages here") - return + t.Error("Did not expect messages here") } - errch <- nil - }(errch) + }() err := upload(ctx, &conn, outch) makePreparedMessage = savedFunc if err != mockedErr { t.Fatal("Not the error we expected") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestSetWriteDeadlineError(t *testing.T) { @@ -158,21 +151,15 @@ func TestSetWriteDeadlineError(t *testing.T) { conn := mocks.Conn{ SetWriteDeadlineResult: mockedErr, } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("Did not expect messages here") - return + t.Error("Did not expect messages here") } - errch <- nil - }(errch) + }() err := upload(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error we expected") } - if err := <-errch; err != nil { - t.Fatal(err) - } } func TestWritePreparedMessageError(t *testing.T) { @@ -185,19 +172,13 @@ func TestWritePreparedMessageError(t *testing.T) { conn := mocks.Conn{ WritePreparedMessageResult: mockedErr, } - errch := make(chan error, 1) - go func(errch chan<- error) { + go func() { for range outch { - errch <- errors.New("Did not expect messages here") - return + t.Error("Did not expect messages here") } - errch <- nil - }(errch) + }() err := upload(ctx, &conn, outch) if err != mockedErr { t.Fatal("Not the error we expected") } - if err := <-errch; err != nil { - t.Fatal(err) - } } From cd9fe5ec2779febbec6d20cfe6a42bed4cc70690 Mon Sep 17 00:00:00 2001 From: Roberto D'Auria Date: Tue, 17 May 2022 04:12:46 +0200 Subject: [PATCH 6/6] Remove extra returns --- internal/download/download_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/download/download_test.go b/internal/download/download_test.go index ad006d0..9eb0193 100644 --- a/internal/download/download_test.go +++ b/internal/download/download_test.go @@ -181,7 +181,6 @@ func TestReaderError(t *testing.T) { go func() { for range outch { t.Error("We didn't expect measurements here") - return } }() err := Run(ctx, &conn, outch) @@ -222,7 +221,6 @@ func TestReadInvalidJSON(t *testing.T) { go func() { for range outch { t.Error("We didn't expect measurements here") - return } }() err := Run(ctx, &conn, outch)