refactor: move Saver, Submitter, and InputProcessor to oonirun

internal/engine/savemeasurement.go

@@ -0,0 +1,39 @@
+package engine
+
+import (
+	"encoding/json"
+	"os"
+
+	"github.com/ooni/probe-cli/v3/internal/model"
+)
+
+// SaveMeasurement saves a measurement on the specified file path.
+func SaveMeasurement(measurement *model.Measurement, filePath string) error {
+	return saveMeasurement(
+		measurement, filePath, json.Marshal, os.OpenFile,
+		func(fp *os.File, b []byte) (int, error) {
+			return fp.Write(b)
+		},
+	)
+}
+
+func saveMeasurement(
+	measurement *model.Measurement, filePath string,
+	marshal func(v interface{}) ([]byte, error),
+	openFile func(name string, flag int, perm os.FileMode) (*os.File, error),
+	write func(fp *os.File, b []byte) (n int, err error),
+) error {
+	data, err := marshal(measurement)
+	if err != nil {
+		return err
+	}
+	data = append(data, byte('\n'))
+	filep, err := openFile(filePath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0600)
+	if err != nil {
+		return err
+	}
+	if _, err := write(filep, data); err != nil {
+		return err
+	}
+	return filep.Close()
+}

internal/engine/saver_test.go → internal/engine/savemeasurement_test.go

@@ -7,80 +7,13 @@ import (
 	"path/filepath"
 	"testing"
 
-	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/must"
 	"github.com/ooni/probe-cli/v3/internal/runtimex"
 	"github.com/ooni/probe-cli/v3/internal/testingx"
 )
 
