refactor: merge psiphonx and torx into tunnel (#287)

* refactor: merge psiphonx and torx into tunnel

This is a case where it seems that merging these three packages into
a single package will enable us to better the implementation.

The goal is still https://github.com/ooni/probe/issues/985.

The roadblock I'm trying to overcome is
https://github.com/ooni/probe-cli/pull/286#pullrequestreview-627460104.

* avoid duplicating logger for now

internal/engine/experiment/urlgetter/getter.go

@@ -6,11 +6,11 @@ import (
 	"path/filepath"
 	"time"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/tunnel"
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/archival"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/errorx"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/trace"
+	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
 )
 
 // The Getter gets the specified target in the context of the

internal/engine/internal/mockable/mockable.go

@@ -6,9 +6,6 @@ import (
 	"net/http"
 	"net/url"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/psiphonx"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/torx"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/tunnel"
 	"github.com/ooni/probe-cli/v3/internal/engine/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
 	"github.com/ooni/probe-cli/v3/internal/engine/probeservices"
@@ -145,6 +142,3 @@ func (sess *Session) UserAgent() string {
 
 var _ model.ExperimentSession = &Session{}
 var _ probeservices.Session = &Session{}
-var _ psiphonx.Session = &Session{}
-var _ tunnel.Session = &Session{}
-var _ torx.Session = &Session{}

internal/engine/internal/psiphonx/private/.gitignore

@@ -1 +0,0 @@
-/psiphon.gz

internal/engine/internal/psiphonx/psiphonx_test.go

@@ -1,160 +0,0 @@
-package psiphonx_test
-
-import (
-	"context"
-	"errors"
-	"os"
-	"testing"
-
-	"github.com/apex/log"
-	engine "github.com/ooni/probe-cli/v3/internal/engine"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/mockable"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/psiphonx"
-	"github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
-)
-
-func TestStartWithCancelledContext(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-	sess, err := engine.NewSession(engine.SessionConfig{
-		Logger:          log.Log,
-		SoftwareName:    "ooniprobe-engine",
-		SoftwareVersion: "0.0.1",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	tunnel, err := psiphonx.Start(ctx, sess, psiphonx.Config{})
-	if !errors.Is(err, context.Canceled) {
-		t.Fatal("not the error we expected")
-	}
-	if tunnel != nil {
-		t.Fatal("expected nil tunnel here")
-	}
-}
-
-func TestStartStop(t *testing.T) {
-	if testing.Short() {
-		t.Skip("skip test in short mode")
-	}
-	sess, err := engine.NewSession(engine.SessionConfig{
-		Logger:          log.Log,
-		SoftwareName:    "ooniprobe-engine",
-		SoftwareVersion: "0.0.1",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if tunnel.SOCKS5ProxyURL() == nil {
-		t.Fatal("expected non nil URL here")
-	}
-	if tunnel.BootstrapTime() <= 0 {
-		t.Fatal("expected positive bootstrap time here")
-	}
-	tunnel.Stop()
-}
-
-func TestFetchPsiphonConfigFailure(t *testing.T) {
-	expected := errors.New("mocked error")
-	sess := &mockable.Session{
-		MockableFetchPsiphonConfigErr: expected,
-	}
-	tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{})
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if tunnel != nil {
-		t.Fatal("expected nil tunnel here")
-	}
-}
-
-func TestMakeMkdirAllFailure(t *testing.T) {
-	expected := errors.New("mocked error")
-	dependencies := FakeDependencies{
-		MkdirAllErr: expected,
-	}
-	sess := &mockable.Session{
-		MockableFetchPsiphonConfigResult: []byte(`{}`),
-	}
-	tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{
-		Dependencies: dependencies,
-	})
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if tunnel != nil {
-		t.Fatal("expected nil tunnel here")
-	}
-}
-
-func TestMakeRemoveAllFailure(t *testing.T) {
-	expected := errors.New("mocked error")
-	dependencies := FakeDependencies{
-		RemoveAllErr: expected,
-	}
-	sess := &mockable.Session{
-		MockableFetchPsiphonConfigResult: []byte(`{}`),
-	}
-	tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{
-		Dependencies: dependencies,
-	})
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if tunnel != nil {
-		t.Fatal("expected nil tunnel here")
-	}
-}
-
-func TestMakeStartFailure(t *testing.T) {
-	expected := errors.New("mocked error")
-	dependencies := FakeDependencies{
-		StartErr: expected,
-	}
-	sess := &mockable.Session{
-		MockableFetchPsiphonConfigResult: []byte(`{}`),
-	}
-	tunnel, err := psiphonx.Start(context.Background(), sess, psiphonx.Config{
-		Dependencies: dependencies,
-	})
-	if !errors.Is(err, expected) {
-		t.Fatal("not the error we expected")
-	}
-	if tunnel != nil {
-		t.Fatal("expected nil tunnel here")
-	}
-}
-
-func TestNilTunnel(t *testing.T) {
-	var tunnel *psiphonx.Tunnel
-	if tunnel.BootstrapTime() != 0 {
-		t.Fatal("expected zero bootstrap time")
-	}
-	if tunnel.SOCKS5ProxyURL() != nil {
-		t.Fatal("expected nil SOCKS Proxy URL")
-	}
-	tunnel.Stop() // must not crash
-}
-
-type FakeDependencies struct {
-	MkdirAllErr  error
-	RemoveAllErr error
-	StartErr     error
-}
-
-func (fd FakeDependencies) MkdirAll(path string, perm os.FileMode) error {
-	return fd.MkdirAllErr
-}
-
-func (fd FakeDependencies) RemoveAll(path string) error {
-	return fd.RemoveAllErr
-}
-
-func (fd FakeDependencies) Start(
-	ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) {
-	return nil, fd.StartErr
-}

internal/engine/internal/torx/torx_internal_test.go

@@ -1,14 +0,0 @@
-package torx
-
-import (
-	"net/url"
-	"time"
-)
-
-func NewTunnel(bootstrapTime time.Duration, instance TorProcess, proxy *url.URL) *Tunnel {
-	return &Tunnel{
-		bootstrapTime: bootstrapTime,
-		instance:      instance,
-		proxy:         proxy,
-	}
-}

internal/engine/session.go

@@ -14,12 +14,12 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
 	"github.com/ooni/probe-cli/v3/internal/engine/internal/platform"
 	"github.com/ooni/probe-cli/v3/internal/engine/internal/sessionresolver"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/tunnel"
 	"github.com/ooni/probe-cli/v3/internal/engine/kvstore"
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx"
 	"github.com/ooni/probe-cli/v3/internal/engine/netx/bytecounter"
 	"github.com/ooni/probe-cli/v3/internal/engine/probeservices"
+	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
 	"github.com/ooni/probe-cli/v3/internal/version"
 )
 

internal/engine/tunnel/integration_test.go

@@ -0,0 +1,62 @@
+package tunnel_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/engine"
+	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
+)
+
+func TestPsiphonStartWithCancelledContext(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	sess, err := engine.NewSession(engine.SessionConfig{
+		Logger:          log.Log,
+		SoftwareName:    "ooniprobe-engine",
+		SoftwareVersion: "0.0.1",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	tunnel, err := tunnel.Start(ctx, tunnel.Config{
+		Name:    "psiphon",
+		Session: sess,
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonStartStop(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	sess, err := engine.NewSession(engine.SessionConfig{
+		Logger:          log.Log,
+		SoftwareName:    "ooniprobe-engine",
+		SoftwareVersion: "0.0.1",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	tunnel, err := tunnel.Start(context.Background(), tunnel.Config{
+		Name:    "psiphon",
+		Session: sess,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if tunnel.SOCKS5ProxyURL() == nil {
+		t.Fatal("expected non nil URL here")
+	}
+	if tunnel.BootstrapTime() <= 0 {
+		t.Fatal("expected positive bootstrap time here")
+	}
+	tunnel.Stop()
+}

internal/engine/tunnel/psiphon.go

@@ -1,5 +1,4 @@
-// Package psiphonx is a wrapper around the psiphon-tunnel-core.
+package tunnel
-package psiphonx
 
 import (
 	"context"
@@ -13,14 +12,8 @@ import (
 	"github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
 )
 
-// Session is the way in which this package sees a Session.
+// psiphonDependencies contains dependencies for psiphonStart
-type Session interface {
+type psiphonDependencies interface {
-	FetchPsiphonConfig(ctx context.Context) ([]byte, error)
-	TempDir() string
-}
-
-// Dependencies contains dependencies for Start
-type Dependencies interface {
 	MkdirAll(path string, perm os.FileMode) error
 	RemoveAll(path string) error
 	Start(ctx context.Context, config []byte,
@@ -43,24 +36,24 @@ func (defaultDependencies) Start(
 		DataRootDirectory: &workdir}, nil, nil)
 }
 
-// Config contains the settings for Start. The empty config object implies
+// psiphonConfig contains the settings for psiphonStart. The empty config object implies
 // that we will be using default settings for starting the tunnel.
-type Config struct {
+type psiphonConfig struct {
 	// Dependencies contains dependencies for Start.
-	Dependencies Dependencies
+	Dependencies psiphonDependencies
 
 	// WorkDir is the directory where Psiphon should store
 	// its configuration database.
 	WorkDir string
 }
 
-// Tunnel is a psiphon tunnel
+// psiphonTunnel is a psiphon tunnel
-type Tunnel struct {
+type psiphonTunnel struct {
 	tunnel   *clientlib.PsiphonTunnel
 	duration time.Duration
 }
 
-func makeworkingdir(config Config) (string, error) {
+func makeworkingdir(config psiphonConfig) (string, error) {
 	const testdirname = "oonipsiphon"
 	workdir := filepath.Join(config.WorkDir, testdirname)
 	if err := config.Dependencies.RemoveAll(workdir); err != nil {
@@ -72,9 +65,9 @@ func makeworkingdir(config Config) (string, error) {
 	return workdir, nil
 }
 
-// Start starts the psiphon tunnel.
+// psiphonStart starts the psiphon tunnel.
-func Start(
+func psiphonStart(
-	ctx context.Context, sess Session, config Config) (*Tunnel, error) {
+	ctx context.Context, sess Session, config psiphonConfig) (Tunnel, error) {
 	select {
 	case <-ctx.Done():
 		return nil, ctx.Err() // simplifies unit testing this code
@@ -100,18 +93,18 @@ func Start(
 		return nil, err
 	}
 	stop := time.Now()
-	return &Tunnel{tunnel: tunnel, duration: stop.Sub(start)}, nil
+	return &psiphonTunnel{tunnel: tunnel, duration: stop.Sub(start)}, nil
 }
 
 // Stop is an idempotent method that shuts down the tunnel
-func (t *Tunnel) Stop() {
+func (t *psiphonTunnel) Stop() {
 	if t != nil {
 		t.tunnel.Stop()
 	}
 }
 
 // SOCKS5ProxyURL returns the SOCKS5 proxy URL.
-func (t *Tunnel) SOCKS5ProxyURL() (proxyURL *url.URL) {
+func (t *psiphonTunnel) SOCKS5ProxyURL() (proxyURL *url.URL) {
 	if t != nil {
 		proxyURL = &url.URL{
 			Scheme: "socks5",
@@ -123,7 +116,7 @@ func (t *Tunnel) SOCKS5ProxyURL() (proxyURL *url.URL) {
 }
 
 // BootstrapTime returns the bootstrap time
-func (t *Tunnel) BootstrapTime() (duration time.Duration) {
+func (t *psiphonTunnel) BootstrapTime() (duration time.Duration) {
 	if t != nil {
 		duration = t.duration
 	}

internal/engine/tunnel/psiphon_test.go

@@ -0,0 +1,112 @@
+package tunnel
+
+import (
+	"context"
+	"errors"
+	"os"
+	"testing"
+
+	"github.com/ooni/probe-cli/v3/internal/engine/internal/mockable"
+	"github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
+)
+
+func TestPsiphonFetchPsiphonConfigFailure(t *testing.T) {
+	expected := errors.New("mocked error")
+	sess := &mockable.Session{
+		MockableFetchPsiphonConfigErr: expected,
+	}
+	tunnel, err := psiphonStart(context.Background(), sess, psiphonConfig{})
+	if !errors.Is(err, expected) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonMakeMkdirAllFailure(t *testing.T) {
+	expected := errors.New("mocked error")
+	dependencies := psiphonFakeDependencies{
+		MkdirAllErr: expected,
+	}
+	sess := &mockable.Session{
+		MockableFetchPsiphonConfigResult: []byte(`{}`),
+	}
+	tunnel, err := psiphonStart(context.Background(), sess, psiphonConfig{
+		Dependencies: dependencies,
+	})
+	if !errors.Is(err, expected) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonMakeRemoveAllFailure(t *testing.T) {
+	expected := errors.New("mocked error")
+	dependencies := psiphonFakeDependencies{
+		RemoveAllErr: expected,
+	}
+	sess := &mockable.Session{
+		MockableFetchPsiphonConfigResult: []byte(`{}`),
+	}
+	tunnel, err := psiphonStart(context.Background(), sess, psiphonConfig{
+		Dependencies: dependencies,
+	})
+	if !errors.Is(err, expected) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonMakeStartFailure(t *testing.T) {
+	expected := errors.New("mocked error")
+	dependencies := psiphonFakeDependencies{
+		StartErr: expected,
+	}
+	sess := &mockable.Session{
+		MockableFetchPsiphonConfigResult: []byte(`{}`),
+	}
+	tunnel, err := psiphonStart(context.Background(), sess, psiphonConfig{
+		Dependencies: dependencies,
+	})
+	if !errors.Is(err, expected) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonNilTunnel(t *testing.T) {
+	var tunnel *psiphonTunnel
+	if tunnel.BootstrapTime() != 0 {
+		t.Fatal("expected zero bootstrap time")
+	}
+	if tunnel.SOCKS5ProxyURL() != nil {
+		t.Fatal("expected nil SOCKS Proxy URL")
+	}
+	tunnel.Stop() // must not crash
+}
+
+type psiphonFakeDependencies struct {
+	MkdirAllErr  error
+	RemoveAllErr error
+	StartErr     error
+}
+
+func (fd psiphonFakeDependencies) MkdirAll(path string, perm os.FileMode) error {
+	return fd.MkdirAllErr
+}
+
+func (fd psiphonFakeDependencies) RemoveAll(path string) error {
+	return fd.RemoveAllErr
+}
+
+func (fd psiphonFakeDependencies) Start(
+	ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) {
+	return nil, fd.StartErr
+}

internal/engine/tunnel/tor.go

@@ -1,5 +1,4 @@
-// Package torx contains code to control tor.
+package tunnel
-package torx
 
 import (
 	"context"
@@ -13,27 +12,20 @@ import (
 	"github.com/cretz/bine/tor"
 )
 
-// Session is the way in which this package sees a Session.
+// torProcess is a running tor process
-type Session interface {
+type torProcess interface {
-	TempDir() string
-	TorArgs() []string
-	TorBinary() string
-}
-
-// TorProcess is a running tor process
-type TorProcess interface {
 	Close() error
 }
 
-// Tunnel is the Tor tunnel
+// torTunnel is the Tor tunnel
-type Tunnel struct {
+type torTunnel struct {
 	bootstrapTime time.Duration
-	instance      TorProcess
+	instance      torProcess
 	proxy         *url.URL
 }
 
 // BootstrapTime is the bootstrsap time
-func (tt *Tunnel) BootstrapTime() (duration time.Duration) {
+func (tt *torTunnel) BootstrapTime() (duration time.Duration) {
 	if tt != nil {
 		duration = tt.bootstrapTime
 	}
@@ -41,7 +33,7 @@ func (tt *Tunnel) BootstrapTime() (duration time.Duration) {
 }
 
 // SOCKS5ProxyURL returns the URL of the SOCKS5 proxy
-func (tt *Tunnel) SOCKS5ProxyURL() (url *url.URL) {
+func (tt *torTunnel) SOCKS5ProxyURL() (url *url.URL) {
 	if tt != nil {
 		url = tt.proxy
 	}
@@ -49,23 +41,23 @@ func (tt *Tunnel) SOCKS5ProxyURL() (url *url.URL) {
 }
 
 // Stop stops the Tor tunnel
-func (tt *Tunnel) Stop() {
+func (tt *torTunnel) Stop() {
 	if tt != nil {
 		tt.instance.Close()
 	}
 }
 
-// StartConfig contains the configuration for StartWithConfig
+// torStartConfig contains the configuration for StartWithConfig
-type StartConfig struct {
+type torStartConfig struct {
 	Sess          Session
 	Start         func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error)
 	EnableNetwork func(ctx context.Context, tor *tor.Tor, wait bool) error
 	GetInfo       func(ctrl *control.Conn, keys ...string) ([]*control.KeyVal, error)
 }
 
-// Start starts the tor tunnel
+// torStart starts the tor tunnel
-func Start(ctx context.Context, sess Session) (*Tunnel, error) {
+func torStart(ctx context.Context, sess Session) (Tunnel, error) {
-	return StartWithConfig(ctx, StartConfig{
+	return torStartWithConfig(ctx, torStartConfig{
 		Sess: sess,
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return tor.Start(ctx, conf)
@@ -79,8 +71,8 @@ func Start(ctx context.Context, sess Session) (*Tunnel, error) {
 	})
 }
 
-// StartWithConfig is a configurable Start for testing
+// torStartWithConfig is a configurable torStart for testing
-func StartWithConfig(ctx context.Context, config StartConfig) (*Tunnel, error) {
+func torStartWithConfig(ctx context.Context, config torStartConfig) (Tunnel, error) {
 	select {
 	case <-ctx.Done():
 		return nil, ctx.Err() // allows to write unit tests using this code
@@ -123,7 +115,7 @@ func StartWithConfig(ctx context.Context, config StartConfig) (*Tunnel, error) {
 		instance.Close()
 		return nil, fmt.Errorf("tor returned unsupported proxy")
 	}
-	return &Tunnel{
+	return &torTunnel{
 		bootstrapTime: stop.Sub(start),
 		instance:      instance,
 		proxy:         &url.URL{Scheme: "socks5", Host: proxyAddress},
@@ -135,3 +127,12 @@ func StartWithConfig(ctx context.Context, config StartConfig) (*Tunnel, error) {
 func LogFile(sess Session) string {
 	return path.Join(sess.TempDir(), "tor.log")
 }
+
+// newTorTunnel creates a new torTunnel
+func newTorTunnel(bootstrapTime time.Duration, instance torProcess, proxy *url.URL) *torTunnel {
+	return &torTunnel{
+		bootstrapTime: bootstrapTime,
+		instance:      instance,
+		proxy:         proxy,
+	}
+}

internal/engine/tunnel/tor_test.go

@@ -1,4 +1,4 @@
-package torx_test
+package tunnel
 
 import (
 	"context"
@@ -9,7 +9,6 @@ import (
 	"github.com/cretz/bine/control"
 	"github.com/cretz/bine/tor"
 	"github.com/ooni/probe-cli/v3/internal/engine/internal/mockable"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/torx"
 )
 
 type Closer struct {
@@ -21,10 +20,10 @@ func (c *Closer) Close() error {
 	return errors.New("mocked mocked mocked")
 }
 
-func TestTunnelNonNil(t *testing.T) {
+func TestTorTunnelNonNil(t *testing.T) {
 	closer := new(Closer)
 	proxy := &url.URL{Scheme: "x", Host: "10.0.0.1:443"}
-	tun := torx.NewTunnel(128, closer, proxy)
+	tun := newTorTunnel(128, closer, proxy)
 	if tun.BootstrapTime() != 128 {
 		t.Fatal("not the bootstrap time we expected")
 	}
@@ -37,8 +36,8 @@ func TestTunnelNonNil(t *testing.T) {
 	}
 }
 
-func TestTunnelNil(t *testing.T) {
+func TestTorTunnelNil(t *testing.T) {
-	var tun *torx.Tunnel
+	var tun *torTunnel
 	if tun.BootstrapTime() != 0 {
 		t.Fatal("not the bootstrap time we expected")
 	}
@@ -48,10 +47,10 @@ func TestTunnelNil(t *testing.T) {
 	tun.Stop() // ensure we don't crash
 }
 
-func TestStartWithCancelledContext(t *testing.T) {
+func TestTorStartWithCancelledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	tun, err := torx.Start(ctx, &mockable.Session{})
+	tun, err := torStart(ctx, &mockable.Session{})
 	if !errors.Is(err, context.Canceled) {
 		t.Fatal("not the error we expected")
 	}
@@ -60,10 +59,10 @@ func TestStartWithCancelledContext(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigStartFailure(t *testing.T) {
+func TestTorStartWithConfigStartFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return nil, expected
@@ -77,10 +76,10 @@ func TestStartWithConfigStartFailure(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigEnableNetworkFailure(t *testing.T) {
+func TestTorStartWithConfigEnableNetworkFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil
@@ -97,10 +96,10 @@ func TestStartWithConfigEnableNetworkFailure(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigGetInfoFailure(t *testing.T) {
+func TestTorStartWithConfigGetInfoFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil
@@ -120,9 +119,9 @@ func TestStartWithConfigGetInfoFailure(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigGetInfoInvalidNumberOfKeys(t *testing.T) {
+func TestTorStartWithConfigGetInfoInvalidNumberOfKeys(t *testing.T) {
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil
@@ -142,9 +141,9 @@ func TestStartWithConfigGetInfoInvalidNumberOfKeys(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigGetInfoInvalidKey(t *testing.T) {
+func TestTorStartWithConfigGetInfoInvalidKey(t *testing.T) {
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil
@@ -164,9 +163,9 @@ func TestStartWithConfigGetInfoInvalidKey(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigGetInfoInvalidProxyType(t *testing.T) {
+func TestTorStartWithConfigGetInfoInvalidProxyType(t *testing.T) {
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil
@@ -186,9 +185,9 @@ func TestStartWithConfigGetInfoInvalidProxyType(t *testing.T) {
 	}
 }
 
-func TestStartWithConfigSuccess(t *testing.T) {
+func TestTorStartWithConfigSuccess(t *testing.T) {
 	ctx := context.Background()
-	tun, err := torx.StartWithConfig(ctx, torx.StartConfig{
+	tun, err := torStartWithConfig(ctx, torStartConfig{
 		Sess: &mockable.Session{},
 		Start: func(ctx context.Context, conf *tor.StartConf) (*tor.Tor, error) {
 			return &tor.Tor{}, nil

internal/engine/tunnel/tunnel.go

@@ -1,4 +1,5 @@
-// Package tunnel contains code to create a psiphon or tor tunnel.
+// Package tunnel allows to create tunnels to speak
+// with OONI backends and other services.
 package tunnel
 
 import (
@@ -7,15 +8,15 @@ import (
 	"net/url"
 	"time"
 
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/psiphonx"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/torx"
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
 )
 
 // Session is the way in which this package sees a Session.
 type Session interface {
-	psiphonx.Session
+	FetchPsiphonConfig(ctx context.Context) ([]byte, error)
-	torx.Session
+	TempDir() string
+	TorArgs() []string
+	TorBinary() string
 	Logger() model.Logger
 }
 
@@ -43,13 +44,13 @@ func Start(ctx context.Context, config Config) (Tunnel, error) {
 		return enforceNilContract(nil, nil)
 	case "psiphon":
 		logger.Infof("starting %s tunnel; please be patient...", config.Name)
-		tun, err := psiphonx.Start(ctx, config.Session, psiphonx.Config{
+		tun, err := psiphonStart(ctx, config.Session, psiphonConfig{
 			WorkDir: config.WorkDir,
 		})
 		return enforceNilContract(tun, err)
 	case "tor":
 		logger.Infof("starting %s tunnel; please be patient...", config.Name)
-		tun, err := torx.Start(ctx, config.Session)
+		tun, err := torStart(ctx, config.Session)
 		return enforceNilContract(tun, err)
 	default:
 		return nil, errors.New("unsupported tunnel")

internal/engine/tunnel/tunnel_test.go

@@ -1,4 +1,4 @@
-package tunnel_test
+package tunnel
 
 import (
 	"context"
@@ -7,13 +7,12 @@ import (
 
 	"github.com/apex/log"
 	"github.com/ooni/probe-cli/v3/internal/engine/internal/mockable"
-	"github.com/ooni/probe-cli/v3/internal/engine/internal/tunnel"
 )
 
-func TestNoTunnel(t *testing.T) {
+func TestStartNoTunnel(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	tunnel, err := tunnel.Start(ctx, tunnel.Config{
+	tunnel, err := Start(ctx, Config{
 		Name: "",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
@@ -27,10 +26,10 @@ func TestNoTunnel(t *testing.T) {
 	}
 }
 
-func TestPsiphonTunnel(t *testing.T) {
+func TestStartPsiphonTunnel(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	tunnel, err := tunnel.Start(ctx, tunnel.Config{
+	tunnel, err := Start(ctx, Config{
 		Name: "psiphon",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
@@ -44,10 +43,10 @@ func TestPsiphonTunnel(t *testing.T) {
 	}
 }
 
-func TestTorTunnel(t *testing.T) {
+func TestStartTorTunnel(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	tunnel, err := tunnel.Start(ctx, tunnel.Config{
+	tunnel, err := Start(ctx, Config{
 		Name: "tor",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
@@ -61,10 +60,10 @@ func TestTorTunnel(t *testing.T) {
 	}
 }
 
-func TestInvalidTunnel(t *testing.T) {
+func TestStartInvalidTunnel(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
-	tunnel, err := tunnel.Start(ctx, tunnel.Config{
+	tunnel, err := Start(ctx, Config{
 		Name: "antani",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,