From 7172e750ddc8747c89aab0b6bf44cc258ce38246 Mon Sep 17 00:00:00 2001
From: Simone Basso <bassosimone@gmail.com>
Date: Mon, 5 Apr 2021 19:51:41 +0200
Subject: [PATCH] fix(session): make sure tunnel code is tested (#301)

* fix(session): make sure tunnel code is tested

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

* fix: add missing TunnelDir for correctness
---
 internal/engine/session_integration_test.go |  3 --
 internal/engine/session_internal_test.go    | 44 +++++++++++++++++++++
 internal/engine/session_nopsiphon.go        |  6 ++-
 internal/engine/session_nopsiphon_test.go   | 20 ++++++++++
 4 files changed, 69 insertions(+), 4 deletions(-)
 create mode 100644 internal/engine/session_nopsiphon_test.go

diff --git a/internal/engine/session_integration_test.go b/internal/engine/session_integration_test.go
index f6a86c8..87674cd 100644
--- a/internal/engine/session_integration_test.go
+++ b/internal/engine/session_integration_test.go
@@ -421,9 +421,6 @@ func TestAllProbeServicesUnsupported(t *testing.T) {
 	}
 }
 
-// TODO(bassosimone): we should write unit/integration tests
-// for the new way in which tunnels work.
-
 func TestUserAgentNoProxy(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skip test in short mode")
diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go
index e254e73..a80f3dc 100644
--- a/internal/engine/session_internal_test.go
+++ b/internal/engine/session_internal_test.go
@@ -3,9 +3,11 @@ package engine
 import (
 	"context"
 	"errors"
+	"net/url"
 	"sync"
 	"testing"
 
+	"github.com/apex/log"
 	"github.com/google/go-cmp/cmp"
 	"github.com/ooni/probe-cli/v3/internal/engine/geolocate"
 	"github.com/ooni/probe-cli/v3/internal/engine/model"
@@ -247,3 +249,45 @@ func TestSessionFetchPsiphonConfigWithCancelledContext(t *testing.T) {
 		t.Fatal("expected nil response here")
 	}
 }
+
+func TestNewSessionWithFakeTunnel(t *testing.T) {
+	ctx := context.Background()
+	sess, err := NewSession(ctx, SessionConfig{
+		Logger:          log.Log,
+		ProxyURL:        &url.URL{Scheme: "fake"},
+		SoftwareName:    "miniooni",
+		SoftwareVersion: "0.1.0-dev",
+		TunnelDir:       "testdata",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if sess == nil {
+		t.Fatal("expected non-nil session here")
+	}
+	if sess.ProxyURL() == nil {
+		t.Fatal("expected non-nil proxyURL here")
+	}
+	if sess.tunnel == nil {
+		t.Fatal("expected non-nil tunnel here")
+	}
+	sess.Close() // ensure we don't crash
+}
+
+func TestNewSessionWithFakeTunnelAndCancelledContext(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // fail immediately
+	sess, err := NewSession(ctx, SessionConfig{
+		Logger:          log.Log,
+		ProxyURL:        &url.URL{Scheme: "fake"},
+		SoftwareName:    "miniooni",
+		SoftwareVersion: "0.1.0-dev",
+		TunnelDir:       "testdata",
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatal("not the error we expected", err)
+	}
+	if sess != nil {
+		t.Fatal("expected nil session here")
+	}
+}
diff --git a/internal/engine/session_nopsiphon.go b/internal/engine/session_nopsiphon.go
index ac43b55..fe09e30 100644
--- a/internal/engine/session_nopsiphon.go
+++ b/internal/engine/session_nopsiphon.go
@@ -20,7 +20,11 @@ func (s *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
 // to tunnel.Start to fetch the Psiphon configuration.
 type sessionTunnelEarlySession struct{}
 
+// errPsiphonNoEmbeddedConfig indicates that there is no
+// embedded psiphong config file in this binary.
+var errPsiphonNoEmbeddedConfig = errors.New("no embedded configuration file")
+
 // FetchPsiphonConfig implements tunnel.Session.FetchPsiphonConfig.
 func (s *sessionTunnelEarlySession) FetchPsiphonConfig(ctx context.Context) ([]byte, error) {
-	return nil, errors.New("no embedded configuration file")
+	return nil, errPsiphonNoEmbeddedConfig
 }
diff --git a/internal/engine/session_nopsiphon_test.go b/internal/engine/session_nopsiphon_test.go
new file mode 100644
index 0000000..e5b3d0d
--- /dev/null
+++ b/internal/engine/session_nopsiphon_test.go
@@ -0,0 +1,20 @@
+// +build !ooni_psiphon_config
+
+package engine
+
+import (
+	"context"
+	"errors"
+	"testing"
+)
+
+func TestEarlySessionNoPsiphonFetchPsiphonConfig(t *testing.T) {
+	s := &sessionTunnelEarlySession{}
+	out, err := s.FetchPsiphonConfig(context.Background())
+	if !errors.Is(err, errPsiphonNoEmbeddedConfig) {
+		t.Fatal("not the error we expected", err)
+	}
+	if out != nil {
+		t.Fatal("expected nil here")
+	}
+}