diff --git a/internal/oonirun/experiment.go b/internal/oonirun/experiment.go index 329b57492..0435b21d1 100644 --- a/internal/oonirun/experiment.go +++ b/internal/oonirun/experiment.go @@ -7,9 +7,7 @@ package oonirun import ( "context" "encoding/json" - "fmt" "math/rand" - "strings" "sync/atomic" "time" @@ -235,22 +233,6 @@ func (ed *Experiment) newTargetLoader(builder model.ExperimentBuilder) targetLoa }) } -// experimentOptionsToStringList convers the options to []string, which is -// the format with which we include them into a OONI Measurement. The resulting -// []string will skip any option that is named with a `Safe` prefix (case -// sensitive). -func experimentOptionsToStringList(options map[string]any) (out []string) { - // the prefix to skip inclusion in the string list - safeOptionPrefix := "Safe" - for key, value := range options { - if strings.HasPrefix(key, safeOptionPrefix) { - continue - } - out = append(out, fmt.Sprintf("%s=%v", key, value)) - } - return -} - // experimentWrapper wraps an experiment and logs progress type experimentWrapper struct { // child is the child experiment wrapper diff --git a/internal/oonirun/experiment_test.go b/internal/oonirun/experiment_test.go index cbe24d803..c840d7a17 100644 --- a/internal/oonirun/experiment_test.go +++ b/internal/oonirun/experiment_test.go @@ -4,8 +4,6 @@ import ( "context" "encoding/json" "errors" - "reflect" - "sort" "testing" "time" @@ -189,48 +187,6 @@ func TestExperimentSetOptions(t *testing.T) { } } -func Test_experimentOptionsToStringList(t *testing.T) { - type args struct { - options map[string]any - } - tests := []struct { - name string - args args - wantOut []string - }{ - { - name: "happy path: a map with three entries returns three items", - args: args{ - map[string]any{ - "foo": 1, - "bar": 2, - "baaz": 3, - }, - }, - wantOut: []string{"baaz=3", "bar=2", "foo=1"}, - }, - { - name: "an option beginning with `Safe` is skipped from the output", - args: args{ - map[string]any{ - "foo": 1, - "Safefoo": 42, - }, - }, - wantOut: []string{"foo=1"}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gotOut := experimentOptionsToStringList(tt.args.options) - sort.Strings(gotOut) - if !reflect.DeepEqual(gotOut, tt.wantOut) { - t.Errorf("experimentOptionsToStringList() = %v, want %v", gotOut, tt.wantOut) - } - }) - } -} - func TestExperimentRun(t *testing.T) { errMocked := errors.New("mocked error") type fields struct { diff --git a/internal/oonirun/inputprocessor.go b/internal/oonirun/inputprocessor.go index 79ef4380e..e8f86e958 100644 --- a/internal/oonirun/inputprocessor.go +++ b/internal/oonirun/inputprocessor.go @@ -143,8 +143,11 @@ func (ip *InputProcessor) run(ctx context.Context) (int, error) { meas.AddAnnotations(ip.Annotations) err = ip.Submitter.Submit(ctx, idx, meas) if err != nil { - // TODO(bassosimone): the default submitter used by - // miniooni ignores errors. Should we make it the default? + // TODO(bassosimone): when re-reading this code, I find it confusing that + // we return on error because I am always like "wait, this is not the right + // thing to do here". Then, I remember that the experimentSubmitterWrapper + // ignores this error and so it's like it does not exist. Maybe we should + // rewrite the code to do the right thing here. return 0, err } // Note: must be after submission because submission modifies