Skip to content

Commit

Permalink
oonimkall: mobile api for running WebConnectivity (#223)
Browse files Browse the repository at this point in the history
* poc: mobile api for running WebConnectivity

Yesterday we had a team meeting where we discussed the importance
of stopping using MK wrappers for running experiments.

Here's a PoC showing how we can do that for WebConnectivity. It
seems to me we probably want to add more tests and merge this code
such that we can experiment with it quite soon.

There seems to be opportunities for auto-generating code, BTW.

While working on this PoC, I've also took a chance to revamp the
external documentation of pkg/oonimkall.

* feat: start making the code more abstract

* chore: write unit tests for new code

* fix(oonimkall): improve naming

* refactor: cosmetic changes

* fix: explain
  • Loading branch information
bassosimone authored Mar 18, 2021
1 parent 53e6b69 commit 0f61778
Show file tree
Hide file tree
Showing 6 changed files with 481 additions and 17 deletions.
96 changes: 96 additions & 0 deletions pkg/oonimkall/experiment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package oonimkall

import (
"context"

"github.com/ooni/probe-cli/v3/internal/engine"
"github.com/ooni/probe-cli/v3/internal/engine/model"
)

// experimentSession is the abstract representation of
// a session according to an experiment.
type experimentSession interface {
// lock locks the session
lock()

// maybeLookupBackends lookups the backends
maybeLookupBackends(ctx context.Context) error

// maybeLookupLocations lookups the probe location
maybeLookupLocation(ctx context.Context) error

// newExperimentBuilder creates a new experiment builder
newExperimentBuilder(name string) (experimentBuilder, error)

// unlock unlocks the session
unlock()
}

// lock implements experimentSession.lock
func (sess *Session) lock() {
sess.mtx.Lock()
}

// maybeLookupBackends implements experimentSession.maybeLookupBackends
func (sess *Session) maybeLookupBackends(ctx context.Context) error {
return sess.sessp.MaybeLookupBackendsContext(ctx)
}

// maybeLookupLocation implements experimentSession.maybeLookupLocation
func (sess *Session) maybeLookupLocation(ctx context.Context) error {
return sess.sessp.MaybeLookupLocationContext(ctx)
}

// newExperimentBuilder implements experimentSession.newExperimentBuilder
func (sess *Session) newExperimentBuilder(name string) (experimentBuilder, error) {
eb, err := sess.sessp.NewExperimentBuilder(name)
if err != nil {
return nil, err
}
return &experimentBuilderWrapper{eb: eb}, nil
}

// unlock implements experimentSession.unlock
func (sess *Session) unlock() {
sess.mtx.Unlock()
}

// experimentBuilder is the representation of an experiment
// builder that we use inside this package.
type experimentBuilder interface {
// newExperiment creates a new experiment instance
newExperiment() experiment

// setCallbacks sets the experiment callbacks
setCallbacks(ExperimentCallbacks)
}

// experimentBuilderWrapper wraps *ExperimentBuilder
type experimentBuilderWrapper struct {
eb *engine.ExperimentBuilder
}

// newExperiment implements experimentBuilder.newExperiment
func (eb *experimentBuilderWrapper) newExperiment() experiment {
return eb.eb.NewExperiment()
}

// setCallbacks implements experimentBuilder.setCallbacks
func (eb *experimentBuilderWrapper) setCallbacks(cb ExperimentCallbacks) {
eb.eb.SetCallbacks(cb)
}

// experiment is the representation of an experiment that
// we use inside this package for running nettests.
type experiment interface {
// MeasureWithContext runs the measurement with the given input
// and context. It returns a measurement or an error.
MeasureWithContext(ctx context.Context, input string) (
measurement *model.Measurement, err error)

// KibiBytesSent returns the number of KiB sent.
KibiBytesSent() float64

// KibiBytesReceived returns the number of KiB received.
KibiBytesReceived() float64
}
93 changes: 93 additions & 0 deletions pkg/oonimkall/experiment_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package oonimkall

import (
"context"
"sync"
"sync/atomic"

"github.com/ooni/probe-cli/v3/internal/engine/model"
)

// FakeExperimentCallbacks contains fake ExperimentCallbacks.
type FakeExperimentCallbacks struct{}

// OnProgress implements ExperimentCallbacks.OnProgress
func (cb *FakeExperimentCallbacks) OnProgress(percentage float64, message string) {}

// FakeExperimentSession is a fake experimentSession
type FakeExperimentSession struct {
ExperimentBuilder experimentBuilder
LockCount int32
LookupBackendsErr error
LookupLocationErr error
NewExperimentBuilderErr error
UnlockCount int32
}

// lock implements experimentSession.lock
func (sess *FakeExperimentSession) lock() {
atomic.AddInt32(&sess.LockCount, 1)
}

// maybeLookupBackends implements experimentSession.maybeLookupBackends
func (sess *FakeExperimentSession) maybeLookupBackends(ctx context.Context) error {
return sess.LookupBackendsErr
}

// maybeLookupLocation implements experimentSession.maybeLookupLocation
func (sess *FakeExperimentSession) maybeLookupLocation(ctx context.Context) error {
return sess.LookupLocationErr
}

// newExperimentBuilder implements experimentSession.newExperimentBuilder
func (sess *FakeExperimentSession) newExperimentBuilder(name string) (experimentBuilder, error) {
return sess.ExperimentBuilder, sess.NewExperimentBuilderErr
}

// unlock implements experimentSession.unlock
func (sess *FakeExperimentSession) unlock() {
atomic.AddInt32(&sess.UnlockCount, 1)
}

// FakeExperimentBuilder is a fake experimentBuilder
type FakeExperimentBuilder struct {
Callbacks ExperimentCallbacks
Experiment experiment
mu sync.Mutex
}

// newExperiment implements experimentBuilder.newExperiment
func (eb *FakeExperimentBuilder) newExperiment() experiment {
return eb.Experiment
}

// setCallbacks implements experimentBuilder.setCallbacks
func (eb *FakeExperimentBuilder) setCallbacks(cb ExperimentCallbacks) {
defer eb.mu.Unlock()
eb.mu.Lock()
eb.Callbacks = cb
}

// FakeExperiment is a fake experiment
type FakeExperiment struct {
Err error
Measurement *model.Measurement
Received float64
Sent float64
}

// MeasureWithContext implements experiment.MeasureWithContext.
func (e *FakeExperiment) MeasureWithContext(ctx context.Context, input string) (
measurement *model.Measurement, err error) {
return e.Measurement, e.Err
}

// KibiBytesSent implements experiment.KibiBytesSent
func (e *FakeExperiment) KibiBytesSent() float64 {
return e.Sent
}

// KibiBytesReceived implements experiment.KibiBytesReceived
func (e *FakeExperiment) KibiBytesReceived() float64 {
return e.Received
}
34 changes: 17 additions & 17 deletions pkg/oonimkall/session_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/ooni/probe-cli/v3/pkg/oonimkall"
)