-func TestNewSaverDisabled(t *testing.T) {
-	saver, err := NewSaver(SaverConfig{
-		Enabled: false,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if _, ok := saver.(fakeSaver); !ok {
-		t.Fatal("not the type of Saver we expected")
-	}
-	m := new(model.Measurement)
-	if err := saver.SaveMeasurement(m); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestNewSaverWithEmptyFilePath(t *testing.T) {
-	saver, err := NewSaver(SaverConfig{
-		Enabled:  true,
-		FilePath: "",
-	})
-	if err == nil || err.Error() != "saver: passed an empty filepath" {
-		t.Fatalf("not the error we expected: %+v", err)
-	}
-	if saver != nil {
-		t.Fatal("saver should be nil here")
-	}
-}
-
-func TestNewSaverWithFailureWhenSaving(t *testing.T) {
-	filep := runtimex.Try1(os.CreateTemp("", ""))
-	filename := filep.Name()
-	filep.Close()
-	expected := errors.New("mocked error")
-	saver, err := NewSaver(SaverConfig{
-		Enabled:  true,
-		FilePath: filename,
-		Logger:   log.Log,
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	realSaver, ok := saver.(*realSaver)
-	if !ok {
-		t.Fatal("not the type of saver we expected")
-	}
-	var (
-		gotMeasurement *model.Measurement
-		gotFilePath    string
-	)
-	realSaver.savefunc = func(measurement *model.Measurement, filePath string) error {
-		gotMeasurement, gotFilePath = measurement, filePath
-		return expected
-	}
-	m := &model.Measurement{Input: "www.kernel.org"}
-	if err := saver.SaveMeasurement(m); !errors.Is(err, expected) {
-		t.Fatalf("not the error we expected: %+v", err)
-	}
-	if diff := cmp.Diff(m, gotMeasurement); diff != "" {
-		t.Fatal(diff)
-	}
-	if gotFilePath != filename {
-		t.Fatal("passed invalid filepath")
-	}
-}
-
 func TestSaveMeasurementSuccess(t *testing.T) {
 	// get temporary file where to write the measurement
 	filep, err := os.CreateTemp("", "")

internal/engine/session.go

@@ -420,7 +420,7 @@ func (s *Session) newProbeServicesClient(ctx context.Context) (*probeservices.Cl
 }
 
 // NewSubmitter creates a new submitter instance.
-func (s *Session) NewSubmitter(ctx context.Context) (Submitter, error) {
+func (s *Session) NewSubmitter(ctx context.Context) (model.Submitter, error) {
 	psc, err := s.newProbeServicesClient(ctx)
 	if err != nil {
 		return nil, err

internal/oonirun/experiment.go

@@ -64,14 +64,14 @@ type Experiment struct {
 	newInputLoaderFn func(inputPolicy model.InputPolicy) inputLoader
 
 	// newSubmitterFn is OPTIONAL and used for testing.
-	newSubmitterFn func(ctx context.Context) (engine.Submitter, error)
+	newSubmitterFn func(ctx context.Context) (model.Submitter, error)
 
 	// newSaverFn is OPTIONAL and used for testing.
-	newSaverFn func(experiment model.Experiment) (engine.Saver, error)
+	newSaverFn func(experiment model.Experiment) (model.Saver, error)
 
 	// newInputProcessorFn is OPTIONAL and used for testing.
 	newInputProcessorFn func(experiment model.Experiment, inputList []model.OOAPIURLInfo,
-		saver engine.Saver, submitter engine.Submitter) inputProcessor
+		saver model.Saver, submitter model.Submitter) inputProcessor
 }
 
 // Run runs the given experiment.
@@ -138,46 +138,46 @@ type inputProcessor = model.ExperimentInputProcessor
 
 // newInputProcessor creates a new inputProcessor instance.
 func (ed *Experiment) newInputProcessor(experiment model.Experiment,
-	inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor {
+	inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor {
 	if ed.newInputProcessorFn != nil {
 		return ed.newInputProcessorFn(experiment, inputList, saver, submitter)
 	}
-	return &engine.InputProcessor{
+	return &InputProcessor{
 		Annotations: ed.Annotations,
 		Experiment: &experimentWrapper{
-			child:  engine.NewInputProcessorExperimentWrapper(experiment),
+			child:  NewInputProcessorExperimentWrapper(experiment),
 			logger: ed.Session.Logger(),
 			total:  len(inputList),
 		},
 		Inputs:     inputList,
 		MaxRuntime: time.Duration(ed.MaxRuntime) * time.Second,
 		Options:    experimentOptionsToStringList(ed.ExtraOptions),
-		Saver:      engine.NewInputProcessorSaverWrapper(saver),
+		Saver:      NewInputProcessorSaverWrapper(saver),
 		Submitter: &experimentSubmitterWrapper{
-			child:  engine.NewInputProcessorSubmitterWrapper(submitter),
+			child:  NewInputProcessorSubmitterWrapper(submitter),
 			logger: ed.Session.Logger(),
 		},
 	}
 }
 
 // newSaver creates a new engine.Saver instance.
-func (ed *Experiment) newSaver(experiment model.Experiment) (engine.Saver, error) {
+func (ed *Experiment) newSaver(experiment model.Experiment) (model.Saver, error) {
 	if ed.newSaverFn != nil {
 		return ed.newSaverFn(experiment)
 	}
-	return engine.NewSaver(engine.SaverConfig{
+	return NewSaver(SaverConfig{
 		Enabled:  !ed.NoJSON,
 		FilePath: ed.ReportFile,
 		Logger:   ed.Session.Logger(),
 	})
 }
 
 // newSubmitter creates a new engine.Submitter instance.
-func (ed *Experiment) newSubmitter(ctx context.Context) (engine.Submitter, error) {
+func (ed *Experiment) newSubmitter(ctx context.Context) (model.Submitter, error) {
 	if ed.newSubmitterFn != nil {
 		return ed.newSubmitterFn(ctx)
 	}
-	return engine.NewSubmitter(ctx, engine.SubmitterConfig{
+	return NewSubmitter(ctx, SubmitterConfig{
 		Enabled: !ed.NoCollector,
 		Session: ed.Session,
 		Logger:  ed.Session.Logger(),
@@ -233,7 +233,7 @@ func experimentOptionsToStringList(options map[string]any) (out []string) {
 // experimentWrapper wraps an experiment and logs progress
 type experimentWrapper struct {
 	// child is the child experiment wrapper
-	child engine.InputProcessorExperimentWrapper
+	child InputProcessorExperimentWrapper
 
 	// logger is the logger to use
 	logger model.Logger
@@ -254,7 +254,7 @@ func (ew *experimentWrapper) MeasureAsync(
 // fail if we cannot submit a measurement
 type experimentSubmitterWrapper struct {
 	// child is the child submitter wrapper
-	child engine.InputProcessorSubmitterWrapper
+	child InputProcessorSubmitterWrapper
 
 	// logger is the logger to use
 	logger model.Logger

internal/oonirun/experiment_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ooni/probe-cli/v3/internal/engine"
 	"github.com/ooni/probe-cli/v3/internal/mocks"
 	"github.com/ooni/probe-cli/v3/internal/model"
 	"github.com/ooni/probe-cli/v3/internal/testingx"
@@ -81,7 +80,7 @@ func TestExperimentRunWithFailureToSubmitAndShuffle(t *testing.T) {
 		},
 		newExperimentBuilderFn: nil,
 		newInputLoaderFn:       nil,
-		newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+		newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 			subm := &mocks.Submitter{
 				MockSubmit: func(ctx context.Context, m *model.Measurement) error {
 					failedToSubmit++
@@ -171,9 +170,9 @@ func TestExperimentRun(t *testing.T) {
 		Session                Session
 		newExperimentBuilderFn func(experimentName string) (model.ExperimentBuilder, error)
 		newInputLoaderFn       func(inputPolicy model.InputPolicy) inputLoader
-		newSubmitterFn         func(ctx context.Context) (engine.Submitter, error)
-		newSaverFn             func(experiment model.Experiment) (engine.Saver, error)
-		newInputProcessorFn    func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver engine.Saver, submitter engine.Submitter) inputProcessor
+		newSubmitterFn         func(ctx context.Context) (model.Submitter, error)
+		newSaverFn             func(experiment model.Experiment) (model.Saver, error)
+		newInputProcessorFn    func(experiment model.Experiment, inputList []model.OOAPIURLInfo, saver model.Saver, submitter model.Submitter) inputProcessor
 	}
 	type args struct {
 		ctx context.Context
@@ -274,7 +273,7 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return nil, errMocked
 			},
 		},
@@ -317,10 +316,10 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return &mocks.Submitter{}, nil
 			},
-			newSaverFn: func(experiment model.Experiment) (engine.Saver, error) {
+			newSaverFn: func(experiment model.Experiment) (model.Saver, error) {
 				return nil, errMocked
 			},
 		},
@@ -363,14 +362,14 @@ func TestExperimentRun(t *testing.T) {
 					},
 				}
 			},
-			newSubmitterFn: func(ctx context.Context) (engine.Submitter, error) {
+			newSubmitterFn: func(ctx context.Context) (model.Submitter, error) {
 				return &mocks.Submitter{}, nil
 			},
-			newSaverFn: func(experiment model.Experiment) (engine.Saver, error) {
+			newSaverFn: func(experiment model.Experiment) (model.Saver, error) {
 				return &mocks.Saver{}, nil
 			},
 			newInputProcessorFn: func(experiment model.Experiment, inputList []model.OOAPIURLInfo,
-				saver engine.Saver, submitter engine.Submitter) inputProcessor {
+				saver model.Saver, submitter model.Submitter) inputProcessor {
 				return &mocks.ExperimentInputProcessor{
 					MockRun: func(ctx context.Context) error {
 						return errMocked