diff --git a/cmd/ooniprobe/internal/nettests/dnscheck.go b/cmd/ooniprobe/internal/nettests/dnscheck.go index 4fe7de5fbd..cb53560155 100644 --- a/cmd/ooniprobe/internal/nettests/dnscheck.go +++ b/cmd/ooniprobe/internal/nettests/dnscheck.go @@ -3,15 +3,15 @@ package nettests import ( "context" - engine "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // DNSCheck nettest implementation. type DNSCheck struct{} func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + targetloader := &targetloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, @@ -21,7 +21,7 @@ func (n DNSCheck) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) SourceFiles: ctl.InputFiles, StaticInputs: ctl.Inputs, } - testlist, err := inputloader.Load(context.Background()) + testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err } diff --git a/cmd/ooniprobe/internal/nettests/stunreachability.go b/cmd/ooniprobe/internal/nettests/stunreachability.go index c09ae23cb2..743d53e8f2 100644 --- a/cmd/ooniprobe/internal/nettests/stunreachability.go +++ b/cmd/ooniprobe/internal/nettests/stunreachability.go @@ -3,15 +3,15 @@ package nettests import ( "context" - engine "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // STUNReachability nettest implementation. type STUNReachability struct{} func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + targetloader := &targetloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ // not needed because we have default static input in the engine }, @@ -21,7 +21,7 @@ func (n STUNReachability) lookupURLs(ctl *Controller) ([]model.ExperimentTarget, SourceFiles: ctl.InputFiles, StaticInputs: ctl.Inputs, } - testlist, err := inputloader.Load(context.Background()) + testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err } diff --git a/cmd/ooniprobe/internal/nettests/web_connectivity.go b/cmd/ooniprobe/internal/nettests/web_connectivity.go index 5d2fc939ea..7e4bcad996 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/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]model.ExperimentTarget, error) { - inputloader := &engine.InputLoader{ + targetloader := &targetloading.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 @@ -27,7 +27,7 @@ func (n WebConnectivity) lookupURLs(ctl *Controller, categories []string) ([]mod SourceFiles: ctl.InputFiles, StaticInputs: ctl.Inputs, } - testlist, err := inputloader.Load(context.Background()) + testlist, err := targetloader.Load(context.Background()) if err != nil { return nil, err } diff --git a/internal/engine/inputloader_network_test.go b/internal/engine/inputloader_integration_test.go similarity index 59% rename from internal/engine/inputloader_network_test.go rename to internal/engine/inputloader_integration_test.go index 040e85ac20..31b21a33ac 100644 --- a/internal/engine/inputloader_network_test.go +++ b/internal/engine/inputloader_integration_test.go @@ -8,9 +8,16 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) -func TestInputLoaderInputOrQueryBackendWithNoInput(t *testing.T) { +// This historical integration test ensures that we're able to fetch URLs from +// the dev infrastructure. We say this test's historical because the targetloading.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 TestTargetLoaderInputOrQueryBackendWithNoInput(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 := &targetloading.Loader{ InputPolicy: model.InputOrQueryBackend, Session: sess, } diff --git a/internal/mocks/experimentinputloader.go b/internal/mocks/experimentinputloader.go deleted file mode 100644 index c46db13bef..0000000000 --- a/internal/mocks/experimentinputloader.go +++ /dev/null @@ -1,19 +0,0 @@ -package mocks - -import ( - "context" - - "github.com/ooni/probe-cli/v3/internal/model" -) - -// ExperimentInputLoader mocks model.ExperimentInputLoader -type ExperimentInputLoader struct { - MockLoad func(ctx context.Context) ([]model.ExperimentTarget, error) -} - -var _ model.ExperimentInputLoader = &ExperimentInputLoader{} - -// Load calls MockLoad -func (eil *ExperimentInputLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { - return eil.MockLoad(ctx) -} diff --git a/internal/mocks/experimenttargetloader.go b/internal/mocks/experimenttargetloader.go new file mode 100644 index 0000000000..08f153232b --- /dev/null +++ b/internal/mocks/experimenttargetloader.go @@ -0,0 +1,19 @@ +package mocks + +import ( + "context" + + "github.com/ooni/probe-cli/v3/internal/model" +) + +// ExperimentTargetLoader mocks model.ExperimentTargetLoader +type ExperimentTargetLoader struct { + MockLoad func(ctx context.Context) ([]model.ExperimentTarget, error) +} + +var _ model.ExperimentTargetLoader = &ExperimentTargetLoader{} + +// Load calls MockLoad +func (eil *ExperimentTargetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { + return eil.MockLoad(ctx) +} diff --git a/internal/mocks/experimentinputloader_test.go b/internal/mocks/experimenttargetloader_test.go similarity index 93% rename from internal/mocks/experimentinputloader_test.go rename to internal/mocks/experimenttargetloader_test.go index a9d872e4bd..2cdc392b84 100644 --- a/internal/mocks/experimentinputloader_test.go +++ b/internal/mocks/experimenttargetloader_test.go @@ -11,7 +11,7 @@ import ( func TestExperimentInputLoader(t *testing.T) { t.Run("Load", func(t *testing.T) { expected := errors.New("mocked error") - eil := &ExperimentInputLoader{ + eil := &ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return nil, expected }, diff --git a/internal/model/experiment.go b/internal/model/experiment.go index bac0ce0a43..9ae3dbcce6 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -245,8 +245,8 @@ type ExperimentOptionInfo struct { Type string } -// ExperimentInputLoader loads inputs from local or remote sources. -type ExperimentInputLoader interface { +// ExperimentTargetLoader loads targets from local or remote sources. +type ExperimentTargetLoader interface { Load(ctx context.Context) ([]ExperimentTarget, error) } diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 7558d71fc8..09deedaedc 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -12,9 +12,9 @@ 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/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // experimentShuffledInputs counts how many times we shuffled inputs @@ -60,8 +60,8 @@ type Experiment struct { // newExperimentBuilderFn is OPTIONAL and used for testing. newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error) - // newInputLoaderFn is OPTIONAL and used for testing. - newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader + // newTargetLoaderFn is OPTIONAL and used for testing. + newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader // newSubmitterFn is OPTIONAL and used for testing. newSubmitterFn func(ctx context.Context) (model.Submitter, error) @@ -84,8 +84,8 @@ func (ed *Experiment) Run(ctx context.Context) error { } // 2. create input loader and load input for this experiment - inputLoader := ed.newInputLoader(builder.InputPolicy()) - inputList, err := inputLoader.Load(ctx) + targetLoader := ed.newTargetLoader(builder.InputPolicy()) + targetList, err := targetLoader.Load(ctx) if err != nil { return err } @@ -93,8 +93,8 @@ func (ed *Experiment) Run(ctx context.Context) error { // 3. randomize input, if needed if ed.Random { rnd := rand.New(rand.NewSource(time.Now().UnixNano())) // #nosec G404 -- not really important - rnd.Shuffle(len(inputList), func(i, j int) { - inputList[i], inputList[j] = inputList[j], inputList[i] + rnd.Shuffle(len(targetList), func(i, j int) { + targetList[i], targetList[j] = targetList[j], targetList[i] }) experimentShuffledInputs.Add(1) } @@ -127,7 +127,7 @@ func (ed *Experiment) Run(ctx context.Context) error { } // 8. create an input processor - inputProcessor := ed.newInputProcessor(experiment, inputList, saver, submitter) + inputProcessor := ed.newInputProcessor(experiment, targetList, saver, submitter) // 9. process input and generate measurements return inputProcessor.Run(ctx) @@ -192,15 +192,15 @@ func (ed *Experiment) newExperimentBuilder(experimentName string) (model.Experim return ed.Session.NewExperimentBuilder(ed.Name) } -// inputLoader is an alias for model.ExperimentInputLoader -type inputLoader = model.ExperimentInputLoader +// targetLoader is an alias for [model.ExperimentTargetLoader]. +type targetLoader = model.ExperimentTargetLoader -// newInputLoader creates a new inputLoader. -func (ed *Experiment) newInputLoader(inputPolicy model.InputPolicy) inputLoader { - if ed.newInputLoaderFn != nil { - return ed.newInputLoaderFn(inputPolicy) +// newTargetLoader creates a new [model.ExperimentTargetLoader]. +func (ed *Experiment) newTargetLoader(inputPolicy model.InputPolicy) targetLoader { + if ed.newTargetLoaderFn != nil { + return ed.newTargetLoaderFn(inputPolicy) } - return &engine.InputLoader{ + return &targetloading.Loader{ CheckInConfig: &model.OOAPICheckInConfig{ RunType: model.RunTypeManual, OnWiFi: true, // meaning: not on 4G diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index 16d6d16bb4..29764d9c8c 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -75,7 +75,7 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) { }, }, newExperimentBuilderFn: nil, - newInputLoaderFn: nil, + newTargetLoaderFn: nil, newSubmitterFn: func(ctx context.Context) (model.Submitter, error) { subm := &mocks.Submitter{ MockSubmit: func(ctx context.Context, m *model.Measurement) error { @@ -165,7 +165,7 @@ func TestExperimentRun(t *testing.T) { ReportFile string Session Session newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error) - newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader + newTargetLoaderFn func(inputPolicy model.InputPolicy) targetLoader newSubmitterFn func(ctx context.Context) (model.Submitter, error) newSaverFn func() (model.Saver, error) newInputProcessorFn func(experiment model.Experiment, @@ -199,8 +199,8 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { - return &mocks.ExperimentInputLoader{ + newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return nil, errMocked }, @@ -223,8 +223,8 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { - return &mocks.ExperimentInputLoader{ + newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil }, @@ -263,8 +263,8 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { - return &mocks.ExperimentInputLoader{ + newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil }, @@ -306,8 +306,8 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { - return &mocks.ExperimentInputLoader{ + newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil }, @@ -352,8 +352,8 @@ func TestExperimentRun(t *testing.T) { } return eb, nil }, - newInputLoaderFn: func(inputPolicy model.InputPolicy) inputLoader { - return &mocks.ExperimentInputLoader{ + newTargetLoaderFn: func(inputPolicy model.InputPolicy) targetLoader { + return &mocks.ExperimentTargetLoader{ MockLoad: func(ctx context.Context) ([]model.ExperimentTarget, error) { return []model.ExperimentTarget{}, nil }, @@ -392,7 +392,7 @@ func TestExperimentRun(t *testing.T) { ReportFile: tt.fields.ReportFile, Session: tt.fields.Session, newExperimentBuilderFn: tt.fields.newExperimentBuilderFn, - newInputLoaderFn: tt.fields.newInputLoaderFn, + newTargetLoaderFn: tt.fields.newTargetLoaderFn, newSubmitterFn: tt.fields.newSubmitterFn, newSaverFn: tt.fields.newSaverFn, newInputProcessorFn: tt.fields.newInputProcessorFn, diff --git a/internal/oonirun/session.go b/internal/oonirun/session.go index 64566dbeca..0f8fbc3602 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/model" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // Session is the definition of Session used by this package. type Session interface { - // A Session is also an InputLoaderSession. - engine.InputLoaderSession + // A Session is also an [targetloading.Session]. + targetloading.Session // A Session is also a SubmitterSession. SubmitterSession diff --git a/internal/oonirun/v1.go b/internal/oonirun/v1.go index 48bf894709..8a5edeaecf 100644 --- a/internal/oonirun/v1.go +++ b/internal/oonirun/v1.go @@ -84,7 +84,7 @@ func v1Measure(ctx context.Context, config *LinkConfig, URL string) error { ReportFile: config.ReportFile, Session: config.Session, newExperimentBuilderFn: nil, - newInputLoaderFn: nil, + newTargetLoaderFn: nil, newSubmitterFn: nil, newSaverFn: nil, newInputProcessorFn: nil, diff --git a/internal/oonirun/v2.go b/internal/oonirun/v2.go index f6b74d32da..4330b07b76 100644 --- a/internal/oonirun/v2.go +++ b/internal/oonirun/v2.go @@ -194,7 +194,7 @@ func V2MeasureDescriptor(ctx context.Context, config *LinkConfig, desc *V2Descri ReportFile: config.ReportFile, Session: config.Session, newExperimentBuilderFn: nil, - newInputLoaderFn: nil, + newTargetLoaderFn: nil, newSubmitterFn: nil, newSaverFn: nil, newInputProcessorFn: nil, diff --git a/internal/engine/inputloader.go b/internal/targetloading/targetloading.go similarity index 85% rename from internal/engine/inputloader.go rename to internal/targetloading/targetloading.go index 06c31b5d57..6590c2c4a7 100644 --- a/internal/engine/inputloader.go +++ b/internal/targetloading/targetloading.go @@ -1,4 +1,5 @@ -package engine +// Package targetloading contains common code to load richer-input targets. +package targetloading 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 @@ -243,11 +243,11 @@ func StaticBareInputForExperiment(name string) ([]string, error) { // staticInputForExperiment returns the static input for the given experiment // or an error if there's no static input for the experiment. func staticInputForExperiment(name string) ([]model.ExperimentTarget, error) { - return inputLoaderStringListToModelExperimentTarget(StaticBareInputForExperiment(name)) + return stringListToModelExperimentTarget(StaticBareInputForExperiment(name)) } // 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)) @@ -275,12 +275,12 @@ func (il *InputLoader) loadLocal() ([]model.ExperimentTarget, error) { return inputs, nil } -// inputLoaderOpenFn is the type of the function to open a file. -type inputLoaderOpenFn func(filepath string) (fs.File, error) +// openFunc is the type of the function to open a file. +type openFunc 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 openFunc) ([]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 @@ -335,11 +335,11 @@ func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.E if reply.WebConnectivity == nil || len(reply.WebConnectivity.URLs) <= 0 { return nil, ErrNoURLsReturned } - output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(reply.WebConnectivity.URLs) + output := modelOOAPIURLInfoToModelExperimentTarget(reply.WebConnectivity.URLs) return output, nil } -func inputLoaderModelOOAPIURLInfoToModelExperimentTarget( +func modelOOAPIURLInfoToModelExperimentTarget( inputs []model.OOAPIURLInfo) (outputs []model.ExperimentTarget) { for _, input := range inputs { // Note: Dammit! Before we switch to go1.22 we need to continue to @@ -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. @@ -372,7 +372,7 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.Experimen // hitting the API too many times. reply, err := il.fetchOpenVPNConfig(ctx, provider) if err != nil { - output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(urls) + output := modelOOAPIURLInfoToModelExperimentTarget(urls) return output, err } for _, input := range reply.Inputs { @@ -386,14 +386,14 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.Experimen // the experiment on the backend. return nil, ErrNoURLsReturned } - output := inputLoaderModelOOAPIURLInfoToModelExperimentTarget(urls) + output := modelOOAPIURLInfoToModelExperimentTarget(urls) return output, nil } // 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,19 +442,19 @@ 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 } return log.Log } -// inputLoaderStringListToModelExperimentTarget is an utility function to convert +// stringListToModelExperimentTarget is an utility function to convert // a list of strings containing URLs into a list of model.ExperimentTarget // which would have been returned by an hypothetical backend // API serving input for a test for which we don't have an API // yet (e.g., stunreachability and dnscheck). -func inputLoaderStringListToModelExperimentTarget(input []string, err error) ([]model.ExperimentTarget, error) { +func stringListToModelExperimentTarget(input []string, err error) ([]model.ExperimentTarget, error) { if err != nil { return nil, err } diff --git a/internal/engine/inputloader_test.go b/internal/targetloading/targetloading_test.go similarity index 75% rename from internal/engine/inputloader_test.go rename to internal/targetloading/targetloading_test.go index 70f3f86cfd..9710ce1b34 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/targetloading/targetloading_test.go @@ -1,4 +1,4 @@ -package engine +package targetloading 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{ +func TestTargetLoaderInputNoneWithStaticInputs(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, InputPolicy: model.InputNone, } @@ -33,11 +32,11 @@ func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) { } } -func TestInputLoaderInputNoneWithFilesInputs(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputNoneWithFilesInputs(t *testing.T) { + il := &Loader{ SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputNone, } @@ -51,12 +50,12 @@ func TestInputLoaderInputNoneWithFilesInputs(t *testing.T) { } } -func TestInputLoaderInputNoneWithBothInputs(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputNoneWithBothInputs(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputNone, } @@ -70,8 +69,8 @@ func TestInputLoaderInputNoneWithBothInputs(t *testing.T) { } } -func TestInputLoaderInputNoneWithNoInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputNoneWithNoInput(t *testing.T) { + il := &Loader{ InputPolicy: model.InputNone, } ctx := context.Background() @@ -84,8 +83,8 @@ func TestInputLoaderInputNoneWithNoInput(t *testing.T) { } } -func TestInputLoaderInputOptionalWithNoInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOptionalWithNoInput(t *testing.T) { + il := &Loader{ InputPolicy: model.InputOptional, } ctx := context.Background() @@ -98,12 +97,12 @@ func TestInputLoaderInputOptionalWithNoInput(t *testing.T) { } } -func TestInputLoaderInputOptionalWithInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOptionalWithInput(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputOptional, } @@ -147,13 +146,13 @@ func TestInputLoaderInputOptionalWithInput(t *testing.T) { } } -func TestInputLoaderInputOptionalNonexistentFile(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOptionalNonexistentFile(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", + "testdata/loader1.txt", "/nonexistent", - "testdata/inputloader2.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputOptional, } @@ -167,12 +166,12 @@ func TestInputLoaderInputOptionalNonexistentFile(t *testing.T) { } } -func TestInputLoaderInputStrictlyRequiredWithInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputStrictlyRequiredWithInput(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputStrictlyRequired, } @@ -216,8 +215,8 @@ func TestInputLoaderInputStrictlyRequiredWithInput(t *testing.T) { } } -func TestInputLoaderInputStrictlyRequiredWithoutInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputStrictlyRequiredWithoutInput(t *testing.T) { + il := &Loader{ InputPolicy: model.InputStrictlyRequired, } ctx := context.Background() @@ -230,13 +229,13 @@ func TestInputLoaderInputStrictlyRequiredWithoutInput(t *testing.T) { } } -func TestInputLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { + il := &Loader{ InputPolicy: model.InputStrictlyRequired, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader3.txt", // we want it before inputloader2.txt - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader3.txt", // we want it before loader2.txt + "testdata/loader2.txt", }, } ctx := context.Background() @@ -249,13 +248,13 @@ func TestInputLoaderInputStrictlyRequiredWithEmptyFile(t *testing.T) { } } -func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrStaticDefaultWithInput(t *testing.T) { + il := &Loader{ ExperimentName: "dnscheck", StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputOrStaticDefault, } @@ -299,14 +298,14 @@ func TestInputLoaderInputOrStaticDefaultWithInput(t *testing.T) { } } -func TestInputLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { + il := &Loader{ ExperimentName: "dnscheck", InputPolicy: model.InputOrStaticDefault, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader3.txt", // we want it before inputloader2.txt - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader3.txt", // we want it before loader2.txt + "testdata/loader2.txt", }, } ctx := context.Background() @@ -319,8 +318,8 @@ func TestInputLoaderInputOrStaticDefaultWithEmptyFile(t *testing.T) { } } -func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { + il := &Loader{ ExperimentName: "dnscheck", InputPolicy: model.InputOrStaticDefault, } @@ -346,8 +345,8 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputDNSCheck(t *testing.T) { } } -func TestInputLoaderInputOrStaticDefaultWithoutInputStunReachability(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrStaticDefaultWithoutInputStunReachability(t *testing.T) { + il := &Loader{ ExperimentName: "stunreachability", InputPolicy: model.InputOrStaticDefault, } @@ -382,8 +381,8 @@ func TestStaticBareInputForExperimentWorksWithNonCanonicalNames(t *testing.T) { } } -func TestInputLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { + il := &Loader{ ExperimentName: "xx", InputPolicy: model.InputOrStaticDefault, } @@ -397,12 +396,12 @@ func TestInputLoaderInputOrStaticDefaultWithoutInputOtherName(t *testing.T) { } } -func TestInputLoaderInputOrQueryBackendWithInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrQueryBackendWithInput(t *testing.T) { + il := &Loader{ StaticInputs: []string{"https://www.google.com/"}, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader2.txt", }, InputPolicy: model.InputOrQueryBackend, } @@ -446,19 +445,16 @@ 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) +func TestTargetLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing.T) { + 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, } @@ -473,13 +469,13 @@ func TestInputLoaderInputOrQueryBackendWithNoInputAndCancelledContext(t *testing } } -func TestInputLoaderInputOrQueryBackendWithEmptyFile(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderInputOrQueryBackendWithEmptyFile(t *testing.T) { + il := &Loader{ InputPolicy: model.InputOrQueryBackend, SourceFiles: []string{ - "testdata/inputloader1.txt", - "testdata/inputloader3.txt", // we want it before inputloader2.txt - "testdata/inputloader2.txt", + "testdata/loader1.txt", + "testdata/loader3.txt", // we want it before loader2.txt + "testdata/loader2.txt", }, } ctx := context.Background() @@ -492,29 +488,29 @@ func TestInputLoaderInputOrQueryBackendWithEmptyFile(t *testing.T) { } } -type InputLoaderBrokenFS struct{} +type TargetLoaderBrokenFS struct{} -func (InputLoaderBrokenFS) Open(filepath string) (fs.File, error) { - return InputLoaderBrokenFile{}, nil +func (TargetLoaderBrokenFS) Open(filepath string) (fs.File, error) { + return TargetLoaderBrokenFile{}, nil } -type InputLoaderBrokenFile struct{} +type TargetLoaderBrokenFile struct{} -func (InputLoaderBrokenFile) Stat() (os.FileInfo, error) { +func (TargetLoaderBrokenFile) Stat() (os.FileInfo, error) { return nil, nil } -func (InputLoaderBrokenFile) Read([]byte) (int, error) { +func (TargetLoaderBrokenFile) Read([]byte) (int, error) { return 0, syscall.EFAULT } -func (InputLoaderBrokenFile) Close() error { +func (TargetLoaderBrokenFile) Close() error { return nil } -func TestInputLoaderReadfileScannerFailure(t *testing.T) { - il := &InputLoader{} - out, err := il.readfile("", InputLoaderBrokenFS{}.Open) +func TestTargetLoaderReadfileScannerFailure(t *testing.T) { + il := &Loader{} + out, err := il.readfile("", TargetLoaderBrokenFS{}.Open) if !errors.Is(err, syscall.EFAULT) { t.Fatal("not the error we expected") } @@ -523,9 +519,9 @@ func TestInputLoaderReadfileScannerFailure(t *testing.T) { } } -// InputLoaderMockableSession is a mockable session -// used by InputLoader tests. -type InputLoaderMockableSession struct { +// TargetLoaderMockableSession is a mockable session +// used by TargetLoader tests. +type TargetLoaderMockableSession struct { // Output contains the output of CheckIn. It should // be nil when Error is not-nil. Output *model.OOAPICheckInResult @@ -539,8 +535,8 @@ type InputLoaderMockableSession struct { Error error } -// CheckIn implements InputLoaderSession.CheckIn. -func (sess *InputLoaderMockableSession) CheckIn( +// CheckIn implements TargetLoaderSession.CheckIn. +func (sess *TargetLoaderMockableSession) CheckIn( ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) { if sess.Output == nil && sess.Error == nil { return nil, errors.New("both Output and Error are nil") @@ -548,16 +544,16 @@ func (sess *InputLoaderMockableSession) CheckIn( return sess.Output, sess.Error } -// FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig. -func (sess *InputLoaderMockableSession) FetchOpenVPNConfig( +// FetchOpenVPNConfig implements TargetLoaderSession.FetchOpenVPNConfig. +func (sess *TargetLoaderMockableSession) FetchOpenVPNConfig( ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { runtimex.Assert(!(sess.Error == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig and Error are nil") return sess.FetchOpenVPNConfigOutput, sess.Error } -func TestInputLoaderCheckInFailure(t *testing.T) { - il := &InputLoader{ - Session: &InputLoaderMockableSession{ +func TestTargetLoaderCheckInFailure(t *testing.T) { + il := &Loader{ + Session: &TargetLoaderMockableSession{ Error: io.EOF, }, } @@ -570,9 +566,9 @@ func TestInputLoaderCheckInFailure(t *testing.T) { } } -func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) { - il := &InputLoader{ - Session: &InputLoaderMockableSession{ +func TestTargetLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) { + il := &Loader{ + Session: &TargetLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{}, }, @@ -587,9 +583,9 @@ func TestInputLoaderCheckInSuccessWithNilWebConnectivity(t *testing.T) { } } -func TestInputLoaderCheckInSuccessWithNoURLs(t *testing.T) { - il := &InputLoader{ - Session: &InputLoaderMockableSession{ +func TestTargetLoaderCheckInSuccessWithNoURLs(t *testing.T) { + il := &Loader{ + Session: &TargetLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{ WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{}, @@ -606,7 +602,7 @@ func TestInputLoaderCheckInSuccessWithNoURLs(t *testing.T) { } } -func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { +func TestTargetLoaderCheckInSuccessWithSomeURLs(t *testing.T) { inputs0 := model.OOAPIURLInfo{ CategoryCode: "NEWS", CountryCode: "IT", @@ -619,8 +615,8 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { } inputs := []model.OOAPIURLInfo{inputs0, inputs1} expect := []model.ExperimentTarget{&inputs0, &inputs1} - il := &InputLoader{ - Session: &InputLoaderMockableSession{ + il := &Loader{ + Session: &TargetLoaderMockableSession{ Output: &model.OOAPICheckInResult{ Tests: model.OOAPICheckInResultNettests{ WebConnectivity: &model.OOAPICheckInInfoWebConnectivity{ @@ -639,11 +635,11 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { } } -func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderOpenVPNSuccessWithNoInput(t *testing.T) { + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, - Session: &InputLoaderMockableSession{ + Session: &TargetLoaderMockableSession{ Error: nil, FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ Provider: "riseup", @@ -660,11 +656,11 @@ func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { } } -func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { - il := &InputLoader{ +func TestTargetLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, - Session: &InputLoaderMockableSession{ + Session: &TargetLoaderMockableSession{ Error: nil, FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ Provider: "riseupvpn", @@ -684,12 +680,12 @@ func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { } } -func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { +func TestTargetLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { expected := errors.New("mocked API error") - il := &InputLoader{ + il := &Loader{ ExperimentName: "openvpn", InputPolicy: model.InputOrQueryBackend, - Session: &InputLoaderMockableSession{ + Session: &TargetLoaderMockableSession{ Error: expected, }, } @@ -702,6 +698,28 @@ func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { } } +func TestTargetLoaderOpenVPNWithNoReturnedURLs(t *testing.T) { + il := &Loader{ + ExperimentName: "openvpn", + InputPolicy: model.InputOrQueryBackend, + Session: &TargetLoaderMockableSession{ + 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,23 +785,23 @@ 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) } } -// InputLoaderFakeLogger is a fake InputLoaderLogger. -type InputLoaderFakeLogger struct{} +// TargetLoaderFakeLogger is a fake TargetLoaderLogger. +type TargetLoaderFakeLogger struct{} -// Warnf implements InputLoaderLogger.Warnf -func (ilfl *InputLoaderFakeLogger) Warnf(format string, v ...interface{}) {} +// Warnf implements TargetLoaderLogger.Warnf +func (ilfl *TargetLoaderFakeLogger) Warnf(format string, v ...interface{}) {} -func TestInputLoaderLoggerWorksAsIntended(t *testing.T) { - logger := &InputLoaderFakeLogger{} - inputLoader := &InputLoader{Logger: logger} - out := inputLoader.logger() +func TestTargetLoaderLoggerWorksAsIntended(t *testing.T) { + logger := &TargetLoaderFakeLogger{} + targetLoader := &Loader{Logger: logger} + out := targetLoader.logger() if out != logger { t.Fatal("logger not working as intended") } @@ -794,7 +812,7 @@ func TestStringListToModelURLInfoWithValidInput(t *testing.T) { "stun://stun.voip.blackberry.com:3478", "stun://stun.altar.com.pl:3478", } - output, err := inputLoaderStringListToModelExperimentTarget(input, nil) + output, err := stringListToModelExperimentTarget(input, nil) if err != nil { t.Fatal(err) } @@ -820,7 +838,7 @@ func TestStringListToModelURLInfoWithInvalidInput(t *testing.T) { "\t", // <- not a valid URL "stun://stun.altar.com.pl:3478", } - output, err := inputLoaderStringListToModelExperimentTarget(input, nil) + output, err := stringListToModelExperimentTarget(input, nil) if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") { t.Fatal("no the error we expected", err) } @@ -836,7 +854,7 @@ func TestStringListToModelURLInfoWithError(t *testing.T) { "stun://stun.altar.com.pl:3478", } expected := errors.New("mocked error") - output, err := inputLoaderStringListToModelExperimentTarget(input, expected) + output, err := stringListToModelExperimentTarget(input, expected) if !errors.Is(err, expected) { t.Fatal("not the error we expected", err) } diff --git a/internal/engine/testdata/inputloader1.txt b/internal/targetloading/testdata/loader1.txt similarity index 100% rename from internal/engine/testdata/inputloader1.txt rename to internal/targetloading/testdata/loader1.txt diff --git a/internal/engine/testdata/inputloader2.txt b/internal/targetloading/testdata/loader2.txt similarity index 100% rename from internal/engine/testdata/inputloader2.txt rename to internal/targetloading/testdata/loader2.txt diff --git a/internal/engine/testdata/inputloader3.txt b/internal/targetloading/testdata/loader3.txt similarity index 100% rename from internal/engine/testdata/inputloader3.txt rename to internal/targetloading/testdata/loader3.txt diff --git a/pkg/oonimkall/taskrunner.go b/pkg/oonimkall/taskrunner.go index 5702ed6499..d5b70947c0 100644 --- a/pkg/oonimkall/taskrunner.go +++ b/pkg/oonimkall/taskrunner.go @@ -10,6 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/targetloading" ) // runnerForTask runs a specific task @@ -169,16 +170,16 @@ func (r *runnerForTask) Run(rootCtx context.Context) { builder.SetCallbacks(&runnerCallbacks{emitter: r.emitter}) // TODO(bassosimone): replace the following code with an - // invocation of the InputLoader. Since I am making these + // invocation of the targetloading.Loader. Since I am making these // changes before a release and I've already changed the // code a lot, I'd rather avoid changing it even more, // for the following reason: // - // If we add an call InputLoader here, this code will + // If we add and call targetloading.Loader here, this code will // magically invoke check-in for InputOrQueryBackend, // which we need to make sure the app can handle. This is // the main reason why now I don't fill like properly - // fixing this code and use InputLoader: too much work + // fixing this code and use targetloading.Loader: too much work // in too little time, so mistakes more likely. // // In fact, our current app assumes that it's its @@ -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 := targetloading.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..d7840e8f12 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/targetloading" "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 := targetloading.StaticBareInputForExperiment(experimentName) if err != nil { t.Fatal(err) }