diff --git a/internal/engine/experiment/tor/tor.go b/internal/engine/experiment/tor/tor.go index ee409cc..09bc51f 100644 --- a/internal/engine/experiment/tor/tor.go +++ b/internal/engine/experiment/tor/tor.go @@ -55,6 +55,11 @@ type TargetResults struct { TargetSource string `json:"target_source,omitempty"` TCPConnect oonidatamodel.TCPConnectList `json:"tcp_connect"` TLSHandshakes oonidatamodel.TLSHandshakesList `json:"tls_handshakes"` + + // Only for testing. We don't care about this field otherwise. We + // cannot make this private because otherwise the IP address sanitizer + // is going to panic over a private field. + DirPortCount int `json:"-"` } func registerExtensions(m *model.Measurement) { @@ -78,6 +83,7 @@ func (tr *TargetResults) fillSummary() { case "dir_port": // The UI currently doesn't care about this protocol // as long as drawing a table is concerned. + tr.DirPortCount++ case "obfs4": // We currently only perform an OBFS4 handshake, hence // the final Failure is the handshake result @@ -136,20 +142,16 @@ func (tk *TestKeys) fillToplevelKeys() { // Measurer performs the measurement. type Measurer struct { - config Config - fetchTorTargets func(ctx context.Context, clnt model.ExperimentOrchestraClient, cc string) (map[string]model.TorTarget, error) - newOrchestraClient func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) + config Config + fetchTorTargets func(ctx context.Context, sess model.ExperimentSession, cc string) (map[string]model.TorTarget, error) } // NewMeasurer creates a new Measurer func NewMeasurer(config Config) *Measurer { return &Measurer{ config: config, - fetchTorTargets: func(ctx context.Context, clnt model.ExperimentOrchestraClient, cc string) (map[string]model.TorTarget, error) { - return clnt.FetchTorTargets(ctx, cc) - }, - newOrchestraClient: func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) { - return sess.NewOrchestraClient(ctx) + fetchTorTargets: func(ctx context.Context, sess model.ExperimentSession, cc string) (map[string]model.TorTarget, error) { + return sess.FetchTorTargets(ctx, cc) }, } } @@ -189,11 +191,7 @@ func (m *Measurer) gimmeTargets( ) (map[string]model.TorTarget, error) { ctx, cancel := context.WithTimeout(ctx, 15*time.Second) defer cancel() - clnt, err := m.newOrchestraClient(ctx, sess) - if err != nil { - return nil, err - } - return m.fetchTorTargets(ctx, clnt, sess.ProbeCC()) + return m.fetchTorTargets(ctx, sess, sess.ProbeCC()) } // keytarget contains a key and the related target diff --git a/internal/engine/experiment/tor/tor_test.go b/internal/engine/experiment/tor/tor_test.go index 9972b59..01eb2da 100644 --- a/internal/engine/experiment/tor/tor_test.go +++ b/internal/engine/experiment/tor/tor_test.go @@ -18,7 +18,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/legacy/oonitemplates" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx/errorx" - "github.com/ooni/probe-cli/v3/internal/engine/probeservices" ) func TestNewExperimentMeasurer(t *testing.T) { @@ -31,32 +30,10 @@ func TestNewExperimentMeasurer(t *testing.T) { } } -func TestMeasurerMeasureNewOrchestraClientError(t *testing.T) { - measurer := NewMeasurer(Config{}) - expected := errors.New("mocked error") - measurer.newOrchestraClient = func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) { - return nil, expected - } - err := measurer.Run( - context.Background(), - &mockable.Session{ - MockableLogger: log.Log, - }, - new(model.Measurement), - model.NewPrinterCallbacks(log.Log), - ) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected") - } -} - func TestMeasurerMeasureFetchTorTargetsError(t *testing.T) { measurer := NewMeasurer(Config{}) expected := errors.New("mocked error") - measurer.newOrchestraClient = func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) { - return new(probeservices.Client), nil - } - measurer.fetchTorTargets = func(ctx context.Context, clnt model.ExperimentOrchestraClient, cc string) (map[string]model.TorTarget, error) { + measurer.fetchTorTargets = func(ctx context.Context, sess model.ExperimentSession, cc string) (map[string]model.TorTarget, error) { return nil, expected } err := measurer.Run( @@ -74,10 +51,7 @@ func TestMeasurerMeasureFetchTorTargetsError(t *testing.T) { func TestMeasurerMeasureFetchTorTargetsEmptyList(t *testing.T) { measurer := NewMeasurer(Config{}) - measurer.newOrchestraClient = func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) { - return new(probeservices.Client), nil - } - measurer.fetchTorTargets = func(ctx context.Context, clnt model.ExperimentOrchestraClient, cc string) (map[string]model.TorTarget, error) { + measurer.fetchTorTargets = func(ctx context.Context, sess model.ExperimentSession, cc string) (map[string]model.TorTarget, error) { return nil, nil } measurement := new(model.Measurement) @@ -102,10 +76,7 @@ func TestMeasurerMeasureGoodWithMockedOrchestra(t *testing.T) { // This test mocks orchestra to return a nil list of targets, so the code runs // but we don't perform any actual network actions. measurer := NewMeasurer(Config{}) - measurer.newOrchestraClient = func(ctx context.Context, sess model.ExperimentSession) (model.ExperimentOrchestraClient, error) { - return new(probeservices.Client), nil - } - measurer.fetchTorTargets = func(ctx context.Context, clnt model.ExperimentOrchestraClient, cc string) (map[string]model.TorTarget, error) { + measurer.fetchTorTargets = func(ctx context.Context, sess model.ExperimentSession, cc string) (map[string]model.TorTarget, error) { return nil, nil } err := measurer.Run( @@ -167,10 +138,8 @@ func TestMeasurerMeasureSanitiseOutput(t *testing.T) { measurer := NewMeasurer(Config{}) sess := newsession() key := "xyz-xyz-xyz-theCh2ju-ahG4chei-Ai2eka0a" - sess.MockableOrchestraClient = &mockable.ExperimentOrchestraClient{ - MockableFetchTorTargetsResult: map[string]model.TorTarget{ - key: staticPrivateTestingTarget, - }, + sess.MockableFetchTorTargetsResult = map[string]model.TorTarget{ + key: staticPrivateTestingTarget, } measurement := new(model.Measurement) err := measurer.Run( @@ -929,3 +898,45 @@ func TestSummaryKeysWorksAsIntended(t *testing.T) { }) } } + +func TestTargetResultsFillSummaryDirPort(t *testing.T) { + tr := &TargetResults{ + TargetProtocol: "dir_port", + TCPConnect: oonidatamodel.TCPConnectList{{ + IP: "1.2.3.4", + Port: 443, + Status: oonidatamodel.TCPConnectStatus{ + Failure: nil, + }, + }}, + } + tr.fillSummary() + if tr.DirPortCount != 1 { + t.Fatal("unexpected dirPortCount") + } +} + +func TestTestKeysFillToplevelKeysCoverMissingFields(t *testing.T) { + failureString := "eof_error" + tk := &TestKeys{ + Targets: map[string]TargetResults{ + "foobar": {Failure: &failureString, TargetProtocol: "dir_port"}, + "baz": {TargetProtocol: "dir_port"}, + "jafar": {Failure: &failureString, TargetProtocol: "or_port_dirauth"}, + "jasmine": {TargetProtocol: "or_port_dirauth"}, + }, + } + tk.fillToplevelKeys() + if tk.DirPortTotal != 2 { + t.Fatal("unexpected DirPortTotal") + } + if tk.DirPortAccessible != 1 { + t.Fatal("unexpected DirPortAccessible") + } + if tk.ORPortDirauthTotal != 2 { + t.Fatal("unexpected ORPortDirauthTotal") + } + if tk.ORPortDirauthAccessible != 1 { + t.Fatal("unexpected ORPortDirauthAccessible") + } +} diff --git a/internal/engine/internal/mockable/mockable.go b/internal/engine/internal/mockable/mockable.go index 7c606b2..6e83217 100644 --- a/internal/engine/internal/mockable/mockable.go +++ b/internal/engine/internal/mockable/mockable.go @@ -12,30 +12,32 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/kvstore" "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/probeservices" - "github.com/ooni/probe-cli/v3/internal/engine/probeservices/testorchestra" - "github.com/ooni/probe-cli/v3/internal/engine/runtimex" ) // Session allows to mock sessions. type Session struct { - MockableTestHelpers map[string][]model.Service - MockableHTTPClient *http.Client - MockableLogger model.Logger - MockableMaybeResolverIP string - MockableOrchestraClient model.ExperimentOrchestraClient - MockableOrchestraClientError error - MockableProbeASNString string - MockableProbeCC string - MockableProbeIP string - MockableProbeNetworkName string - MockableProxyURL *url.URL - MockableResolverIP string - MockableSoftwareName string - MockableSoftwareVersion string - MockableTempDir string - MockableTorArgs []string - MockableTorBinary string - MockableUserAgent string + MockableTestHelpers map[string][]model.Service + MockableHTTPClient *http.Client + MockableLogger model.Logger + MockableMaybeResolverIP string + MockableProbeASNString string + MockableProbeCC string + MockableProbeIP string + MockableProbeNetworkName string + MockableProxyURL *url.URL + MockableFetchPsiphonConfigResult []byte + MockableFetchPsiphonConfigErr error + MockableFetchTorTargetsResult map[string]model.TorTarget + MockableFetchTorTargetsErr error + MockableFetchURLListResult []model.URLInfo + MockableFetchURLListErr error + MockableResolverIP string + MockableSoftwareName string + MockableSoftwareVersion string + MockableTempDir string + MockableTorArgs []string + MockableTorBinary string + MockableUserAgent string } // GetTestHelpersByName implements ExperimentSession.GetTestHelpersByName @@ -49,6 +51,23 @@ func (sess *Session) DefaultHTTPClient() *http.Client { return sess.MockableHTTPClient } +// FetchPsiphonConfig implements ExperimentSession.FetchPsiphonConfig +func (sess *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) { + return sess.MockableFetchPsiphonConfigResult, sess.MockableFetchPsiphonConfigErr +} + +// FetchTorTargets implements ExperimentSession.TorTargets +func (sess *Session) FetchTorTargets( + ctx context.Context, cc string) (map[string]model.TorTarget, error) { + return sess.MockableFetchTorTargetsResult, sess.MockableFetchTorTargetsErr +} + +// FetchURLList implements ExperimentSession.FetchURLList. +func (sess *Session) FetchURLList( + ctx context.Context, config model.URLListConfig) ([]model.URLInfo, error) { + return sess.MockableFetchURLListResult, sess.MockableFetchURLListErr +} + // KeyValueStore returns the configured key-value store. func (sess *Session) KeyValueStore() model.KeyValueStore { return kvstore.NewMemoryKeyValueStore() @@ -64,29 +83,6 @@ func (sess *Session) MaybeResolverIP() string { return sess.MockableMaybeResolverIP } -// NewOrchestraClient implements ExperimentSession.NewOrchestraClient -func (sess *Session) NewOrchestraClient(ctx context.Context) (model.ExperimentOrchestraClient, error) { - if sess.MockableOrchestraClient != nil { - return sess.MockableOrchestraClient, nil - } - if sess.MockableOrchestraClientError != nil { - return nil, sess.MockableOrchestraClientError - } - clnt, err := probeservices.NewClient(sess, model.Service{ - Address: "https://ams-pg-test.ooni.org/", - Type: "https", - }) - runtimex.PanicOnError(err, "orchestra.NewClient should not fail here") - meta := testorchestra.MetadataFixture() - if err := clnt.MaybeRegister(ctx, meta); err != nil { - return nil, err - } - if err := clnt.MaybeLogin(ctx); err != nil { - return nil, err - } - return clnt, nil -} - // ProbeASNString implements ExperimentSession.ProbeASNString func (sess *Session) ProbeASNString() string { return sess.MockableProbeASNString @@ -152,42 +148,3 @@ var _ probeservices.Session = &Session{} var _ psiphonx.Session = &Session{} var _ tunnel.Session = &Session{} var _ torx.Session = &Session{} - -// ExperimentOrchestraClient is the experiment's view of -// a client for querying the OONI orchestra. -type ExperimentOrchestraClient struct { - MockableCheckInInfo *model.CheckInInfo - MockableCheckInErr error - MockableFetchPsiphonConfigResult []byte - MockableFetchPsiphonConfigErr error - MockableFetchTorTargetsResult map[string]model.TorTarget - MockableFetchTorTargetsErr error - MockableFetchURLListResult []model.URLInfo - MockableFetchURLListErr error -} - -// CheckIn implements ExperimentOrchestraClient.CheckIn. -func (c ExperimentOrchestraClient) CheckIn( - ctx context.Context, config model.CheckInConfig) (*model.CheckInInfo, error) { - return c.MockableCheckInInfo, c.MockableCheckInErr -} - -// FetchPsiphonConfig implements ExperimentOrchestraClient.FetchPsiphonConfig -func (c ExperimentOrchestraClient) FetchPsiphonConfig( - ctx context.Context) ([]byte, error) { - return c.MockableFetchPsiphonConfigResult, c.MockableFetchPsiphonConfigErr -} - -// FetchTorTargets implements ExperimentOrchestraClient.TorTargets -func (c ExperimentOrchestraClient) FetchTorTargets( - ctx context.Context, cc string) (map[string]model.TorTarget, error) { - return c.MockableFetchTorTargetsResult, c.MockableFetchTorTargetsErr -} - -// FetchURLList implements ExperimentOrchestraClient.FetchURLList. -func (c ExperimentOrchestraClient) FetchURLList( - ctx context.Context, config model.URLListConfig) ([]model.URLInfo, error) { - return c.MockableFetchURLListResult, c.MockableFetchURLListErr -} - -var _ model.ExperimentOrchestraClient = ExperimentOrchestraClient{} diff --git a/internal/engine/internal/psiphonx/psiphonx.go b/internal/engine/internal/psiphonx/psiphonx.go index c7ba9c1..11cbae0 100644 --- a/internal/engine/internal/psiphonx/psiphonx.go +++ b/internal/engine/internal/psiphonx/psiphonx.go @@ -10,13 +10,12 @@ import ( "path/filepath" "time" - "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" ) // Session is the way in which this package sees a Session. type Session interface { - NewOrchestraClient(ctx context.Context) (model.ExperimentOrchestraClient, error) + FetchPsiphonConfig(ctx context.Context) ([]byte, error) TempDir() string } @@ -87,11 +86,7 @@ func Start( if config.WorkDir == "" { config.WorkDir = sess.TempDir() } - clnt, err := sess.NewOrchestraClient(ctx) - if err != nil { - return nil, err - } - configJSON, err := clnt.FetchPsiphonConfig(ctx) + configJSON, err := sess.FetchPsiphonConfig(ctx) if err != nil { return nil, err } diff --git a/internal/engine/internal/psiphonx/psiphonx_test.go b/internal/engine/internal/psiphonx/psiphonx_test.go index 427820b..9f69b17 100644 --- a/internal/engine/internal/psiphonx/psiphonx_test.go +++ b/internal/engine/internal/psiphonx/psiphonx_test.go @@ -58,27 +58,10 @@ func TestStartStop(t *testing.T) { tunnel.Stop() } -func TestNewOrchestraClientFailure(t *testing.T) { - expected := errors.New("mocked error") - sess := &mockable.Session{ - MockableOrchestraClientError: expected, - } - tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{}) - if !errors.Is(err, expected) { - t.Fatal("not the error we expected") - } - if tunnel != nil { - t.Fatal("expected nil tunnel here") - } -} - func TestFetchPsiphonConfigFailure(t *testing.T) { expected := errors.New("mocked error") - clnt := mockable.ExperimentOrchestraClient{ - MockableFetchPsiphonConfigErr: expected, - } sess := &mockable.Session{ - MockableOrchestraClient: clnt, + MockableFetchPsiphonConfigErr: expected, } tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{}) if !errors.Is(err, expected) { @@ -94,11 +77,8 @@ func TestMakeMkdirAllFailure(t *testing.T) { dependencies := FakeDependencies{ MkdirAllErr: expected, } - clnt := mockable.ExperimentOrchestraClient{ - MockableFetchPsiphonConfigResult: []byte(`{}`), - } sess := &mockable.Session{ - MockableOrchestraClient: clnt, + MockableFetchPsiphonConfigResult: []byte(`{}`), } tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{ Dependencies: dependencies, @@ -116,11 +96,8 @@ func TestMakeRemoveAllFailure(t *testing.T) { dependencies := FakeDependencies{ RemoveAllErr: expected, } - clnt := mockable.ExperimentOrchestraClient{ - MockableFetchPsiphonConfigResult: []byte(`{}`), - } sess := &mockable.Session{ - MockableOrchestraClient: clnt, + MockableFetchPsiphonConfigResult: []byte(`{}`), } tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{ Dependencies: dependencies, @@ -138,11 +115,8 @@ func TestMakeStartFailure(t *testing.T) { dependencies := FakeDependencies{ StartErr: expected, } - clnt := mockable.ExperimentOrchestraClient{ - MockableFetchPsiphonConfigResult: []byte(`{}`), - } sess := &mockable.Session{ - MockableOrchestraClient: clnt, + MockableFetchPsiphonConfigResult: []byte(`{}`), } tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{ Dependencies: dependencies, diff --git a/internal/engine/internal/sessionresolver/childresolver_test.go b/internal/engine/internal/sessionresolver/childresolver_test.go index 8be08ff..888e636 100644 --- a/internal/engine/internal/sessionresolver/childresolver_test.go +++ b/internal/engine/internal/sessionresolver/childresolver_test.go @@ -62,7 +62,7 @@ func TestTimeLimitedLookupFailure(t *testing.T) { func TestTimeLimitedLookupWillTimeout(t *testing.T) { if testing.Short() { - t.Skip("skipping test in short mode") + t.Skip("skip test in short mode") } reso := &Resolver{} re := &FakeResolver{ diff --git a/internal/engine/internal/sessionresolver/integration_test.go b/internal/engine/internal/sessionresolver/integration_test.go index f359aca..52d9fe0 100644 --- a/internal/engine/internal/sessionresolver/integration_test.go +++ b/internal/engine/internal/sessionresolver/integration_test.go @@ -9,7 +9,7 @@ import ( func TestSessionResolverGood(t *testing.T) { if testing.Short() { - t.Skip("skipping test in short mode") + t.Skip("skip test in short mode") } reso := &sessionresolver.Resolver{} defer reso.CloseIdleConnections() diff --git a/internal/engine/model/experiment.go b/internal/engine/model/experiment.go index 25e39e1..a6ed445 100644 --- a/internal/engine/model/experiment.go +++ b/internal/engine/model/experiment.go @@ -5,29 +5,14 @@ import ( "net/http" ) -// ExperimentOrchestraClient is the experiment's view of -// a client for querying the OONI orchestra API. -type ExperimentOrchestraClient interface { - // CheckIn calls the check-in API. - CheckIn(ctx context.Context, config CheckInConfig) (*CheckInInfo, error) - - // FetchPsiphonConfig returns psiphon config from the API. - FetchPsiphonConfig(ctx context.Context) ([]byte, error) - - // FetchTorTargets returns tor targets from the API. - FetchTorTargets(ctx context.Context, cc string) (map[string]TorTarget, error) - - // FetchURLList returns URLs from the API. - // This method is deprecated and will be removed soon. - FetchURLList(ctx context.Context, config URLListConfig) ([]URLInfo, error) -} - // ExperimentSession is the experiment's view of a session. type ExperimentSession interface { GetTestHelpersByName(name string) ([]Service, bool) DefaultHTTPClient() *http.Client + FetchPsiphonConfig(ctx context.Context) ([]byte, error) + FetchTorTargets(ctx context.Context, cc string) (map[string]TorTarget, error) + FetchURLList(ctx context.Context, config URLListConfig) ([]URLInfo, error) Logger() Logger - NewOrchestraClient(ctx context.Context) (ExperimentOrchestraClient, error) ProbeCC() string ResolverIP() string TempDir() string diff --git a/internal/engine/netx/archival/archival_test.go b/internal/engine/netx/archival/archival_test.go index 5172ed6..7fdc471 100644 --- a/internal/engine/netx/archival/archival_test.go +++ b/internal/engine/netx/archival/archival_test.go @@ -129,7 +129,7 @@ func TestNewRequestList(t *testing.T) { }, { Name: "http_response_metadata", HTTPHeaders: http.Header{ - "Server": []string{"orchestra/0.1.0-dev"}, + "Server": []string{"miniooni/0.1.0-dev"}, }, HTTPStatusCode: 200, }, { @@ -194,11 +194,11 @@ func TestNewRequestList(t *testing.T) { HeadersList: []archival.HTTPHeader{{ Key: "Server", Value: archival.MaybeBinaryValue{ - Value: "orchestra/0.1.0-dev", + Value: "miniooni/0.1.0-dev", }, }}, Headers: map[string]archival.MaybeBinaryValue{ - "Server": {Value: "orchestra/0.1.0-dev"}, + "Server": {Value: "miniooni/0.1.0-dev"}, }, Locations: nil, }, @@ -223,7 +223,7 @@ func TestNewRequestList(t *testing.T) { }, { Name: "http_response_metadata", HTTPHeaders: http.Header{ - "Server": []string{"orchestra/0.1.0-dev"}, + "Server": []string{"miniooni/0.1.0-dev"}, "Location": []string{"https://x.example.com", "https://y.example.com"}, }, HTTPStatusCode: 302, @@ -260,11 +260,11 @@ func TestNewRequestList(t *testing.T) { }, { Key: "Server", Value: archival.MaybeBinaryValue{ - Value: "orchestra/0.1.0-dev", + Value: "miniooni/0.1.0-dev", }, }}, Headers: map[string]archival.MaybeBinaryValue{ - "Server": {Value: "orchestra/0.1.0-dev"}, + "Server": {Value: "miniooni/0.1.0-dev"}, "Location": {Value: "https://x.example.com"}, }, Locations: []string{ diff --git a/internal/engine/session.go b/internal/engine/session.go index af165e1..30371aa 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -275,6 +275,35 @@ func (s *Session) DefaultHTTPClient() *http.Client { return &http.Client{Transport: s.httpDefaultTransport} } +// FetchPsiphonConfig fetches psiphon config from the API. +func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) { + clnt, err := s.NewOrchestraClient(ctx) + if err != nil { + return nil, err + } + return clnt.FetchPsiphonConfig(ctx) +} + +// FetchTorTargets fetches tor targets from the API. +func (s *Session) FetchTorTargets( + ctx context.Context, cc string) (map[string]model.TorTarget, error) { + clnt, err := s.NewOrchestraClient(ctx) + if err != nil { + return nil, err + } + return clnt.FetchTorTargets(ctx, cc) +} + +// FetchURLList fetches the URL list from the API. +func (s *Session) FetchURLList( + ctx context.Context, config model.URLListConfig) ([]model.URLInfo, error) { + clnt, err := s.NewOrchestraClient(ctx) + if err != nil { + return nil, err + } + return clnt.FetchURLList(ctx, config) +} + // KeyValueStore returns the configured key-value store. func (s *Session) KeyValueStore() model.KeyValueStore { return s.kvStore @@ -387,7 +416,10 @@ func (s *Session) NewSubmitter(ctx context.Context) (Submitter, error) { // NewOrchestraClient creates a new orchestra client. This client is registered // and logged in with the OONI orchestra. An error is returned on failure. -func (s *Session) NewOrchestraClient(ctx context.Context) (model.ExperimentOrchestraClient, error) { +// +// This function is DEPRECATED. New code SHOULD NOT use it. It will eventually +// be made private or entirely removed from the codebase. +func (s *Session) NewOrchestraClient(ctx context.Context) (*probeservices.Client, error) { clnt, err := s.NewProbeServicesClient(ctx) if err != nil { return nil, err diff --git a/internal/engine/session_integration_test.go b/internal/engine/session_integration_test.go index 08aa198..e7954b4 100644 --- a/internal/engine/session_integration_test.go +++ b/internal/engine/session_integration_test.go @@ -166,21 +166,6 @@ func newSessionForTesting(t *testing.T) *Session { return sess } -func TestNewOrchestraClient(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess := newSessionForTestingNoLookups(t) - defer sess.Close() - clnt, err := sess.NewOrchestraClient(context.Background()) - if err != nil { - t.Fatal(err) - } - if clnt == nil { - t.Fatal("expected non nil client here") - } -} - func TestInitOrchestraClientMaybeRegisterError(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") @@ -634,3 +619,17 @@ func TestSessionNewSubmitterReturnsNonNilSubmitter(t *testing.T) { t.Fatal("expected non nil submitter here") } } + +func TestSessionFetchURLList(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + sess := newSessionForTesting(t) + resp, err := sess.FetchURLList(context.Background(), model.URLListConfig{}) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("expected non-nil response here") + } +} diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go index e13e60b..e254e73 100644 --- a/internal/engine/session_internal_test.go +++ b/internal/engine/session_internal_test.go @@ -208,3 +208,42 @@ func TestSessionMaybeLookupLocationContextLookupLocationContextFailure(t *testin t.Fatal("not the error we expected", err) } } + +func TestSessionFetchURLListWithCancelledContext(t *testing.T) { + sess := &Session{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cause failure + resp, err := sess.FetchURLList(ctx, model.URLListConfig{}) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if resp != nil { + t.Fatal("expected nil response here") + } +} + +func TestSessionFetchTorTargetsWithCancelledContext(t *testing.T) { + sess := &Session{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cause failure + resp, err := sess.FetchTorTargets(ctx, "IT") + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if resp != nil { + t.Fatal("expected nil response here") + } +} + +func TestSessionFetchPsiphonConfigWithCancelledContext(t *testing.T) { + sess := &Session{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cause failure + resp, err := sess.FetchPsiphonConfig(ctx) + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if resp != nil { + t.Fatal("expected nil response here") + } +}