diff --git a/pkg/oonimkall/session.go b/pkg/oonimkall/session.go index c4fc92ce7..b10cf8315 100644 --- a/pkg/oonimkall/session.go +++ b/pkg/oonimkall/session.go @@ -2,8 +2,6 @@ package oonimkall import ( "context" - "encoding/json" - "errors" "net/url" "runtime" "sync" @@ -13,7 +11,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/legacy/assetsdir" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/runtimex" ) // AtomicInt64 allows us to export atomic.Int64 variables to @@ -128,10 +125,9 @@ type Session struct { TestingCheckInBeforeNewProbeServicesClient func(ctx *Context) TestingCheckInBeforeCheckIn func(ctx *Context) - cl []context.CancelFunc - mtx sync.Mutex - submitter model.Submitter - sessp *engine.Session + cl []context.CancelFunc + mtx sync.Mutex + sessp *engine.Session } // NewSession is like NewSessionWithContext but without context. This @@ -302,194 +298,3 @@ func (sess *Session) Geolocate(ctx *Context) (*GeolocateResults, error) { Org: sess.sessp.ProbeNetworkName(), }, nil } - -// SubmitMeasurementResults contains the results of a single measurement submission -// to the OONI backends using the OONI collector API. -type SubmitMeasurementResults struct { - // UpdateMeasurement is the measurement with updated report ID. - UpdatedMeasurement string - - // UpdatedReportID is the report ID used for the measurement. - UpdatedReportID string - - // MeasurementUID is the measurement unique identifier returned from the backend - MeasurementUID string -} - -// Submit submits the given measurement and returns the results. -// -// This function locks the session until it's done. That is, no other operation -// can be performed as long as this function is pending. -func (sess *Session) Submit(ctx *Context, measurement string) (*SubmitMeasurementResults, error) { - sess.mtx.Lock() - defer sess.mtx.Unlock() - if sess.submitter == nil { - submitter, err := sess.sessp.NewSubmitter(ctx.ctx) - if err != nil { - return nil, err - } - sess.submitter = submitter - } - var mm model.Measurement - if err := json.Unmarshal([]byte(measurement), &mm); err != nil { - return nil, err - } - muid, err := sess.submitter.Submit(ctx.ctx, &mm) - if err != nil { - return nil, err - } - data, err := json.Marshal(mm) - runtimex.PanicOnError(err, "json.Marshal should not fail here") - return &SubmitMeasurementResults{ - UpdatedMeasurement: string(data), - UpdatedReportID: mm.ReportID, - MeasurementUID: muid, - }, nil -} - -// CheckInConfigWebConnectivity contains WebConnectivity -// configuration for the check-in API. -type CheckInConfigWebConnectivity struct { - // CategoryCodes contains zero or more category codes (e.g. "HUMR"). - CategoryCodes []string -} - -// AddCategory adds a category code to ckw.CategoryCode. This method allows you to -// edit ckw.CategoryCodes, which is inaccessible from Java/ObjC. -func (ckw *CheckInConfigWebConnectivity) AddCategory(cat string) { - ckw.CategoryCodes = append(ckw.CategoryCodes, cat) -} - -func (ckw *CheckInConfigWebConnectivity) toModel() model.OOAPICheckInConfigWebConnectivity { - return model.OOAPICheckInConfigWebConnectivity{ - CategoryCodes: ckw.CategoryCodes, - } -} - -// CheckInConfig contains configuration for the check-in API. -type CheckInConfig struct { - // Charging indicates whether the phone is charging. - Charging bool - - // OnWiFi indicates whether the phone is using the Wi-Fi. - OnWiFi bool - - // Platform is the mobile platform (e.g. "android") - Platform string - - // RunType indicates whether this is an automated (model.RunTypeTimed) run - // or otherwise a manual run initiated by the user. - RunType string - - // SoftwareName is the name of the application. - SoftwareName string - - // SoftwareVersion is the version of the application. - SoftwareVersion string - - // WebConnectivity contains configuration items specific of - // the WebConnectivity experiment. - WebConnectivity *CheckInConfigWebConnectivity -} - -// CheckInInfoWebConnectivity contains the WebConnectivity -// specific results of the check-in API call. -type CheckInInfoWebConnectivity struct { - // ReportID is the report ID we should be using. - ReportID string - - // URLs contains the list of URLs to measure. - URLs []model.OOAPIURLInfo -} - -// URLInfo contains info on a specific URL to measure. -type URLInfo struct { - // CategoryCode is the URL's category code (e.g. "HUMR"). - CategoryCode string - - // CountryCode is the test list from which this URL - // comes from (e.g. "IT", "FR"). - CountryCode string - - // URL is the URL itself. - URL string -} - -// Size returns the number of URLs included into the result. -func (ckw *CheckInInfoWebConnectivity) Size() int64 { - return int64(len(ckw.URLs)) -} - -// At returns the URLInfo at index idx. Note that this function will -// return nil/null if the index is out of bounds. -func (ckw *CheckInInfoWebConnectivity) At(idx int64) *URLInfo { - if idx < 0 || int(idx) >= len(ckw.URLs) { - return nil - } - w := ckw.URLs[idx] - return &URLInfo{ - CategoryCode: w.CategoryCode, - CountryCode: w.CountryCode, - URL: w.URL, - } -} - -func newCheckInInfoWebConnectivity(ckw *model.OOAPICheckInInfoWebConnectivity) *CheckInInfoWebConnectivity { - if ckw == nil { - return nil - } - return &CheckInInfoWebConnectivity{ - ReportID: ckw.ReportID, - URLs: ckw.URLs, - } -} - -// CheckInInfo contains the result of the check-in API. -type CheckInInfo struct { - // WebConnectivity contains results that are specific to - // the WebConnectivity experiment. This field MAY be null - // if the server's response did not contain any info. - WebConnectivity *CheckInInfoWebConnectivity -} - -// CheckIn calls the check-in API. Both ctx and config MUST NOT be nil. This -// function will fail if config is missing required settings. The return value -// is either an error or a valid CheckInInfo instance. Beware that the returned -// object MAY still contain nil fields depending on the server's response. -// -// This function locks the session until it's done. That is, no other operation -// can be performed as long as this function is pending. -func (sess *Session) CheckIn(ctx *Context, config *CheckInConfig) (*CheckInInfo, error) { - sess.mtx.Lock() - defer sess.mtx.Unlock() - if config.WebConnectivity == nil { - return nil, errors.New("oonimkall: missing webconnectivity config") - } - if err := sess.sessp.MaybeLookupLocationContext(ctx.ctx); err != nil { - return nil, err - } - if sess.TestingCheckInBeforeNewProbeServicesClient != nil { - sess.TestingCheckInBeforeNewProbeServicesClient(ctx) // for testing - } - if sess.TestingCheckInBeforeCheckIn != nil { - sess.TestingCheckInBeforeCheckIn(ctx) // for testing - } - cfg := model.OOAPICheckInConfig{ - Charging: config.Charging, - OnWiFi: config.OnWiFi, - Platform: config.Platform, - ProbeASN: sess.sessp.ProbeASNString(), - ProbeCC: sess.sessp.ProbeCC(), - RunType: model.RunType(config.RunType), - SoftwareName: config.SoftwareName, - SoftwareVersion: config.SoftwareVersion, - WebConnectivity: config.WebConnectivity.toModel(), - } - result, err := sess.sessp.CheckIn(ctx.ctx, &cfg) - if err != nil { - return nil, err - } - return &CheckInInfo{ - WebConnectivity: newCheckInInfoWebConnectivity(result.Tests.WebConnectivity), - }, nil -} diff --git a/pkg/oonimkall/session_integration_test.go b/pkg/oonimkall/session_integration_test.go index bcb83861e..ef16fff90 100644 --- a/pkg/oonimkall/session_integration_test.go +++ b/pkg/oonimkall/session_integration_test.go @@ -2,7 +2,6 @@ package oonimkall_test import ( "context" - "encoding/json" "errors" "fmt" "os" @@ -13,7 +12,6 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/enginelocate" - "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/pkg/oonimkall" ) @@ -104,297 +102,6 @@ func TestGeolocateGood(t *testing.T) { } } -func ReduceErrorForSubmitter(err error) error { - if err == nil { - return errors.New("we expected an error here") - } - if errors.Is(err, context.Canceled) { - return nil // when we have not downloaded the resources yet - } - if err.Error() == "all available probe services failed" { - return nil // otherwise - } - return fmt.Errorf("not the error we expected: %w", err) -} - -func TestSubmitWithCancelledContext(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - ctx.Cancel() // cause immediate failure - result, err := sess.Submit(ctx, "{}") - if err := ReduceErrorForSubmitter(err); err != nil { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("expected nil result here") - } -} - -func TestSubmitWithInvalidJSON(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - result, err := sess.Submit(ctx, "{") - if err == nil || err.Error() != "unexpected end of JSON input" { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("expected nil result here") - } -} - -func DoSubmission(ctx *oonimkall.Context, sess *oonimkall.Session) error { - inputm := model.Measurement{ - DataFormatVersion: "0.2.0", - MeasurementStartTime: "2019-10-28 12:51:07", - MeasurementRuntime: 1.71, - ProbeASN: "AS30722", - ProbeCC: "IT", - ProbeIP: "127.0.0.1", - ReportID: "", - ResolverIP: "172.217.33.129", - SoftwareName: "miniooni", - SoftwareVersion: "0.1.0-dev", - TestKeys: map[string]bool{"success": true}, - TestName: "example", - TestVersion: "0.1.0", - } - inputd, err := json.Marshal(inputm) - if err != nil { - return err - } - result, err := sess.Submit(ctx, string(inputd)) - if err != nil { - return fmt.Errorf("session_test.go: submit failed: %w", err) - } - if result.UpdatedMeasurement == "" { - return errors.New("expected non empty measurement") - } - if result.UpdatedReportID == "" { - return errors.New("expected non empty report ID") - } - var outputm model.Measurement - return json.Unmarshal([]byte(result.UpdatedMeasurement), &outputm) -} - -func TestSubmitMeasurementGood(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - if err := DoSubmission(ctx, sess); err != nil { - t.Fatal(err) - } -} - -func TestSubmitCancelContextAfterFirstSubmission(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - if err := DoSubmission(ctx, sess); err != nil { - t.Fatal(err) - } - ctx.Cancel() // fail second submission - err = DoSubmission(ctx, sess) - if err == nil || !strings.HasPrefix(err.Error(), "session_test.go: submit failed") { - t.Fatalf("not the error we expected: %+v", err) - } - if !errors.Is(err, context.Canceled) { - t.Fatalf("not the error we expected: %+v", err) - } -} - -func TestCheckInSuccess(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - config := oonimkall.CheckInConfig{ - Charging: true, - OnWiFi: true, - Platform: "android", - RunType: string(model.RunTypeTimed), - SoftwareName: "ooniprobe-android", - SoftwareVersion: "2.7.1", - WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{}, - } - config.WebConnectivity.AddCategory("NEWS") - config.WebConnectivity.AddCategory("CULTR") - result, err := sess.CheckIn(ctx, &config) - if err != nil { - t.Fatalf("unexpected error: %+v", err) - } - if result == nil || result.WebConnectivity == nil { - t.Fatal("got nil result or WebConnectivity") - } - if len(result.WebConnectivity.URLs) < 1 { - t.Fatal("unexpected number of URLs") - } - if result.WebConnectivity.ReportID == "" { - t.Fatal("got empty report ID") - } - siz := result.WebConnectivity.Size() - if siz <= 0 { - t.Fatal("unexpected number of URLs") - } - for idx := int64(0); idx < siz; idx++ { - entry := result.WebConnectivity.At(idx) - if entry.CategoryCode != "NEWS" && entry.CategoryCode != "CULTR" { - t.Fatalf("unexpected category code: %+v", entry) - } - } - if result.WebConnectivity.At(-1) != nil { - t.Fatal("expected nil here") - } - if result.WebConnectivity.At(siz) != nil { - t.Fatal("expected nil here") - } -} - -func TestCheckInLookupLocationFailure(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - config := oonimkall.CheckInConfig{ - Charging: true, - OnWiFi: true, - Platform: "android", - RunType: string(model.RunTypeTimed), - SoftwareName: "ooniprobe-android", - SoftwareVersion: "2.7.1", - WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{}, - } - config.WebConnectivity.AddCategory("NEWS") - config.WebConnectivity.AddCategory("CULTR") - ctx.Cancel() // immediate failure - result, err := sess.CheckIn(ctx, &config) - if !errors.Is(err, context.Canceled) { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("expected nil result here") - } -} - -func TestCheckInNewProbeServicesFailure(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - sess.TestingCheckInBeforeNewProbeServicesClient = func(ctx *oonimkall.Context) { - ctx.Cancel() // cancel execution - } - ctx := sess.NewContext() - config := oonimkall.CheckInConfig{ - Charging: true, - OnWiFi: true, - Platform: "android", - RunType: string(model.RunTypeTimed), - SoftwareName: "ooniprobe-android", - SoftwareVersion: "2.7.1", - WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{}, - } - config.WebConnectivity.AddCategory("NEWS") - config.WebConnectivity.AddCategory("CULTR") - result, err := sess.CheckIn(ctx, &config) - if !errors.Is(err, context.Canceled) { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("expected nil result here") - } -} - -func TestCheckInCheckInFailure(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - sess.TestingCheckInBeforeCheckIn = func(ctx *oonimkall.Context) { - ctx.Cancel() // cancel execution - } - ctx := sess.NewContext() - config := oonimkall.CheckInConfig{ - Charging: true, - OnWiFi: true, - Platform: "android", - RunType: string(model.RunTypeTimed), - SoftwareName: "ooniprobe-android", - SoftwareVersion: "2.7.1", - WebConnectivity: &oonimkall.CheckInConfigWebConnectivity{}, - } - config.WebConnectivity.AddCategory("NEWS") - config.WebConnectivity.AddCategory("CULTR") - result, err := sess.CheckIn(ctx, &config) - if !errors.Is(err, context.Canceled) { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("expected nil result here") - } -} - -func TestCheckInNoParams(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - sess, err := NewSessionForTesting() - if err != nil { - t.Fatal(err) - } - ctx := sess.NewContext() - config := oonimkall.CheckInConfig{ - Charging: true, - OnWiFi: true, - Platform: "android", - RunType: string(model.RunTypeTimed), - SoftwareName: "ooniprobe-android", - SoftwareVersion: "2.7.1", - } - result, err := sess.CheckIn(ctx, &config) - if err == nil || err.Error() != "oonimkall: missing webconnectivity config" { - t.Fatalf("not the error we expected: %+v", err) - } - if result != nil { - t.Fatal("unexpected not nil result here") - } -} - func TestMain(m *testing.M) { // Here we're basically testing whether eventually the finalizers // will run and the number of active sessions and cancels will become diff --git a/pkg/oonimkall/session_test.go b/pkg/oonimkall/session_test.go deleted file mode 100644 index 19ca838b0..000000000 --- a/pkg/oonimkall/session_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package oonimkall - -import "testing" - -func TestNewCheckInInfoWebConnectivityNilPointer(t *testing.T) { - out := newCheckInInfoWebConnectivity(nil) - if out != nil { - t.Fatal("expected nil pointer") - } -} diff --git a/pkg/oonimkall/taskmodel.go b/pkg/oonimkall/taskmodel.go index 3d069250e..ed6696de1 100644 --- a/pkg/oonimkall/taskmodel.go +++ b/pkg/oonimkall/taskmodel.go @@ -53,26 +53,24 @@ const ( // type of emitted events. const ( - eventTypeFailureIPLookup = "failure.ip_lookup" - eventTypeFailureASNLookup = "failure.asn_lookup" - eventTypeFailureCCLookup = "failure.cc_lookup" - eventTypeFailureMeasurement = "failure.measurement" - eventTypeFailureMeasurementSubmission = "failure.measurement_submission" - eventTypeFailureReportCreate = "failure.report_create" - eventTypeFailureResolverLookup = "failure.resolver_lookup" - eventTypeFailureStartup = "failure.startup" - eventTypeLog = "log" - eventTypeMeasurement = "measurement" - eventTypeStatusEnd = "status.end" - eventTypeStatusGeoIPLookup = "status.geoip_lookup" - eventTypeStatusMeasurementDone = "status.measurement_done" - eventTypeStatusMeasurementStart = "status.measurement_start" - eventTypeStatusMeasurementSubmission = "status.measurement_submission" - eventTypeStatusProgress = "status.progress" - eventTypeStatusQueued = "status.queued" - eventTypeStatusReportCreate = "status.report_create" - eventTypeStatusResolverLookup = "status.resolver_lookup" - eventTypeStatusStarted = "status.started" + eventTypeFailureIPLookup = "failure.ip_lookup" + eventTypeFailureASNLookup = "failure.asn_lookup" + eventTypeFailureCCLookup = "failure.cc_lookup" + eventTypeFailureMeasurement = "failure.measurement" + eventTypeFailureReportCreate = "failure.report_create" + eventTypeFailureResolverLookup = "failure.resolver_lookup" + eventTypeFailureStartup = "failure.startup" + eventTypeLog = "log" + eventTypeMeasurement = "measurement" + eventTypeStatusEnd = "status.end" + eventTypeStatusGeoIPLookup = "status.geoip_lookup" + eventTypeStatusMeasurementDone = "status.measurement_done" + eventTypeStatusMeasurementStart = "status.measurement_start" + eventTypeStatusProgress = "status.progress" + eventTypeStatusQueued = "status.queued" + eventTypeStatusReportCreate = "status.report_create" + eventTypeStatusResolverLookup = "status.resolver_lookup" + eventTypeStatusStarted = "status.started" ) // taskEmitter is anything that allows us to diff --git a/pkg/oonimkall/taskrunner.go b/pkg/oonimkall/taskrunner.go index dc224821e..1a271cdb5 100644 --- a/pkg/oonimkall/taskrunner.go +++ b/pkg/oonimkall/taskrunner.go @@ -123,9 +123,6 @@ func (r *runnerForTask) Run(rootCtx context.Context) { // - measCtx derives from rootCtx and is possibly tied to the // maximum runtime and is used to choose when to stop measuring; // - // - submitCtx is like measCtx but, in case we're using a max - // runtime, is given more time to finish submitting. - // // See https://github.com/ooni/probe/issues/2037. var logger model.Logger = newTaskLogger(r.emitter, r.settings.LogLevel) r.emitter.Emit(eventTypeStatusQueued, eventEmpty{}) @@ -221,28 +218,11 @@ func (r *runnerForTask) Run(rootCtx context.Context) { endEvent.UploadedKB = experiment.KibiBytesSent() }() - // open a new report if possible - if !r.settings.Options.NoCollector { - logger.Info("Opening report... please, be patient") - if err := experiment.OpenReportContext(rootCtx); err != nil { - r.emitter.EmitFailureGeneric(eventTypeFailureReportCreate, err.Error()) - return - } - r.emitter.EmitStatusProgress(0.4, "open report") - r.emitter.Emit(eventTypeStatusReportCreate, eventStatusReportGeneric{ - ReportID: experiment.ReportID(), - }) - } - // create the default context for measuring measCtx, measCancel := context.WithCancel(rootCtx) defer measCancel() - // create the default context for submitting - submitCtx, submitCancel := context.WithCancel(rootCtx) - defer submitCancel() - - // Update measCtx and submitCtx to be timeout bound in case there's + // Update measCtx to be timeout bound in case there's // more than one input/target to measure. // // This deviates a little bit from measurement-kit, for which @@ -258,19 +238,11 @@ func (r *runnerForTask) Run(rootCtx context.Context) { // See https://github.com/measurement-kit/measurement-kit/issues/1922 if r.settings.Options.MaxRuntime > 0 && len(targets) > 1 { var ( - cancelMeas context.CancelFunc - cancelSubmit context.CancelFunc + cancelMeas context.CancelFunc ) - // We give the context used for submitting extra time so that - // it's possible to submit the last measurement. - // - // See https://github.com/ooni/probe/issues/2037 for more info. maxRuntime := time.Duration(r.settings.Options.MaxRuntime) * time.Second measCtx, cancelMeas = context.WithTimeout(measCtx, maxRuntime) defer cancelMeas() - maxRuntime += 30 * time.Second - submitCtx, cancelSubmit = context.WithTimeout(submitCtx, maxRuntime) - defer cancelSubmit() } // prepare for cycling through the targets @@ -319,7 +291,7 @@ func (r *runnerForTask) Run(rootCtx context.Context) { // Handle the case where our time for measuring has elapsed while // we were measuring and assume the context interrupted the measurement - // midway, so it doesn't make sense to submit it. + // midway if builder.Interruptible() && measCtx.Err() != nil { break } @@ -352,20 +324,6 @@ func (r *runnerForTask) Run(rootCtx context.Context) { JSONStr: string(data), }) - // if possible, submit the measurement to the OONI backend - if !r.settings.Options.NoCollector { - logger.Info("Submitting measurement... please, be patient") - muid, err := experiment.SubmitAndUpdateMeasurementContext(submitCtx, m) - warnOnFailure(logger, "cannot submit measurement", err) - r.emitter.Emit(measurementSubmissionEventName(err), eventMeasurementGeneric{ - Idx: int64(idx), - Input: target.Input(), - JSONStr: string(data), - Failure: measurementSubmissionFailure(err), - MeasurementUID: muid, - }) - } - // let the app know that we're done measuring this entry r.emitter.Emit(eventTypeStatusMeasurementDone, eventMeasurementGeneric{ Idx: int64(idx), @@ -379,17 +337,3 @@ func warnOnFailure(logger model.Logger, message string, err error) { logger.Warnf("%s: %s (%+v)", message, err.Error(), err) } } - -func measurementSubmissionEventName(err error) string { - if err != nil { - return eventTypeFailureMeasurementSubmission - } - return eventTypeStatusMeasurementSubmission -} - -func measurementSubmissionFailure(err error) string { - if err != nil { - return err.Error() - } - return "" -} diff --git a/pkg/oonimkall/taskrunner_test.go b/pkg/oonimkall/taskrunner_test.go index f59997307..a50aee64c 100644 --- a/pkg/oonimkall/taskrunner_test.go +++ b/pkg/oonimkall/taskrunner_test.go @@ -12,24 +12,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/model" ) -func TestMeasurementSubmissionEventName(t *testing.T) { - if measurementSubmissionEventName(nil) != eventTypeStatusMeasurementSubmission { - t.Fatal("unexpected submission event name") - } - if measurementSubmissionEventName(errors.New("mocked error")) != eventTypeFailureMeasurementSubmission { - t.Fatal("unexpected submission event name") - } -} - -func TestMeasurementSubmissionFailure(t *testing.T) { - if measurementSubmissionFailure(nil) != "" { - t.Fatal("unexpected submission failure") - } - if measurementSubmissionFailure(errors.New("mocked error")) != "mocked error" { - t.Fatal("unexpected submission failure") - } -} - // MockableTaskRunnerDependencies is the mockable struct used by [TestTaskRunnerRun]. type MockableTaskRunnerDependencies struct { Builder *mocks.ExperimentBuilder @@ -399,27 +381,6 @@ func TestTaskRunnerRun(t *testing.T) { assertReducedEventsLike(t, expect, reduced) }) - t.Run("with failure opening report", func(t *testing.T) { - runner, emitter := newRunnerForTesting() - fake := fakeSuccessfulDeps() - fake.Experiment.MockOpenReportContext = func(ctx context.Context) error { - return errors.New("mocked error") - } - runner.newSession = fake.NewSession - events := runAndCollect(runner, emitter) - reduced := reduceEventsKeysIgnoreLog(t, events) - expect := []eventKeyCount{ - {Key: eventTypeStatusQueued, Count: 1}, - {Key: eventTypeStatusStarted, Count: 1}, - {Key: eventTypeStatusProgress, Count: 3}, - {Key: eventTypeStatusGeoIPLookup, Count: 1}, - {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeFailureReportCreate, Count: 1}, - {Key: eventTypeStatusEnd, Count: 1}, - } - assertReducedEventsLike(t, expect, reduced) - }) - t.Run("with success and just a single entry", func(t *testing.T) { runner, emitter := newRunnerForTesting() fake := fakeSuccessfulDeps() @@ -432,11 +393,8 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, {Key: eventTypeStatusEnd, Count: 1}, } @@ -458,8 +416,6 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeFailureMeasurement, Count: 1}, {Key: eventTypeStatusEnd, Count: 1}, @@ -488,8 +444,6 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeFailureMeasurement, Count: 1}, {Key: eventTypeStatusEnd, Count: 1}, @@ -524,31 +478,25 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusEnd, Count: 1}, @@ -586,19 +534,15 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, // {Key: eventTypeStatusEnd, Count: 1}, @@ -640,8 +584,6 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, // {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, @@ -651,51 +593,6 @@ func TestTaskRunnerRun(t *testing.T) { assertReducedEventsLike(t, expect, reduced) }) - t.Run("with measurement submission failure", func(t *testing.T) { - // Implementation note: this experiment needs a non-empty input otherwise the - // code will not emit a progress event when it finished measuring the input and - // we would be missing the eventTypeStatusProgress event. - inputs := []string{"a"} - runner, emitter := newRunnerForTesting() - runner.settings.Inputs = inputs // this is basically ignored because we override MockLoad - fake := fakeSuccessfulDeps() - fake.Builder.MockNewTargetLoader = func(config *model.ExperimentTargetLoaderConfig) model.ExperimentTargetLoader { - return &mocks.ExperimentTargetLoader{ - MockLoad: func(ctx context.Context) (targets []model.ExperimentTarget, err error) { - // We need to mimic what would happen when settings.Inputs is explicitly provided - for _, input := range inputs { - targets = append(targets, model.NewOOAPIURLInfoWithDefaultCategoryAndCountry(input)) - } - return - }, - } - } - fake.Experiment.MockSubmitAndUpdateMeasurementContext = func(ctx context.Context, measurement *model.Measurement) (string, error) { - return "", errors.New("cannot submit") - } - runner.newSession = fake.NewSession - events := runAndCollect(runner, emitter) - reduced := reduceEventsKeysIgnoreLog(t, events) - expect := []eventKeyCount{ - {Key: eventTypeStatusQueued, Count: 1}, - {Key: eventTypeStatusStarted, Count: 1}, - {Key: eventTypeStatusProgress, Count: 3}, - {Key: eventTypeStatusGeoIPLookup, Count: 1}, - {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, - // - {Key: eventTypeStatusMeasurementStart, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeFailureMeasurementSubmission, Count: 1}, - {Key: eventTypeStatusMeasurementDone, Count: 1}, - // - {Key: eventTypeStatusEnd, Count: 1}, - } - assertReducedEventsLike(t, expect, reduced) - }) - t.Run("with success and progress", func(t *testing.T) { runner, emitter := newRunnerForTesting() fake := fakeSuccessfulDeps() @@ -716,12 +613,9 @@ func TestTaskRunnerRun(t *testing.T) { {Key: eventTypeStatusProgress, Count: 3}, {Key: eventTypeStatusGeoIPLookup, Count: 1}, {Key: eventTypeStatusResolverLookup, Count: 1}, - {Key: eventTypeStatusProgress, Count: 1}, - {Key: eventTypeStatusReportCreate, Count: 1}, {Key: eventTypeStatusMeasurementStart, Count: 1}, {Key: eventTypeStatusProgress, Count: 1}, {Key: eventTypeMeasurement, Count: 1}, - {Key: eventTypeStatusMeasurementSubmission, Count: 1}, {Key: eventTypeStatusMeasurementDone, Count: 1}, {Key: eventTypeStatusEnd, Count: 1}, }