From 34062cb1779bc7c1e4e8668a0e0de23a94ba16d2 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 15 Jun 2021 18:11:47 +0200 Subject: [PATCH] refactor(mlablocate*): use internal testing (#382) This is not an external package and it's fine to just use internal testing. It reduces the complexity a little bit. --- .../internal/mlablocate/mlablocate_test.go | 17 +++-- .../internal/mlablocatev2/mlablocatev2.go | 2 +- .../mlablocatev2_internal_test.go | 16 ----- .../mlablocatev2/mlablocatev2_test.go | 63 +++++++++---------- 4 files changed, 40 insertions(+), 58 deletions(-) delete mode 100644 internal/engine/internal/mlablocatev2/mlablocatev2_internal_test.go diff --git a/internal/engine/internal/mlablocate/mlablocate_test.go b/internal/engine/internal/mlablocate/mlablocate_test.go index e87a635..b4d7b6c 100644 --- a/internal/engine/internal/mlablocate/mlablocate_test.go +++ b/internal/engine/internal/mlablocate/mlablocate_test.go @@ -1,4 +1,4 @@ -package mlablocate_test +package mlablocate import ( "context" @@ -9,11 +9,10 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/internal/mlablocate" ) func TestWithoutProxy(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -28,7 +27,7 @@ func TestWithoutProxy(t *testing.T) { } func Test404Response(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -43,7 +42,7 @@ func Test404Response(t *testing.T) { } func TestNewRequestFailure(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -59,7 +58,7 @@ func TestNewRequestFailure(t *testing.T) { } func TestHTTPClientDoFailure(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -86,7 +85,7 @@ func (txp *roundTripFails) RoundTrip(*http.Request) (*http.Response, error) { } func TestCannotReadBody(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -128,7 +127,7 @@ func (b *readingBodyFailsBody) Close() error { } func TestInvalidJSON(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", @@ -169,7 +168,7 @@ func (b *invalidJSONBody) Close() error { } func TestEmptyFQDN(t *testing.T) { - client := mlablocate.NewClient( + client := NewClient( http.DefaultClient, log.Log, "miniooni/0.1.0-dev", diff --git a/internal/engine/internal/mlablocatev2/mlablocatev2.go b/internal/engine/internal/mlablocatev2/mlablocatev2.go index 36777d3..2bad414 100644 --- a/internal/engine/internal/mlablocatev2/mlablocatev2.go +++ b/internal/engine/internal/mlablocatev2/mlablocatev2.go @@ -1,4 +1,4 @@ -// Package mlablocatev2 use m-lab locate services API v2. +// Package mlablocatev2 implements m-lab locate services API v2. package mlablocatev2 import ( diff --git a/internal/engine/internal/mlablocatev2/mlablocatev2_internal_test.go b/internal/engine/internal/mlablocatev2/mlablocatev2_internal_test.go deleted file mode 100644 index 0fceaf3..0000000 --- a/internal/engine/internal/mlablocatev2/mlablocatev2_internal_test.go +++ /dev/null @@ -1,16 +0,0 @@ -package mlablocatev2 - -import "context" - -type ResultRecord resultRecord - -func (c Client) Query(ctx context.Context, path string) (ResultRecord, error) { - out, err := c.query(ctx, path) - if err != nil { - return ResultRecord{}, err - - } - return ResultRecord(out), nil -} - -type EntryRecord = entryRecord diff --git a/internal/engine/internal/mlablocatev2/mlablocatev2_test.go b/internal/engine/internal/mlablocatev2/mlablocatev2_test.go index 19d179b..8650a26 100644 --- a/internal/engine/internal/mlablocatev2/mlablocatev2_test.go +++ b/internal/engine/internal/mlablocatev2/mlablocatev2_test.go @@ -1,4 +1,4 @@ -package mlablocatev2_test +package mlablocatev2 import ( "context" @@ -10,14 +10,13 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/engine/internal/mlablocatev2" ) func TestSuccess(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") result, err := client.QueryNDT7(context.Background()) if err != nil { t.Fatal(err) @@ -51,9 +50,9 @@ func Test404Response(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") - result, err := client.Query(context.Background(), "nonexistent") - if !errors.Is(err, mlablocatev2.ErrRequestFailed) { + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + result, err := client.query(context.Background(), "nonexistent") + if !errors.Is(err, ErrRequestFailed) { t.Fatal("not the error we expected") } if result.Results != nil { @@ -62,9 +61,9 @@ func Test404Response(t *testing.T) { } func TestNewRequestFailure(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") client.Hostname = "\t" - result, err := client.Query(context.Background(), "nonexistent") + result, err := client.query(context.Background(), "nonexistent") if err == nil || !strings.Contains(err.Error(), "invalid URL escape") { t.Fatal("not the error we expected") } @@ -74,12 +73,12 @@ func TestNewRequestFailure(t *testing.T) { } func TestHTTPClientDoFailure(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") expected := errors.New("mocked error") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{Err: expected}, + Transport: FakeTransport{Err: expected}, } - result, err := client.Query(context.Background(), "nonexistent") + result, err := client.query(context.Background(), "nonexistent") if !errors.Is(err, expected) { t.Fatal("not the error we expected") } @@ -89,19 +88,19 @@ func TestHTTPClientDoFailure(t *testing.T) { } func TestCannotReadBody(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") expected := errors.New("mocked error") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{ + Transport: FakeTransport{ Resp: &http.Response{ StatusCode: 200, - Body: mlablocatev2.FakeBody{ + Body: FakeBody{ Err: expected, }, }, }, } - result, err := client.Query(context.Background(), "nonexistent") + result, err := client.query(context.Background(), "nonexistent") if !errors.Is(err, expected) { t.Fatal("not the error we expected") } @@ -111,19 +110,19 @@ func TestCannotReadBody(t *testing.T) { } func TestInvalidJSON(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{ + Transport: FakeTransport{ Resp: &http.Response{ StatusCode: 200, - Body: mlablocatev2.FakeBody{ + Body: FakeBody{ Err: io.EOF, Data: []byte(`{`), }, }, }, } - result, err := client.Query(context.Background(), "nonexistent") + result, err := client.query(context.Background(), "nonexistent") if err == nil || !strings.Contains(err.Error(), "unexpected end of JSON input") { t.Fatal("not the error we expected") } @@ -133,12 +132,12 @@ func TestInvalidJSON(t *testing.T) { } func TestEmptyResponse(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{ + Transport: FakeTransport{ Resp: &http.Response{ StatusCode: 200, - Body: mlablocatev2.FakeBody{ + Body: FakeBody{ Err: io.EOF, Data: []byte(`{}`), }, @@ -146,7 +145,7 @@ func TestEmptyResponse(t *testing.T) { }, } result, err := client.QueryNDT7(context.Background()) - if !errors.Is(err, mlablocatev2.ErrEmptyResponse) { + if !errors.Is(err, ErrEmptyResponse) { t.Fatal("not the error we expected") } if result != nil { @@ -155,17 +154,17 @@ func TestEmptyResponse(t *testing.T) { } func TestNDT7QueryFails(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{ + Transport: FakeTransport{ Resp: &http.Response{ StatusCode: 404, - Body: mlablocatev2.FakeBody{Err: io.EOF}, + Body: FakeBody{Err: io.EOF}, }, }, } result, err := client.QueryNDT7(context.Background()) - if !errors.Is(err, mlablocatev2.ErrRequestFailed) { + if !errors.Is(err, ErrRequestFailed) { t.Fatal("not the error we expected") } if result != nil { @@ -174,12 +173,12 @@ func TestNDT7QueryFails(t *testing.T) { } func TestNDT7InvalidURLs(t *testing.T) { - client := mlablocatev2.NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") + client := NewClient(http.DefaultClient, log.Log, "miniooni/0.1.0-dev") client.HTTPClient = &http.Client{ - Transport: mlablocatev2.FakeTransport{ + Transport: FakeTransport{ Resp: &http.Response{ StatusCode: 200, - Body: mlablocatev2.FakeBody{ + Body: FakeBody{ Data: []byte( `{"results":[{"machine":"mlab3-mil04.mlab-oti.measurement-lab.org","urls":{"wss:///ndt/v7/download":":","wss:///ndt/v7/upload":":"}}]}`), Err: io.EOF, @@ -188,7 +187,7 @@ func TestNDT7InvalidURLs(t *testing.T) { }, } result, err := client.QueryNDT7(context.Background()) - if !errors.Is(err, mlablocatev2.ErrEmptyResponse) { + if !errors.Is(err, ErrEmptyResponse) { t.Fatal("not the error we expected") } if result != nil { @@ -220,7 +219,7 @@ func TestEntryRecordSite(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - er := mlablocatev2.EntryRecord{ + er := entryRecord{ Machine: tt.fields.Machine, URLs: tt.fields.URLs, }