diff --git a/cmd/ooniprobe/internal/nettests/dnscheck.go b/cmd/ooniprobe/internal/nettests/dnscheck.go index 4fe7de5fbd..44c7f86e2a 100644 --- a/cmd/ooniprobe/internal/nettests/dnscheck.go +++ b/cmd/ooniprobe/internal/nettests/dnscheck.go @@ -3,7 +3,7 @@ package nettests import ( "context" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -11,7 +11,7 @@ import ( type DNSCheck struct{} func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + inputloader := &inputloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, diff --git a/cmd/ooniprobe/internal/nettests/stunreachability.go b/cmd/ooniprobe/internal/nettests/stunreachability.go index c09ae23cb2..f77fe3ea84 100644 --- a/cmd/ooniprobe/internal/nettests/stunreachability.go +++ b/cmd/ooniprobe/internal/nettests/stunreachability.go @@ -3,7 +3,7 @@ package nettests import ( "context" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -11,7 +11,7 @@ import ( type STUNReachability struct{} func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + inputloader := &inputloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, diff --git a/cmd/ooniprobe/internal/nettests/web_connectivity.go b/cmd/ooniprobe/internal/nettests/web_connectivity.go index 5d2fc939ea..9131e811a9 100644 --- a/cmd/ooniprobe/internal/nettests/web_connectivity.go +++ b/cmd/ooniprobe/internal/nettests/web_connectivity.go @@ -4,12 +4,12 @@ import ( "context" "github.com/apex/log" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + inputloader := &inputloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ // Setting Charging and OnWiFi to true causes the CheckIn // API to return to us as much URL as possible with the diff --git a/internal/engine/inputloader_network_test.go b/internal/engine/inputloader_integration_test.go similarity index 63% rename from internal/engine/inputloader_network_test.go rename to internal/engine/inputloader_integration_test.go index 040e85ac20..a5a0bad598 100644 --- a/internal/engine/inputloader_network_test.go +++ b/internal/engine/inputloader_integration_test.go @@ -6,10 +6,17 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" ) +// This historical integration test ensures that we're able to fetch URLs from +// the dev infrastructure. We say this test's historical because the inputloading.Loader +// belonged to the engine package before we introduced richer input. It kind of feels +// good to keep this integration test here since we want to use a real session and a real +// Loader and double check whether we can get inputs. In a more distant future it would +// kind of make sense to have a broader package with this kind of integration tests. func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") @@ -29,7 +36,7 @@ func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { t.Fatal(err) } defer sess.Close() - il := &engine.InputLoader{ + il := &inputloading.Loader{ InputPolicy: model.InputOrQueryBackend, Session: sess, } diff --git a/internal/engine/inputloader.go b/internal/inputloading/inputloading.go similarity index 89% rename from internal/engine/inputloader.go rename to internal/inputloading/inputloading.go index 06c31b5d57..3b339cceaf 100644 --- a/internal/engine/inputloader.go +++ b/internal/inputloading/inputloading.go @@ -1,4 +1,5 @@ -package engine +// Package inputloading contains common code to load input. +package inputloading import ( "bufio" @@ -16,7 +17,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/stuninput" ) -// These errors are returned by the InputLoader. +// These errors are returned by the [*Loader]. var ( ErrNoURLsReturned = errors.New("no URLs returned") ErrDetectedEmptyFile = errors.New("file did not contain any input") @@ -25,21 +26,20 @@ var ( ErrNoStaticInput = errors.New("no static input for this experiment") ) -// InputLoaderSession is the session according to an InputLoader. We -// introduce this abstraction because it helps us with testing. -type InputLoaderSession interface { +// Session is the session according to a [*Loader]. +type Session interface { CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) } -// InputLoaderLogger is the logger according to an InputLoader. -type InputLoaderLogger interface { +// Logger is the [model.Logger] according to a [*Loader]. +type Logger interface { // Warnf formats and emits a warning message. Warnf(format string, v ...interface{}) } -// InputLoader loads input according to the specified policy +// Loader loads input according to the specified policy // either from command line and input files or from OONI services. The // behaviour depends on the input policy as described below. // @@ -74,7 +74,7 @@ type InputLoaderLogger interface { // // We gather input from StaticInput and SourceFiles. If there is // input, we return it. Otherwise, we return an error. -type InputLoader struct { +type Loader struct { // CheckInConfig contains options for the CheckIn API. If // not set, then we'll create a default config. If set but // there are fields inside it that are not set, then we @@ -91,14 +91,14 @@ type InputLoader struct { // this field. InputPolicy model.InputPolicy - // Logger is the optional logger that the InputLoader + // Logger is the optional logger that the [*Loader] // should be using. If not set, we will use the default // logger of github.com/apex/log. - Logger InputLoaderLogger + Logger Logger // Session is the current measurement session. You // MUST fill in this field. - Session InputLoaderSession + Session Session // StaticInputs contains optional input to be added // to the resulting input list if possible. @@ -113,7 +113,7 @@ type InputLoader struct { // Load attempts to load input using the specified input loader. We will // return a list of URLs because this is the only input we support. -func (il *InputLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { switch il.InputPolicy { case model.InputOptional: return il.loadOptional() @@ -129,7 +129,7 @@ func (il *InputLoader) Load(ctx context.Context) ([]model.ExperimentTarget, erro } // loadNone implements the InputNone policy. -func (il *InputLoader) loadNone() ([]model.ExperimentTarget, error) { +func (il *Loader) loadNone() ([]model.ExperimentTarget, error) { if len(il.StaticInputs) > 0 || len(il.SourceFiles) > 0 { return nil, ErrNoInputExpected } @@ -140,7 +140,7 @@ func (il *InputLoader) loadNone() ([]model.ExperimentTarget, error) { } // loadOptional implements the InputOptional policy. -func (il *InputLoader) loadOptional() ([]model.ExperimentTarget, error) { +func (il *Loader) loadOptional() ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err == nil && len(inputs) <= 0 { // Implementation note: the convention for input-less experiments is that @@ -151,7 +151,7 @@ func (il *InputLoader) loadOptional() ([]model.ExperimentTarget, error) { } // loadStrictlyRequired implements the InputStrictlyRequired policy. -func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadStrictlyRequired(_ context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -160,7 +160,7 @@ func (il *InputLoader) loadStrictlyRequired(_ context.Context) ([]model.Experime } // loadOrQueryBackend implements the InputOrQueryBackend policy. -func (il *InputLoader) loadOrQueryBackend(ctx context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadOrQueryBackend(ctx context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -247,7 +247,7 @@ func staticInputForExperiment(name string) ([]model.ExperimentTarget, error) { } // loadOrStaticDefault implements the InputOrStaticDefault policy. -func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadOrStaticDefault(_ context.Context) ([]model.ExperimentTarget, error) { inputs, err := il.loadLocal() if err != nil || len(inputs) > 0 { return inputs, err @@ -256,7 +256,7 @@ func (il *InputLoader) loadOrStaticDefault(_ context.Context) ([]model.Experimen } // loadLocal loads inputs from StaticInputs and SourceFiles. -func (il *InputLoader) loadLocal() ([]model.ExperimentTarget, error) { +func (il *Loader) loadLocal() ([]model.ExperimentTarget, error) { inputs := []model.ExperimentTarget{} for _, input := range il.StaticInputs { inputs = append(inputs, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input)) @@ -280,7 +280,7 @@ type inputLoaderOpenFn func(filepath string) (fs.File, error) // readfile reads inputs from the specified file. The open argument should be // compatible with stdlib's fs.Open and helps us with unit testing. -func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]model.ExperimentTarget, error) { +func (il *Loader) readfile(filepath string, open inputLoaderOpenFn) ([]model.ExperimentTarget, error) { inputs := []model.ExperimentTarget{} filep, err := open(filepath) if err != nil { @@ -304,7 +304,7 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode } // loadRemote loads inputs from a remote source. -func (il *InputLoader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadRemote(ctx context.Context) ([]model.ExperimentTarget, error) { switch registry.CanonicalizeExperimentName(il.ExperimentName) { case "openvpn": // TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass @@ -319,7 +319,7 @@ func (il *InputLoader) loadRemote(ctx context.Context) ([]model.ExperimentTarget } // loadRemoteWebConnectivity loads webconnectivity inputs from a remote source. -func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadRemoteWebConnectivity(ctx context.Context) ([]model.ExperimentTarget, error) { config := il.CheckInConfig if config == nil { // Note: Session.CheckIn documentation says it will fill in @@ -356,7 +356,7 @@ func inputLoaderModelOOAPIURLInfoToModelExperimentTarget( } // loadRemoteOpenVPN loads openvpn inputs from a remote source. -func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.ExperimentTarget, error) { +func (il *Loader) loadRemoteOpenVPN(ctx context.Context) ([]model.ExperimentTarget, error) { // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], // since OOAPIURLInfo is oriented towards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. @@ -393,7 +393,7 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.Experimen // checkIn executes the check-in and filters the returned URLs to exclude // the URLs that are not part of the requested categories. This is done for // robustness, just in case we or the API do something wrong. -func (il *InputLoader) checkIn( +func (il *Loader) checkIn( ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResultNettests, error) { reply, err := il.Session.CheckIn(ctx, config) if err != nil { @@ -409,7 +409,7 @@ func (il *InputLoader) checkIn( } // fetchOpenVPNConfig fetches vpn information for the configured providers -func (il *InputLoader) fetchOpenVPNConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) { +func (il *Loader) fetchOpenVPNConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) { reply, err := il.Session.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { return nil, err @@ -420,7 +420,7 @@ func (il *InputLoader) fetchOpenVPNConfig(ctx context.Context, provider string) // preventMistakes makes the code more robust with respect to any possible // integration issue where the backend returns to us URLs that don't // belong to the category codes we requested. -func (il *InputLoader) preventMistakes(input []model.OOAPIURLInfo, categories []string) (output []model.OOAPIURLInfo) { +func (il *Loader) preventMistakes(input []model.OOAPIURLInfo, categories []string) (output []model.OOAPIURLInfo) { if len(categories) <= 0 { return input } @@ -442,7 +442,7 @@ func (il *InputLoader) preventMistakes(input []model.OOAPIURLInfo, categories [] } // logger returns the configured logger or apex/log's default. -func (il *InputLoader) logger() InputLoaderLogger { +func (il *Loader) logger() Logger { if il.Logger != nil { return il.Logger } diff --git a/internal/engine/inputloader_test.go b/internal/inputloading/inputloading_test.go similarity index 93% rename from internal/engine/inputloader_test.go rename to internal/inputloading/inputloading_test.go index 70f3f86cfd..bcb77f4a56 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/inputloading/inputloading_test.go @@ -1,4 +1,4 @@ -package engine +package inputloading import ( "context" @@ -11,15 +11,14 @@ import ( "testing" "time" - "github.com/apex/log" "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, InputPolicy: model.InputNone, } @@ -34,7 +33,7 @@ func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) { } func TestInputLoaderInputNoneWithFilesInputs(t *testing.T) { - il := &InputLoader{ + il := &Loader{ SourceFiles: []string{ "testdata/inputloader1.txt", "testdata/inputloader2.txt", @@ -52,7 +51,7 @@ func TestInputLoaderInputNoneWithFilesInputs(t *testing.T) { } func TestInputLoaderInputNoneWithBothInputs(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -71,7 +70,7 @@ func TestInputLoaderInputNoneWithBothInputs(t *testing.T) { } func TestInputLoaderInputNoneWithNoInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputNone, } ctx := context.Background() @@ -85,7 +84,7 @@ func TestInputLoaderInputNoneWithNoInput(t *testing.T) { } func TestInputLoaderInputOptionalWithNoInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputOptional, } ctx := context.Background() @@ -99,7 +98,7 @@ func TestInputLoaderInputOptionalWithNoInput(t *testing.T) { } func TestInputLoaderInputOptionalWithInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -148,7 +147,7 @@ func TestInputLoaderInputOptionalWithInput(t *testing.T) { } func TestInputLoaderInputOptionalNonexistentFile(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -168,7 +167,7 @@ func TestInputLoaderInputOptionalNonexistentFile(t *testing.T) { } func TestInputLoaderInputStrictlyRequiredWithInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -217,7 +216,7 @@ func TestInputLoaderInputStrictlyRequiredWithInput(t *testing.T) { } func TestInputLoaderInputStrictlyRequiredWithoutInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputStrictlyRequired, } ctx := context.Background() @@ -231,7 +230,7 @@ func TestInputLoaderInputStrictlyRequiredWithoutInput(t *testing.T) { } func TestInputLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputStrictlyRequired, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -250,7 +249,7 @@ func TestInputLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { } func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "dnscheck", StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ @@ -300,7 +299,7 @@ func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { } func TestInputLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "dnscheck", InputPolicy: model.InputOrStaticDefault, SourceFiles: []string{ @@ -320,7 +319,7 @@ func TestInputLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { } func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "dnscheck", InputPolicy: model.InputOrStaticDefault, } @@ -347,7 +346,7 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { } func TestInputLoaderInputOrStaticDefaultWithoutInputStunReachability(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "stunreachability", InputPolicy: model.InputOrStaticDefault, } @@ -383,7 +382,7 @@ func TestStaticBareInputForExperimentWorksWithNonCanonicalNames(t *testing.T) { } func TestInputLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "xx", InputPolicy: model.InputOrStaticDefault, } @@ -398,7 +397,7 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { } func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -447,18 +446,15 @@ func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { } func TestInputLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing.T) { - sess, err := NewSession(context.Background(), SessionConfig{ - KVStore: &kvstore.Memory{}, - Logger: log.Log, - SoftwareName: "miniooni", - SoftwareVersion: "0.1.0-dev", - TempDir: "testdata", - }) - if err != nil { - t.Fatal(err) + sess := &mocks.Session{ + MockCheckIn: func(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + panic("should not arrive here") + }, } - defer sess.Close() - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputOrQueryBackend, Session: sess, } @@ -474,7 +470,7 @@ func TestInputLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing } func TestInputLoaderInputOrQueryBackendWithEmptyFile(t *testing.T) { - il := &InputLoader{ + il := &Loader{ InputPolicy: model.InputOrQueryBackend, SourceFiles: []string{ "testdata/inputloader1.txt", @@ -513,7 +509,7 @@ func (InputLoaderBrokenFile) Close() error { } func TestInputLoaderReadfileScannerFailure(t *testing.T) { - il := &InputLoader{} + il := &Loader{} out, err := il.readfile("", InputLoaderBrokenFS{}.Open) if !errors.Is(err, syscall.EFAULT) { t.Fatal("not the error we expected") @@ -556,7 +552,7 @@ func (sess *InputLoaderMockableSession) FetchOpenVPNConfig( } func TestInputLoaderCheckInFailure(t *testing.T) { - il := &InputLoader{ + il := &Loader{ Session: &InputLoaderMockableSession{ Error: io.EOF, }, @@ -571,7 +567,7 @@ func TestInputLoaderCheckInFailure(t *testing.T) { } func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) { - il := &InputLoader{ + il := &Loader{ Session: &InputLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{}, @@ -588,7 +584,7 @@ func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) { } func TestInputLoaderCheckInSuccessWithNoURLs(t *testing.T) { - il := &InputLoader{ + il := &Loader{ Session: &InputLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{ @@ -619,7 +615,7 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { } inputs := []model.OOAPIURLInfo{inputs0, inputs1} expect := []model.ExperimentTarget{&inputs0, &inputs1} - il := &InputLoader{ + il := &Loader{ Session: &InputLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{ @@ -640,7 +636,7 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { } func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, Session: &InputLoaderMockableSession{ @@ -661,7 +657,7 @@ func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { } func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { - il := &InputLoader{ + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, Session: &InputLoaderMockableSession{ @@ -686,7 +682,7 @@ func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { expected := errors.New("mocked API error") - il := &InputLoader{ + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, Session: &InputLoaderMockableSession{ @@ -702,6 +698,28 @@ func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { } } +func TestInputLoaderOpenVPNWithNoReturnedURLs(t *testing.T) { + il := &Loader{ + ExperimentName: "openvpn", + InputPolicy: model.InputOrQueryBackend, + Session: &InputLoaderMockableSession{ + FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ + Provider: "riseupvpn", + Config: &model.OOAPIVPNConfig{}, + Inputs: []string{}, + DateUpdated: time.Time{}, + }, + }, + } + out, err := il.loadRemote(context.Background()) + if !errors.Is(err, ErrNoURLsReturned) { + t.Fatal("unexpected a error") + } + if len(out) != 0 { + t.Fatal("we expected no fallback URLs") + } +} + func TestPreventMistakesWithCategories(t *testing.T) { input := []model.OOAPIURLInfo{{ CategoryCode: "NEWS", @@ -725,7 +743,7 @@ func TestPreventMistakesWithCategories(t *testing.T) { URL: "https://addons.mozilla.org/", CountryCode: "XX", }} - il := &InputLoader{} + il := &Loader{} output := il.preventMistakes(input, []string{"NEWS", "FILE"}) if diff := cmp.Diff(desired, output); diff != "" { t.Fatal(diff) @@ -746,7 +764,7 @@ func TestPreventMistakesWithoutCategoriesAndNil(t *testing.T) { URL: "https://addons.mozilla.org/", CountryCode: "XX", }} - il := &InputLoader{} + il := &Loader{} output := il.preventMistakes(input, nil) if diff := cmp.Diff(input, output); diff != "" { t.Fatal(diff) @@ -767,7 +785,7 @@ func TestPreventMistakesWithoutCategoriesAndEmpty(t *testing.T) { URL: "https://addons.mozilla.org/", CountryCode: "XX", }} - il := &InputLoader{} + il := &Loader{} output := il.preventMistakes(input, []string{}) if diff := cmp.Diff(input, output); diff != "" { t.Fatal(diff) @@ -782,7 +800,7 @@ func (ilfl *InputLoaderFakeLogger) Warnf(format string, v ...interface{}) {} func TestInputLoaderLoggerWorksAsIntended(t *testing.T) { logger := &InputLoaderFakeLogger{} - inputLoader := &InputLoader{Logger: logger} + inputLoader := &Loader{Logger: logger} out := inputLoader.logger() if out != logger { t.Fatal("logger not working as intended") diff --git a/internal/engine/testdata/inputloader1.txt b/internal/inputloading/testdata/inputloader1.txt similarity index 100% rename from internal/engine/testdata/inputloader1.txt rename to internal/inputloading/testdata/inputloader1.txt diff --git a/internal/engine/testdata/inputloader2.txt b/internal/inputloading/testdata/inputloader2.txt similarity index 100% rename from internal/engine/testdata/inputloader2.txt rename to internal/inputloading/testdata/inputloader2.txt diff --git a/internal/engine/testdata/inputloader3.txt b/internal/inputloading/testdata/inputloader3.txt similarity index 100% rename from internal/engine/testdata/inputloader3.txt rename to internal/inputloading/testdata/inputloader3.txt diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 7558d71fc8..01eada8abb 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -12,8 +12,8 @@ import ( "sync/atomic" "time" - "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/humanize" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -200,7 +200,7 @@ func (ed *Experiment) newInputLoader(inputPolicy model.InputPolicy) inputLoader if ed.newInputLoaderFn != nil { return ed.newInputLoaderFn(inputPolicy) } - return &engine.InputLoader{ + return &inputloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ RunType: model.RunTypeManual, OnWiFi: true, // meaning: not on 4G diff --git a/internal/oonirun/session.go b/internal/oonirun/session.go index 64566dbeca..4b55da17de 100644 --- a/internal/oonirun/session.go +++ b/internal/oonirun/session.go @@ -10,14 +10,14 @@ package oonirun // import ( - "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) // Session is the definition of Session used by this package. type Session interface { // A Session is also an InputLoaderSession. - engine.InputLoaderSession + inputloading.Session // A Session is also a SubmitterSession. SubmitterSession diff --git a/pkg/oonimkall/taskrunner.go b/pkg/oonimkall/taskrunner.go index 5702ed6499..78f2cec6a1 100644 --- a/pkg/oonimkall/taskrunner.go +++ b/pkg/oonimkall/taskrunner.go @@ -8,6 +8,7 @@ import ( "time" "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -191,7 +192,7 @@ func (r *runnerForTask) Run(rootCtx context.Context) { } case model.InputOrStaticDefault: if len(r.settings.Inputs) <= 0 { - inputs, err := engine.StaticBareInputForExperiment(r.settings.Name) + inputs, err := inputloading.StaticBareInputForExperiment(r.settings.Name) if err != nil { r.emitter.EmitFailureStartup("no default static input for this experiment") return diff --git a/pkg/oonimkall/taskrunner_test.go b/pkg/oonimkall/taskrunner_test.go index 1a3e732062..1c349dfdd0 100644 --- a/pkg/oonimkall/taskrunner_test.go +++ b/pkg/oonimkall/taskrunner_test.go @@ -7,7 +7,7 @@ import ( "time" "github.com/google/go-cmp/cmp" - engine "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/inputloading" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -645,7 +645,7 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeStatusReportCreate, Count: 1}, } - allEntries, err := engine.StaticBareInputForExperiment(experimentName) + allEntries, err := inputloading.StaticBareInputForExperiment(experimentName) if err != nil { t.Fatal(err) }