func NewSessionWithAssetsDir(assetsDir string) (*oonimkall.Session, error) {
func NewSessionForTestingWithAssetsDir(assetsDir string) (*oonimkall.Session, error) {
return oonimkall.NewSession(&oonimkall.SessionConfig{
AssetsDir: assetsDir,
ProbeServicesURL: "https://ams-pg-test.ooni.org/",
Expand All @@ -29,8 +29,8 @@ func NewSessionWithAssetsDir(assetsDir string) (*oonimkall.Session, error) {
})
}

func NewSession() (*oonimkall.Session, error) {
return NewSessionWithAssetsDir("../testdata/oonimkall/assets")
func NewSessionForTesting() (*oonimkall.Session, error) {
return NewSessionForTestingWithAssetsDir("../testdata/oonimkall/assets")
}

func TestNewSessionWithInvalidStateDir(t *testing.T) {
Expand Down Expand Up @@ -72,7 +72,7 @@ func TestMaybeUpdateResourcesWithCancelledContext(t *testing.T) {
t.Fatal(err)
}
defer os.RemoveAll(dir)
sess, err := NewSessionWithAssetsDir(dir)
sess, err := NewSessionForTestingWithAssetsDir(dir)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestGeolocateWithCancelledContext(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -123,7 +123,7 @@ func TestGeolocateGood(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestSubmitWithCancelledContext(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -182,7 +182,7 @@ func TestSubmitWithInvalidJSON(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -234,7 +234,7 @@ func TestSubmitMeasurementGood(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -248,7 +248,7 @@ func TestSubmitCancelContextAfterFirstSubmission(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -267,7 +267,7 @@ func TestSubmitCancelContextAfterFirstSubmission(t *testing.T) {
}

func TestCheckInSuccess(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -315,7 +315,7 @@ func TestCheckInSuccess(t *testing.T) {
}

func TestCheckInLookupLocationFailure(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -342,7 +342,7 @@ func TestCheckInLookupLocationFailure(t *testing.T) {
}

func TestCheckInNewProbeServicesFailure(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -371,7 +371,7 @@ func TestCheckInNewProbeServicesFailure(t *testing.T) {
}

func TestCheckInCheckInFailure(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -400,7 +400,7 @@ func TestCheckInCheckInFailure(t *testing.T) {
}

func TestCheckInNoParams(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -423,7 +423,7 @@ func TestCheckInNoParams(t *testing.T) {
}

func TestFetchURLListSuccess(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand All @@ -448,7 +448,7 @@ func TestFetchURLListSuccess(t *testing.T) {
}

func TestFetchURLListWithCC(t *testing.T) {
sess, err := NewSession()
sess, err := NewSessionForTesting()
if err != nil {
t.Fatal(err)
}
Expand Down
Loading

0 comments on commit 0f61778

Please sign in to comment.