From cc252040ddeadb84e8d0ee626bce5c172eefbc66 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 2 May 2024 15:17:24 +0200 Subject: [PATCH] refactor(webconnectivity*): don't call httpapi directly (#1582) The https://github.com/ooni/probe-cli/pull/1581 pull request missed that adding a `CallWebConnectivityTestHelper` method to `*engine.Session` means we need to mock such a method in `webconnectivityqa`. I could have mocked the method, but I was not super happy about doing this, because I would rather have wanted to add more QA tests inside of `webconnectivityqa` making sure we're indeed falling back when using test helpers. (We have unit tests testing that, but it would be nice to have integration tests, so we could observe the overall code behavior.) My initial approach for allowing this was to create a `CallWebConnectivityTestHelper` function inside the engine, taking a `model.ExperimentSession` as its fourth argument and then modifying the `CallWebConnectivityTestHelper` method to just invoke the function passing to it the session. However, this approach created import cycles in tests. To avoid import cycles, I moved the `CallWebConnectivityTestHelper` function to `webconnectivityalgo` and modified the mocks inside `webconnectivityqa` to call such a function. This is where I paused and realized that the cleanest solution is actually to just always call the `CallWebConnectivityTestHelper` function in `webconnectivityalgo` and undo part of the changes in https://github.com/ooni/probe-cli/pull/1581 to avoid having a `CallWebConnectivityTestHelper` method inside of `model.ExperimentSession`. And so I also implemented this change. All of this leads us to the current diff, which is part of https://github.com/ooni/probe/issues/2725. --- internal/engine/session.go | 32 -- internal/engine/session_internal_test.go | 303 ----------------- .../experiment/webconnectivity/control.go | 9 +- .../experiment/webconnectivitylte/control.go | 11 +- internal/legacy/mockable/mockable.go | 9 - internal/mocks/session.go | 8 - internal/mocks/session_test.go | 20 -- internal/model/experiment.go | 16 - .../webconnectivityalgo/calltesthelpers.go | 50 +++ .../calltesthelpers_test.go | 309 ++++++++++++++++++ 10 files changed, 363 insertions(+), 404 deletions(-) create mode 100644 internal/webconnectivityalgo/calltesthelpers.go create mode 100644 internal/webconnectivityalgo/calltesthelpers_test.go diff --git a/internal/engine/session.go b/internal/engine/session.go index 7d82f741ff..855191f95a 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -13,10 +13,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/enginelocate" "github.com/ooni/probe-cli/v3/internal/enginenetx" "github.com/ooni/probe-cli/v3/internal/engineresolver" - "github.com/ooni/probe-cli/v3/internal/httpapi" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/ooapi" "github.com/ooni/probe-cli/v3/internal/platform" "github.com/ooni/probe-cli/v3/internal/probeservices" "github.com/ooni/probe-cli/v3/internal/runtimex" @@ -690,34 +688,4 @@ func (s *Session) MaybeLookupLocationContext(ctx context.Context) error { return nil } -// CallWebConnectivityTestHelper implements [model.EngineExperimentSession]. -func (s *Session) CallWebConnectivityTestHelper(ctx context.Context, - creq *model.THRequest, testhelpers []model.OOAPIService) (*model.THResponse, int, error) { - // handle the case where there are no available web connectivity test helpers - if len(testhelpers) <= 0 { - return nil, 0, model.ErrNoAvailableTestHelpers - } - - // initialize a sequence caller for invoking the THs in FIFO order - seqCaller := httpapi.NewSequenceCaller( - ooapi.NewDescriptorTH(creq), - httpapi.NewEndpointList(s.DefaultHTTPClient(), s.Logger(), s.UserAgent(), testhelpers...)..., - ) - - // issue the composed call proper and obtain a response and an index or an error - cresp, idx, err := seqCaller.Call(ctx) - - // handle the case where all test helpers failed - if err != nil { - return nil, 0, err - } - - // apply some sanity checks to the results - runtimex.Assert(idx >= 0 && idx < len(testhelpers), "idx out of bounds") - runtimex.Assert(cresp != nil, "out is nil") - - // return the results to the web connectivity caller - return cresp, idx, nil -} - var _ model.ExperimentSession = &Session{} diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go index 4bec76abbf..b2971af49b 100644 --- a/internal/engine/session_internal_test.go +++ b/internal/engine/session_internal_test.go @@ -3,7 +3,6 @@ package engine import ( "context" "errors" - "net/http" "net/url" "sync" "testing" @@ -11,18 +10,13 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/bytecounter" "github.com/ooni/probe-cli/v3/internal/checkincache" "github.com/ooni/probe-cli/v3/internal/enginelocate" - "github.com/ooni/probe-cli/v3/internal/enginenetx" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/registry" - "github.com/ooni/probe-cli/v3/internal/testingx" - "github.com/ooni/probe-cli/v3/internal/version" ) func (s *Session) GetAvailableProbeServices() []model.OOAPIService { @@ -411,300 +405,3 @@ func TestSessionNewExperimentBuilder(t *testing.T) { } }) } - -// This function tests the [*Session.CallWebConnectivityTestHelper] method. -func TestSessionCallWebConnectivityTestHelper(t *testing.T) { - // We start with simple tests that exercise the basic functionality of the method - // without bothering with having more than one available test helper. - - t.Run("when there are no available test helpers", func(t *testing.T) { - // create a new session only initializing the fields that - // are going to matter for running this specific test - sess := &Session{ - network: enginenetx.NewNetwork( - bytecounter.New(), - &kvstore.Memory{}, - model.DiscardLogger, - nil, - (&netxlite.Netx{}).NewStdlibResolver(model.DiscardLogger), - ), - logger: model.DiscardLogger, - softwareName: "miniooni", - softwareVersion: version.Version, - } - - // create a new background context - ctx := context.Background() - - // create a fake request for the test helper - // - // note: no need to fill the request for this test case - creq := &model.THRequest{} - - // invoke the API - cresp, idx, err := sess.CallWebConnectivityTestHelper(ctx, creq, nil) - - // make sure we get the expected error - if !errors.Is(err, model.ErrNoAvailableTestHelpers) { - t.Fatal("unexpected error", err) - } - - // make sure idx is zero - if idx != 0 { - t.Fatal("expected zero, got", idx) - } - - // make sure cresp is nil - if cresp != nil { - t.Fatal("expected nil, got", cresp) - } - }) - - t.Run("when the call fails", func(t *testing.T) { - // create a local test server that always resets the connection - server := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) - defer server.Close() - - // create a new session only initializing the fields that - // are going to matter for running this specific test - sess := &Session{ - network: enginenetx.NewNetwork( - bytecounter.New(), - &kvstore.Memory{}, - model.DiscardLogger, - nil, - (&netxlite.Netx{}).NewStdlibResolver(model.DiscardLogger), - ), - logger: model.DiscardLogger, - softwareName: "miniooni", - softwareVersion: version.Version, - } - - // create a new background context - ctx := context.Background() - - // create a fake request for the test helper - // - // note: no need to fill the request for this test case - creq := &model.THRequest{} - - // create the list of test helpers to use - testhelpers := []model.OOAPIService{{ - Address: server.URL, - Type: "https", - Front: "", - }} - - // invoke the API - cresp, idx, err := sess.CallWebConnectivityTestHelper(ctx, creq, testhelpers) - - // make sure we get the expected error - if !errors.Is(err, netxlite.ECONNRESET) { - t.Fatal("unexpected error", err) - } - - // make sure idx is zero - if idx != 0 { - t.Fatal("expected zero, got", idx) - } - - // make sure cresp is nil - if cresp != nil { - t.Fatal("expected nil, got", cresp) - } - }) - - t.Run("when the call succeeds", func(t *testing.T) { - // create a local test server that always returns an ~empty response - server := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{}`)) - })) - defer server.Close() - - // create a new session only initializing the fields that - // are going to matter for running this specific test - sess := &Session{ - network: enginenetx.NewNetwork( - bytecounter.New(), - &kvstore.Memory{}, - model.DiscardLogger, - nil, - (&netxlite.Netx{}).NewStdlibResolver(model.DiscardLogger), - ), - logger: model.DiscardLogger, - softwareName: "miniooni", - softwareVersion: version.Version, - } - - // create a new background context - ctx := context.Background() - - // create a fake request for the test helper - // - // note: no need to fill the request for this test case - creq := &model.THRequest{} - - // create the list of test helpers to use - testhelpers := []model.OOAPIService{{ - Address: server.URL, - Type: "https", - Front: "", - }} - - // invoke the API - cresp, idx, err := sess.CallWebConnectivityTestHelper(ctx, creq, testhelpers) - - // make sure we get the expected error - if err != nil { - t.Fatal("unexpected error", err) - } - - // make sure idx is zero - if idx != 0 { - t.Fatal("expected zero, got", idx) - } - - // make sure cresp is not nil - if cresp == nil { - t.Fatal("expected not nil, got", cresp) - } - }) - - t.Run("with two test helpers where the first one resets the connection and the second works", func(t *testing.T) { - // create a local test server1 that always resets the connection - server1 := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) - defer server1.Close() - - // create a local test server2 that always returns an ~empty response - server2 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{}`)) - })) - defer server2.Close() - - // create a new session only initializing the fields that - // are going to matter for running this specific test - sess := &Session{ - network: enginenetx.NewNetwork( - bytecounter.New(), - &kvstore.Memory{}, - model.DiscardLogger, - nil, - (&netxlite.Netx{}).NewStdlibResolver(model.DiscardLogger), - ), - logger: model.DiscardLogger, - softwareName: "miniooni", - softwareVersion: version.Version, - } - - // create a new background context - ctx := context.Background() - - // create a fake request for the test helper - // - // note: no need to fill the request for this test case - creq := &model.THRequest{} - - // create the list of test helpers to use - testhelpers := []model.OOAPIService{{ - Address: server1.URL, - Type: "https", - Front: "", - }, { - Address: server2.URL, - Type: "https", - Front: "", - }} - - // invoke the API - cresp, idx, err := sess.CallWebConnectivityTestHelper(ctx, creq, testhelpers) - - // make sure we get the expected error - if err != nil { - t.Fatal("unexpected error", err) - } - - // make sure idx is one - if idx != 1 { - t.Fatal("expected one, got", idx) - } - - // make sure cresp is not nil - if cresp == nil { - t.Fatal("expected not nil, got", cresp) - } - }) - - t.Run("with two test helpers where the first one times out the connection and the second works", func(t *testing.T) { - // TODO(bassosimone): the utility of this test will become more obvious - // once we switch this specific test to using httpclientx. - - // create a local test server1 that resets the connection after a ~long delay - server1 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - select { - case <-time.After(10 * time.Second): - testingx.HTTPHandlerReset().ServeHTTP(w, r) - case <-r.Context().Done(): - return - } - })) - defer server1.Close() - - // create a local test server2 that always returns an ~empty response - server2 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte(`{}`)) - })) - defer server2.Close() - - // create a new session only initializing the fields that - // are going to matter for running this specific test - sess := &Session{ - network: enginenetx.NewNetwork( - bytecounter.New(), - &kvstore.Memory{}, - model.DiscardLogger, - nil, - (&netxlite.Netx{}).NewStdlibResolver(model.DiscardLogger), - ), - logger: model.DiscardLogger, - softwareName: "miniooni", - softwareVersion: version.Version, - } - - // create a new background context - ctx := context.Background() - - // create a fake request for the test helper - // - // note: no need to fill the request for this test case - creq := &model.THRequest{} - - // create the list of test helpers to use - testhelpers := []model.OOAPIService{{ - Address: server1.URL, - Type: "https", - Front: "", - }, { - Address: server2.URL, - Type: "https", - Front: "", - }} - - // invoke the API - cresp, idx, err := sess.CallWebConnectivityTestHelper(ctx, creq, testhelpers) - - // make sure we get the expected error - if err != nil { - t.Fatal("unexpected error", err) - } - - // make sure idx is one - if idx != 1 { - t.Fatal("expected one, got", idx) - } - - // make sure cresp is not nil - if cresp == nil { - t.Fatal("expected not nil, got", cresp) - } - }) -} diff --git a/internal/experiment/webconnectivity/control.go b/internal/experiment/webconnectivity/control.go index 47c06eab4b..59f9893356 100644 --- a/internal/experiment/webconnectivity/control.go +++ b/internal/experiment/webconnectivity/control.go @@ -4,11 +4,10 @@ import ( "context" "github.com/ooni/probe-cli/v3/internal/geoipx" - "github.com/ooni/probe-cli/v3/internal/httpapi" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/ooapi" "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/webconnectivityalgo" ) // Redirect to types defined inside the model package @@ -24,12 +23,8 @@ type ( func Control( ctx context.Context, sess model.ExperimentSession, testhelpers []model.OOAPIService, creq ControlRequest) (ControlResponse, *model.OOAPIService, error) { - seqCaller := httpapi.NewSequenceCaller( - ooapi.NewDescriptorTH(&creq), - httpapi.NewEndpointList(sess.DefaultHTTPClient(), sess.Logger(), sess.UserAgent(), testhelpers...)..., - ) sess.Logger().Infof("control for %s...", creq.HTTPRequest) - out, idx, err := seqCaller.Call(ctx) + out, idx, err := webconnectivityalgo.CallWebConnectivityTestHelper(ctx, &creq, testhelpers, sess) sess.Logger().Infof("control for %s... %+v", creq.HTTPRequest, model.ErrorToStringOrOK(err)) if err != nil { // make sure error is wrapped diff --git a/internal/experiment/webconnectivitylte/control.go b/internal/experiment/webconnectivitylte/control.go index e408a92d34..4e0e82212e 100644 --- a/internal/experiment/webconnectivitylte/control.go +++ b/internal/experiment/webconnectivitylte/control.go @@ -8,12 +8,11 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivity" - "github.com/ooni/probe-cli/v3/internal/httpapi" "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/ooapi" "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/webconnectivityalgo" ) // EndpointMeasurementsStarter is used by Control to start extra @@ -109,14 +108,8 @@ func (c *Control) Run(parentCtx context.Context) { c.TestHelpers, ) - // create an httpapi sequence caller - seqCaller := httpapi.NewSequenceCaller( - ooapi.NewDescriptorTH(creq), - httpapi.NewEndpointList(c.Session.DefaultHTTPClient(), c.Logger, c.Session.UserAgent(), c.TestHelpers...)..., - ) - // issue the control request and wait for the response - cresp, idx, err := seqCaller.Call(opCtx) + cresp, idx, err := webconnectivityalgo.CallWebConnectivityTestHelper(opCtx, creq, c.TestHelpers, c.Session) if err != nil { // make sure error is wrapped err = netxlite.NewTopLevelGenericErrWrapper(err) diff --git a/internal/legacy/mockable/mockable.go b/internal/legacy/mockable/mockable.go index 8a96c171c3..2f39a254c0 100644 --- a/internal/legacy/mockable/mockable.go +++ b/internal/legacy/mockable/mockable.go @@ -13,9 +13,6 @@ import ( // // Deprecated: use ./internal/model/mocks.Session instead. type Session struct { - MocakbleCallWCTHResp *model.THResponse - MockableCallWCTHCount int - MockableCallWCTHErr error MockableTestHelpers map[string][]model.OOAPIService MockableHTTPClient model.HTTPClient MockableLogger model.Logger @@ -41,12 +38,6 @@ type Session struct { MockableUserAgent string } -// CallWebConnectivityTestHelper implements [model.EngineExperimentSession]. -func (sess *Session) CallWebConnectivityTestHelper( - ctx context.Context, request *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) { - return sess.MocakbleCallWCTHResp, sess.MockableCallWCTHCount, sess.MockableCallWCTHErr -} - // GetTestHelpersByName implements ExperimentSession.GetTestHelpersByName func (sess *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) { services, okay := sess.MockableTestHelpers[name] diff --git a/internal/mocks/session.go b/internal/mocks/session.go index 613f56c00e..c0750f7169 100644 --- a/internal/mocks/session.go +++ b/internal/mocks/session.go @@ -9,9 +9,6 @@ import ( // Session allows to mock sessions. type Session struct { - MockCallWebConnectivityTestHelper func(ctx context.Context, - req *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) - MockGetTestHelpersByName func(name string) ([]model.OOAPIService, bool) MockDefaultHTTPClient func() model.HTTPClient @@ -61,11 +58,6 @@ type Session struct { config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) } -func (sess *Session) CallWebConnectivityTestHelper(ctx context.Context, - req *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) { - return sess.MockCallWebConnectivityTestHelper(ctx, req, ths) -} - func (sess *Session) GetTestHelpersByName(name string) ([]model.OOAPIService, bool) { return sess.MockGetTestHelpersByName(name) } diff --git a/internal/mocks/session_test.go b/internal/mocks/session_test.go index 25a7b375fd..b794941736 100644 --- a/internal/mocks/session_test.go +++ b/internal/mocks/session_test.go @@ -13,26 +13,6 @@ import ( ) func TestSession(t *testing.T) { - t.Run("CallWebConnectivityTestHelper", func(t *testing.T) { - expect := errors.New("mocked error") - s := &Session{ - MockCallWebConnectivityTestHelper: func( - ctx context.Context, req *model.THRequest, ths []model.OOAPIService) (*model.THResponse, int, error) { - return nil, 0, expect - }, - } - resp, count, err := s.CallWebConnectivityTestHelper(context.Background(), &model.THRequest{}, nil) - if !errors.Is(err, expect) { - t.Fatal("unexpected error", err) - } - if count != 0 { - t.Fatal("expected zero") - } - if resp != nil { - t.Fatal("expected nil") - } - }) - t.Run("GetTestHelpersByName", func(t *testing.T) { var expect []model.OOAPIService ff := &testingx.FakeFiller{} diff --git a/internal/model/experiment.go b/internal/model/experiment.go index cb5ba39faa..410233f6fb 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -15,26 +15,10 @@ var ErrNoAvailableTestHelpers = errors.New("no available helpers") // ExperimentSession is the experiment's view of a session. type ExperimentSession interface { - // CallWebConnectivityTestHelper invokes the Web Connectivity test helper with the - // given request object and the given list of available test helpers. - // - // If the list of test helpers is empty this function immediately returns nil, zero, - // and the [ErrNoAvailableTestHelpers] error to the caller. - // - // In case of any other failure, this function returns nil, zero, and an error - // - // On success, it returns the response, the used TH index, and nil. - // - // Note that the returned error won't be wrapped, so you need to wrap it yourself. - CallWebConnectivityTestHelper( - ctx context.Context, request *THRequest, ths []OOAPIService) (*THResponse, int, error) - // GetTestHelpersByName returns a list of test helpers with the given name. GetTestHelpersByName(name string) ([]OOAPIService, bool) // DefaultHTTPClient returns the default HTTPClient used by the session. - // - // Deprecated: Web Connectivity should use CallWebConnectivityTestHelper instead. DefaultHTTPClient() HTTPClient // FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error. diff --git a/internal/webconnectivityalgo/calltesthelpers.go b/internal/webconnectivityalgo/calltesthelpers.go new file mode 100644 index 0000000000..044c0d7661 --- /dev/null +++ b/internal/webconnectivityalgo/calltesthelpers.go @@ -0,0 +1,50 @@ +package webconnectivityalgo + +import ( + "context" + + "github.com/ooni/probe-cli/v3/internal/httpapi" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/ooapi" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// CallWebConnectivityTestHelper invokes the Web Connectivity test helper with the +// given request object, the given list of available test helpers, and the given session. +// +// If the list of test helpers is empty this function immediately returns nil, zero, +// and the [ErrNoAvailableTestHelpers] error to the caller. +// +// In case of any other failure, this function returns nil, zero, and an error. +// +// On success, it returns the response, the used TH index, and nil. +// +// Note that the returned error won't be wrapped, so you need to wrap it yourself. +func CallWebConnectivityTestHelper(ctx context.Context, creq *model.THRequest, + testhelpers []model.OOAPIService, sess model.ExperimentSession) (*model.THResponse, int, error) { + // handle the case where there are no available web connectivity test helpers + if len(testhelpers) <= 0 { + return nil, 0, model.ErrNoAvailableTestHelpers + } + + // initialize a sequence caller for invoking the THs in FIFO order + seqCaller := httpapi.NewSequenceCaller( + ooapi.NewDescriptorTH(creq), + httpapi.NewEndpointList(sess.DefaultHTTPClient(), sess.Logger(), sess.UserAgent(), testhelpers...)..., + ) + + // issue the composed call proper and obtain a response and an index or an error + cresp, idx, err := seqCaller.Call(ctx) + + // handle the case where all test helpers failed + if err != nil { + return nil, 0, err + } + + // apply some sanity checks to the results + runtimex.Assert(idx >= 0 && idx < len(testhelpers), "idx out of bounds") + runtimex.Assert(cresp != nil, "out is nil") + + // return the results to the web connectivity caller + return cresp, idx, nil +} diff --git a/internal/webconnectivityalgo/calltesthelpers_test.go b/internal/webconnectivityalgo/calltesthelpers_test.go new file mode 100644 index 0000000000..0fa53b6cd3 --- /dev/null +++ b/internal/webconnectivityalgo/calltesthelpers_test.go @@ -0,0 +1,309 @@ +package webconnectivityalgo + +import ( + "context" + "errors" + "net/http" + "testing" + "time" + + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +// This function tests the [CallWebConnectivityTestHelper] function. +func TestSessionCallWebConnectivityTestHelper(t *testing.T) { + // We start with simple tests that exercise the basic functionality of the method + // without bothering with having more than one available test helper. + + t.Run("when there are no available test helpers", func(t *testing.T) { + // create a new session only initializing the fields that + // are going to matter for running this specific test + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockDefaultHTTPClient: func() model.HTTPClient { + return http.DefaultClient + }, + MockUserAgent: func() string { + return model.HTTPHeaderUserAgent + }, + } + + // create a new background context + ctx := context.Background() + + // create a fake request for the test helper + // + // note: no need to fill the request for this test case + creq := &model.THRequest{} + + // invoke the API + cresp, idx, err := CallWebConnectivityTestHelper(ctx, creq, nil, sess) + + // make sure we get the expected error + if !errors.Is(err, model.ErrNoAvailableTestHelpers) { + t.Fatal("unexpected error", err) + } + + // make sure idx is zero + if idx != 0 { + t.Fatal("expected zero, got", idx) + } + + // make sure cresp is nil + if cresp != nil { + t.Fatal("expected nil, got", cresp) + } + }) + + t.Run("when the call fails", func(t *testing.T) { + // create a local test server that always resets the connection + server := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) + defer server.Close() + + // create a new session only initializing the fields that + // are going to matter for running this specific test + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockDefaultHTTPClient: func() model.HTTPClient { + return http.DefaultClient + }, + MockUserAgent: func() string { + return model.HTTPHeaderUserAgent + }, + } + + // create a new background context + ctx := context.Background() + + // create a fake request for the test helper + // + // note: no need to fill the request for this test case + creq := &model.THRequest{} + + // create the list of test helpers to use + testhelpers := []model.OOAPIService{{ + Address: server.URL, + Type: "https", + Front: "", + }} + + // invoke the API + cresp, idx, err := CallWebConnectivityTestHelper(ctx, creq, testhelpers, sess) + + // make sure we get the expected error + if !errors.Is(err, netxlite.ECONNRESET) { + t.Fatal("unexpected error", err) + } + + // make sure idx is zero + if idx != 0 { + t.Fatal("expected zero, got", idx) + } + + // make sure cresp is nil + if cresp != nil { + t.Fatal("expected nil, got", cresp) + } + }) + + t.Run("when the call succeeds", func(t *testing.T) { + // create a local test server that always returns an ~empty response + server := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{}`)) + })) + defer server.Close() + + // create a new session only initializing the fields that + // are going to matter for running this specific test + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockDefaultHTTPClient: func() model.HTTPClient { + return http.DefaultClient + }, + MockUserAgent: func() string { + return model.HTTPHeaderUserAgent + }, + } + + // create a new background context + ctx := context.Background() + + // create a fake request for the test helper + // + // note: no need to fill the request for this test case + creq := &model.THRequest{} + + // create the list of test helpers to use + testhelpers := []model.OOAPIService{{ + Address: server.URL, + Type: "https", + Front: "", + }} + + // invoke the API + cresp, idx, err := CallWebConnectivityTestHelper(ctx, creq, testhelpers, sess) + + // make sure we get the expected error + if err != nil { + t.Fatal("unexpected error", err) + } + + // make sure idx is zero + if idx != 0 { + t.Fatal("expected zero, got", idx) + } + + // make sure cresp is not nil + if cresp == nil { + t.Fatal("expected not nil, got", cresp) + } + }) + + // Now we include some tests that ensure that we can chain (in more or less + // smart fashion) API calls using multiple test helper endpoints. + + t.Run("with two test helpers where the first one resets the connection and the second works", func(t *testing.T) { + // create a local test server1 that always resets the connection + server1 := testingx.MustNewHTTPServer(testingx.HTTPHandlerReset()) + defer server1.Close() + + // create a local test server2 that always returns an ~empty response + server2 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{}`)) + })) + defer server2.Close() + + // create a new session only initializing the fields that + // are going to matter for running this specific test + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockDefaultHTTPClient: func() model.HTTPClient { + return http.DefaultClient + }, + MockUserAgent: func() string { + return model.HTTPHeaderUserAgent + }, + } + + // create a new background context + ctx := context.Background() + + // create a fake request for the test helper + // + // note: no need to fill the request for this test case + creq := &model.THRequest{} + + // create the list of test helpers to use + testhelpers := []model.OOAPIService{{ + Address: server1.URL, + Type: "https", + Front: "", + }, { + Address: server2.URL, + Type: "https", + Front: "", + }} + + // invoke the API + cresp, idx, err := CallWebConnectivityTestHelper(ctx, creq, testhelpers, sess) + + // make sure we get the expected error + if err != nil { + t.Fatal("unexpected error", err) + } + + // make sure idx is one + if idx != 1 { + t.Fatal("expected one, got", idx) + } + + // make sure cresp is not nil + if cresp == nil { + t.Fatal("expected not nil, got", cresp) + } + }) + + t.Run("with two test helpers where the first one times out the connection and the second works", func(t *testing.T) { + // TODO(bassosimone): the utility of this test will become more obvious + // once we switch this specific test to using httpclientx. + + // create a local test server1 that resets the connection after a ~long delay + server1 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + select { + case <-time.After(10 * time.Second): + testingx.HTTPHandlerReset().ServeHTTP(w, r) + case <-r.Context().Done(): + return + } + })) + defer server1.Close() + + // create a local test server2 that always returns an ~empty response + server2 := testingx.MustNewHTTPServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{}`)) + })) + defer server2.Close() + + // create a new session only initializing the fields that + // are going to matter for running this specific test + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockDefaultHTTPClient: func() model.HTTPClient { + return http.DefaultClient + }, + MockUserAgent: func() string { + return model.HTTPHeaderUserAgent + }, + } + + // create a new background context + ctx := context.Background() + + // create a fake request for the test helper + // + // note: no need to fill the request for this test case + creq := &model.THRequest{} + + // create the list of test helpers to use + testhelpers := []model.OOAPIService{{ + Address: server1.URL, + Type: "https", + Front: "", + }, { + Address: server2.URL, + Type: "https", + Front: "", + }} + + // invoke the API + cresp, idx, err := CallWebConnectivityTestHelper(ctx, creq, testhelpers, sess) + + // make sure we get the expected error + if err != nil { + t.Fatal("unexpected error", err) + } + + // make sure idx is one + if idx != 1 { + t.Fatal("expected one, got", idx) + } + + // make sure cresp is not nil + if cresp == nil { + t.Fatal("expected not nil, got", cresp) + } + }) +}