feat(tunnel): improve the test suite (#297)

Part of https://github.com/ooni/probe/issues/985

internal/engine/tunnel/psiphon_integration_test.go

@@ -2,7 +2,7 @@ package tunnel_test
 
 import (
 	"context"
-	"errors"
+	"io/ioutil"
 	"testing"
 
 	"github.com/apex/log"
@@ -10,43 +10,20 @@ import (
 	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
 )
 
-func TestPsiphonStartWithCancelledContext(t *testing.T) {
-	// TODO(bassosimone): this test can use a mockable session so we
-	// can move it inside of the internal tests.
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel() // fail immediately
-	sess, err := engine.NewSession(ctx, engine.SessionConfig{
-		Logger:          log.Log,
-		SoftwareName:    "miniooni",
-		SoftwareVersion: "0.1.0-dev",
-		TunnelDir:       "testdata",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-	tunnel, err := tunnel.Start(ctx, &tunnel.Config{
-		Name:      "psiphon",
-		Session:   sess,
-		TunnelDir: "testdata",
-	})
-	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")
 	}
+	tunnelDir, err := ioutil.TempDir("testdata", "psiphon")
+	if err != nil {
+		t.Fatal(err)
+	}
 	ctx := context.Background()
 	sess, err := engine.NewSession(ctx, engine.SessionConfig{
 		Logger:          log.Log,
-		SoftwareName:    "ooniprobe-engine",
+		SoftwareName:    "miniooni",
-		SoftwareVersion: "0.0.1",
+		SoftwareVersion: "0.1.0-dev",
-		TunnelDir:       "testdata",
+		TunnelDir:       tunnelDir,
 	})
 	if err != nil {
 		t.Fatal(err)
@@ -54,7 +31,7 @@ func TestPsiphonStartStop(t *testing.T) {
 	tunnel, err := tunnel.Start(context.Background(), &tunnel.Config{
 		Name:      "psiphon",
 		Session:   sess,
-		TunnelDir: "testdata",
+		TunnelDir: tunnelDir,
 	})
 	if err != nil {
 		t.Fatal(err)

internal/engine/tunnel/psiphon_test.go

@@ -10,6 +10,37 @@ import (
 	"github.com/ooni/psiphon/oopsi/github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib"
 )
 
+func TestPsiphonWithCancelledContext(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // immediately fail
+	sess := &mockable.Session{}
+	tunnel, err := psiphonStart(ctx, &Config{
+		Session:   sess,
+		TunnelDir: "testdata",
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestPsiphonWithEmptyTunnelDir(t *testing.T) {
+	ctx := context.Background()
+	sess := &mockable.Session{}
+	tunnel, err := psiphonStart(ctx, &Config{
+		Session:   sess,
+		TunnelDir: "",
+	})
+	if !errors.Is(err, ErrEmptyTunnelDir) {
+		t.Fatal("not the error we expected")
+	}
+	if tunnel != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
 func TestPsiphonFetchPsiphonConfigFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	sess := &mockable.Session{

internal/engine/tunnel/tor.go

@@ -2,6 +2,7 @@ package tunnel
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net/url"
@@ -47,6 +48,16 @@ func (tt *torTunnel) Stop() {
 // TODO(bassosimone): the current design is such that we have a bunch of
 // torrc-$number and a growing tor.log file inside of stateDir.
 
+// ErrTorUnableToGetSOCKSProxyAddress indicates that we could not
+// get the SOCKS proxy address via the control port.
+var ErrTorUnableToGetSOCKSProxyAddress = errors.New(
+	"unable to get socks proxy address")
+
+// ErrTorReturnedUnsupportedProxy indicates that tor returned to
+// us the address of a proxy that we don't support.
+var ErrTorReturnedUnsupportedProxy = errors.New(
+	"tor returned unsupported proxy")
+
 // torStart starts the tor tunnel.
 func torStart(ctx context.Context, config *Config) (Tunnel, error) {
 	select {
@@ -88,12 +99,12 @@ func torStart(ctx context.Context, config *Config) (Tunnel, error) {
 	}
 	if len(info) != 1 || info[0].Key != "net/listeners/socks" {
 		instance.Close()
-		return nil, fmt.Errorf("unable to get socks proxy address")
+		return nil, ErrTorUnableToGetSOCKSProxyAddress
 	}
 	proxyAddress := info[0].Val
 	if strings.HasPrefix(proxyAddress, "unix:") {
 		instance.Close()
-		return nil, fmt.Errorf("tor returned unsupported proxy")
+		return nil, ErrTorReturnedUnsupportedProxy
 	}
 	return &torTunnel{
 		bootstrapTime: stop.Sub(start),

internal/engine/tunnel/tor_integration_test.go

@@ -0,0 +1,52 @@
+package tunnel_test
+
+import (
+	"context"
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/v3/internal/engine"
+	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
+)
+
+func TestTorStartStop(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skip test in short mode")
+	}
+	torBinaryPath := "/usr/bin/tor"
+	if m, err := os.Stat(torBinaryPath); err != nil || !m.Mode().IsRegular() {
+		t.Skip("missing precondition for the test")
+	}
+	tunnelDir, err := ioutil.TempDir("testdata", "tor")
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx := context.Background()
+	sess, err := engine.NewSession(ctx, engine.SessionConfig{
+		Logger:          log.Log,
+		SoftwareName:    "miniooni",
+		SoftwareVersion: "0.1.0-dev",
+		TunnelDir:       tunnelDir,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	tunnel, err := tunnel.Start(context.Background(), &tunnel.Config{
+		Name:      "tor",
+		Session:   sess,
+		TorBinary: torBinaryPath,
+		TunnelDir: tunnelDir,
+	})
+	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/tor_test.go

@@ -43,7 +43,7 @@ func TestTorTunnelNonNil(t *testing.T) {
 	}
 }
 
-func TestTorStartWithCancelledContext(t *testing.T) {
+func TestTorWithCancelledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // fail immediately
 	tun, err := torStart(ctx, &Config{
@@ -58,7 +58,21 @@ func TestTorStartWithCancelledContext(t *testing.T) {
 	}
 }
 
-func TestTorStartStartFailure(t *testing.T) {
+func TestTorWithEmptyTunnelDir(t *testing.T) {
+	ctx := context.Background()
+	tun, err := torStart(ctx, &Config{
+		Session:   &mockable.Session{},
+		TunnelDir: "",
+	})
+	if !errors.Is(err, ErrEmptyTunnelDir) {
+		t.Fatal("not the error we expected")
+	}
+	if tun != nil {
+		t.Fatal("expected nil tunnel here")
+	}
+}
+
+func TestTorStartFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
@@ -76,7 +90,7 @@ func TestTorStartStartFailure(t *testing.T) {
 	}
 }
 
-func TestTorStartEnableNetworkFailure(t *testing.T) {
+func TestTorEnableNetworkFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
@@ -97,7 +111,7 @@ func TestTorStartEnableNetworkFailure(t *testing.T) {
 	}
 }
 
-func TestTorStartGetInfoFailure(t *testing.T) {
+func TestTorGetInfoFailure(t *testing.T) {
 	expected := errors.New("mocked error")
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
@@ -121,7 +135,7 @@ func TestTorStartGetInfoFailure(t *testing.T) {
 	}
 }
 
-func TestTorStartGetInfoInvalidNumberOfKeys(t *testing.T) {
+func TestTorGetInfoInvalidNumberOfKeys(t *testing.T) {
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
 		Session:   &mockable.Session{},
@@ -136,15 +150,15 @@ func TestTorStartGetInfoInvalidNumberOfKeys(t *testing.T) {
 			return nil, nil
 		},
 	})
-	if err.Error() != "unable to get socks proxy address" {
+	if !errors.Is(err, ErrTorUnableToGetSOCKSProxyAddress) {
-		t.Fatal("not the error we expected")
+		t.Fatal("not the error we expected", err)
 	}
 	if tun != nil {
 		t.Fatal("expected nil tunnel here")
 	}
 }
 
-func TestTorStartGetInfoInvalidKey(t *testing.T) {
+func TestTorGetInfoInvalidKey(t *testing.T) {
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
 		Session:   &mockable.Session{},
@@ -159,7 +173,7 @@ func TestTorStartGetInfoInvalidKey(t *testing.T) {
 			return []*control.KeyVal{{}}, nil
 		},
 	})
-	if err.Error() != "unable to get socks proxy address" {
+	if !errors.Is(err, ErrTorUnableToGetSOCKSProxyAddress) {
 		t.Fatal("not the error we expected")
 	}
 	if tun != nil {
@@ -167,7 +181,7 @@ func TestTorStartGetInfoInvalidKey(t *testing.T) {
 	}
 }
 
-func TestTorStartGetInfoInvalidProxyType(t *testing.T) {
+func TestTorGetInfoInvalidProxyType(t *testing.T) {
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
 		Session:   &mockable.Session{},
@@ -190,7 +204,7 @@ func TestTorStartGetInfoInvalidProxyType(t *testing.T) {
 	}
 }
 
-func TestTorStartUnsupportedProxy(t *testing.T) {
+func TestTorUnsupportedProxy(t *testing.T) {
 	ctx := context.Background()
 	tun, err := torStart(ctx, &Config{
 		Session:   &mockable.Session{},
@@ -205,7 +219,7 @@ func TestTorStartUnsupportedProxy(t *testing.T) {
 			return []*control.KeyVal{{Key: "net/listeners/socks", Val: "unix:/foo/bar"}}, nil
 		},
 	})
-	if err.Error() != "tor returned unsupported proxy" {
+	if !errors.Is(err, ErrTorReturnedUnsupportedProxy) {
 		t.Fatal("not the error we expected")
 	}
 	if tun != nil {

internal/engine/tunnel/tunnel_test.go

@@ -1,4 +1,4 @@
-package tunnel
+package tunnel_test
 
 import (
 	"context"
@@ -7,20 +7,21 @@ import (
 
 	"github.com/apex/log"
 	"github.com/ooni/probe-cli/v3/internal/engine/internal/mockable"
+	"github.com/ooni/probe-cli/v3/internal/engine/tunnel"
 )
 
 func TestStartNoTunnel(t *testing.T) {
 	ctx := context.Background()
-	tunnel, err := Start(ctx, &Config{
+	tun, err := tunnel.Start(ctx, &tunnel.Config{
 		Name: "",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
 		},
 	})
-	if !errors.Is(err, ErrUnsupportedTunnelName) {
+	if !errors.Is(err, tunnel.ErrUnsupportedTunnelName) {
 		t.Fatal("not the error we expected", err)
 	}
-	if tunnel != nil {
+	if tun != nil {
 		t.Fatal("expected nil tunnel here")
 	}
 }
@@ -28,7 +29,7 @@ func TestStartNoTunnel(t *testing.T) {
 func TestStartPsiphonWithCancelledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // fail immediately
-	tunnel, err := Start(ctx, &Config{
+	tun, err := tunnel.Start(ctx, &tunnel.Config{
 		Name: "psiphon",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
@@ -38,7 +39,7 @@ func TestStartPsiphonWithCancelledContext(t *testing.T) {
 	if !errors.Is(err, context.Canceled) {
 		t.Fatal("not the error we expected")
 	}
-	if tunnel != nil {
+	if tun != nil {
 		t.Fatal("expected nil tunnel here")
 	}
 }
@@ -46,7 +47,7 @@ func TestStartPsiphonWithCancelledContext(t *testing.T) {
 func TestStartTorWithCancelledContext(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // fail immediately
-	tunnel, err := Start(ctx, &Config{
+	tun, err := tunnel.Start(ctx, &tunnel.Config{
 		Name: "tor",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
@@ -56,24 +57,24 @@ func TestStartTorWithCancelledContext(t *testing.T) {
 	if !errors.Is(err, context.Canceled) {
 		t.Fatal("not the error we expected")
 	}
-	if tunnel != nil {
+	if tun != nil {
 		t.Fatal("expected nil tunnel here")
 	}
 }
 
 func TestStartInvalidTunnel(t *testing.T) {
 	ctx := context.Background()
-	tunnel, err := Start(ctx, &Config{
+	tun, err := tunnel.Start(ctx, &tunnel.Config{
 		Name: "antani",
 		Session: &mockable.Session{
 			MockableLogger: log.Log,
 		},
 		TunnelDir: "testdata",
 	})
-	if !errors.Is(err, ErrUnsupportedTunnelName) {
+	if !errors.Is(err, tunnel.ErrUnsupportedTunnelName) {
 		t.Fatal("not the error we expected")
 	}
-	if tunnel != nil {
+	if tun != nil {
 		t.Fatal("expected nil tunnel here")
 	}
 }