feat(geoip, info): write comprehensive unit tests

internal/cli/geoip/geoip.go

@@ -4,38 +4,54 @@ import (
 	"github.com/alecthomas/kingpin"
 	"github.com/apex/log"
 	"github.com/ooni/probe-cli/internal/cli/root"
+	"github.com/ooni/probe-cli/internal/ooni"
 	"github.com/ooni/probe-cli/internal/output"
 )
 
 func init() {
 	cmd := root.Command("geoip", "Perform a geoip lookup")
-
 	cmd.Action(func(_ *kingpin.ParseContext) error {
-		output.SectionTitle("GeoIP lookup")
-		probeCLI, err := root.Init()
-		if err != nil {
-			return err
-		}
-
-		engine, err := probeCLI.NewProbeEngine()
-		if err != nil {
-			return err
-		}
-		defer engine.Close()
-
-		err = engine.MaybeLookupLocation()
-		if err != nil {
-			return err
-		}
-
-		log.WithFields(log.Fields{
-			"type":         "table",
-			"asn":          engine.ProbeASNString(),
-			"network_name": engine.ProbeNetworkName(),
-			"country_code": engine.ProbeCC(),
-			"ip":           engine.ProbeIP(),
-		}).Info("Looked up your location")
-
-		return nil
+		return dogeoip(defaultconfig)
 	})
 }
+
+type dogeoipconfig struct {
+	Logger       log.Interface
+	NewProbeCLI  func() (ooni.ProbeCLI, error)
+	SectionTitle func(string)
+}
+
+var defaultconfig = dogeoipconfig{
+	Logger:       log.Log,
+	NewProbeCLI:  root.NewProbeCLI,
+	SectionTitle: output.SectionTitle,
+}
+
+func dogeoip(config dogeoipconfig) error {
+	config.SectionTitle("GeoIP lookup")
+	probeCLI, err := config.NewProbeCLI()
+	if err != nil {
+		return err
+	}
+
+	engine, err := probeCLI.NewProbeEngine()
+	if err != nil {
+		return err
+	}
+	defer engine.Close()
+
+	err = engine.MaybeLookupLocation()
+	if err != nil {
+		return err
+	}
+
+	config.Logger.WithFields(log.Fields{
+		"type":         "table",
+		"asn":          engine.ProbeASNString(),
+		"network_name": engine.ProbeNetworkName(),
+		"country_code": engine.ProbeCC(),
+		"ip":           engine.ProbeIP(),
+	}).Info("Looked up your location")
+
+	return nil
+}

internal/cli/geoip/geoip_test.go

@@ -0,0 +1,134 @@
+package geoip
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/internal/ooni"
+	"github.com/ooni/probe-cli/internal/oonitest"
+)
+
+func TestNewProbeCLIFailed(t *testing.T) {
+	fo := &oonitest.FakeOutput{}
+	expected := errors.New("mocked error")
+	err := dogeoip(dogeoipconfig{
+		SectionTitle: fo.SectionTitle,
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return nil, expected
+		},
+	})
+	if !errors.Is(err, expected) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if len(fo.FakeSectionTitle) != 1 {
+		t.Fatal("invalid section title list size")
+	}
+	if fo.FakeSectionTitle[0] != "GeoIP lookup" {
+		t.Fatal("unexpected string")
+	}
+}
+
+func TestNewProbeEngineFailed(t *testing.T) {
+	fo := &oonitest.FakeOutput{}
+	expected := errors.New("mocked error")
+	cli := &oonitest.FakeProbeCLI{
+		FakeProbeEngineErr: expected,
+	}
+	err := dogeoip(dogeoipconfig{
+		SectionTitle: fo.SectionTitle,
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return cli, nil
+		},
+	})
+	if !errors.Is(err, expected) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if len(fo.FakeSectionTitle) != 1 {
+		t.Fatal("invalid section title list size")
+	}
+	if fo.FakeSectionTitle[0] != "GeoIP lookup" {
+		t.Fatal("unexpected string")
+	}
+}
+
+func TestMaybeLookupLocationFailed(t *testing.T) {
+	fo := &oonitest.FakeOutput{}
+	expected := errors.New("mocked error")
+	engine := &oonitest.FakeProbeEngine{
+		FakeMaybeLookupLocation: expected,
+	}
+	cli := &oonitest.FakeProbeCLI{
+		FakeProbeEnginePtr: engine,
+	}
+	err := dogeoip(dogeoipconfig{
+		SectionTitle: fo.SectionTitle,
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return cli, nil
+		},
+	})
+	if !errors.Is(err, expected) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if len(fo.FakeSectionTitle) != 1 {
+		t.Fatal("invalid section title list size")
+	}
+	if fo.FakeSectionTitle[0] != "GeoIP lookup" {
+		t.Fatal("unexpected string")
+	}
+}
+
+func TestMaybeLookupLocationSuccess(t *testing.T) {
+	fo := &oonitest.FakeOutput{}
+	engine := &oonitest.FakeProbeEngine{
+		FakeProbeASNString:   "AS30722",
+		FakeProbeCC:          "IT",
+		FakeProbeNetworkName: "Vodafone Italia S.p.A.",
+		FakeProbeIP:          "130.25.90.216",
+	}
+	cli := &oonitest.FakeProbeCLI{
+		FakeProbeEnginePtr: engine,
+	}
+	handler := &oonitest.FakeLoggerHandler{}
+	err := dogeoip(dogeoipconfig{
+		SectionTitle: fo.SectionTitle,
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return cli, nil
+		},
+		Logger: &log.Logger{
+			Handler: handler,
+			Level:   log.DebugLevel,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(fo.FakeSectionTitle) != 1 {
+		t.Fatal("invalid section title list size")
+	}
+	if fo.FakeSectionTitle[0] != "GeoIP lookup" {
+		t.Fatal("unexpected string")
+	}
+	if len(handler.FakeEntries) != 1 {
+		t.Fatal("invalid number of written entries")
+	}
+	entry := handler.FakeEntries[0]
+	if entry.Level != log.InfoLevel {
+		t.Fatal("invalid log level")
+	}
+	if entry.Message != "Looked up your location" {
+		t.Fatal("invalid .Message")
+	}
+	if entry.Fields["asn"].(string) != "AS30722" {
+		t.Fatal("invalid asn")
+	}
+	if entry.Fields["country_code"].(string) != "IT" {
+		t.Fatal("invalid asn")
+	}
+	if entry.Fields["network_name"].(string) != "Vodafone Italia S.p.A." {
+		t.Fatal("invalid asn")
+	}
+	if entry.Fields["ip"].(string) != "130.25.90.216" {
+		t.Fatal("invalid asn")
+	}
+}

internal/cli/info/info.go

@@ -4,24 +4,33 @@ import (
 	"github.com/alecthomas/kingpin"
 	"github.com/apex/log"
 	"github.com/ooni/probe-cli/internal/cli/root"
+	"github.com/ooni/probe-cli/internal/ooni"
 )
 
 func init() {
 	cmd := root.Command("info", "Display information about OONI Probe")
-
 	cmd.Action(func(_ *kingpin.ParseContext) error {
-		probeCLI, err := root.Init()
-		if err != nil {
-			log.Errorf("%s", err)
-			return err
-		}
-		log.WithFields(log.Fields{
-			"path": probeCLI.Home(),
-		}).Info("Home")
-		log.WithFields(log.Fields{
-			"path": probeCLI.TempDir(),
-		}).Info("TempDir")
-
-		return nil
+		return doinfo(defaultconfig)
 	})
 }
+
+type doinfoconfig struct {
+	Logger      log.Interface
+	NewProbeCLI func() (ooni.ProbeCLI, error)
+}
+
+var defaultconfig = doinfoconfig{
+	Logger:      log.Log,
+	NewProbeCLI: root.NewProbeCLI,
+}
+
+func doinfo(config doinfoconfig) error {
+	probeCLI, err := config.NewProbeCLI()
+	if err != nil {
+		config.Logger.Errorf("%s", err)
+		return err
+	}
+	config.Logger.WithFields(log.Fields{"path": probeCLI.Home()}).Info("Home")
+	config.Logger.WithFields(log.Fields{"path": probeCLI.TempDir()}).Info("TempDir")
+	return nil
+}

internal/cli/info/info_test.go

@@ -0,0 +1,80 @@
+package info
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/internal/ooni"
+	"github.com/ooni/probe-cli/internal/oonitest"
+)
+
+func TestNewProbeCLIFailed(t *testing.T) {
+	expected := errors.New("mocked error")
+	handler := &oonitest.FakeLoggerHandler{}
+	err := doinfo(doinfoconfig{
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return nil, expected
+		},
+		Logger: &log.Logger{
+			Handler: handler,
+			Level:   log.DebugLevel,
+		},
+	})
+	if !errors.Is(err, expected) {
+		t.Fatalf("not the error we expected: %+v", err)
+	}
+	if len(handler.FakeEntries) != 1 {
+		t.Fatal("invalid number of log entries")
+	}
+	entry := handler.FakeEntries[0]
+	if entry.Level != log.ErrorLevel {
+		t.Fatal("invalid log level")
+	}
+	if entry.Message != "mocked error" {
+		t.Fatal("invalid .Message")
+	}
+}
+
+func TestSuccess(t *testing.T) {
+	handler := &oonitest.FakeLoggerHandler{}
+	cli := &oonitest.FakeProbeCLI{
+		FakeHome:    "fakehome",
+		FakeTempDir: "faketempdir",
+	}
+	err := doinfo(doinfoconfig{
+		NewProbeCLI: func() (ooni.ProbeCLI, error) {
+			return cli, nil
+		},
+		Logger: &log.Logger{
+			Handler: handler,
+			Level:   log.DebugLevel,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(handler.FakeEntries) != 2 {
+		t.Fatal("invalid number of log entries")
+	}
+	entry := handler.FakeEntries[0]
+	if entry.Level != log.InfoLevel {
+		t.Fatal("invalid log level")
+	}
+	if entry.Message != "Home" {
+		t.Fatal("invalid .Message")
+	}
+	if entry.Fields["path"].(string) != "fakehome" {
+		t.Fatal("invalid path")
+	}
+	entry = handler.FakeEntries[1]
+	if entry.Level != log.InfoLevel {
+		t.Fatal("invalid log level")
+	}
+	if entry.Message != "TempDir" {
+		t.Fatal("invalid .Message")
+	}
+	if entry.Fields["path"].(string) != "faketempdir" {
+		t.Fatal("invalid path")
+	}
+}

internal/cli/root/root.go

@@ -19,6 +19,15 @@ var Command = Cmd.Command
 // Init should be called by all subcommand that care to have a ooni.Context instance
 var Init func() (*ooni.Probe, error)
 
+// NewProbeCLI is like Init but returns a ooni.ProbeCLI instead.
+func NewProbeCLI() (ooni.ProbeCLI, error) {
+	probeCLI, err := Init()
+	if err != nil {
+		return nil, err
+	}
+	return probeCLI, nil
+}
+
 func init() {
 	configPath := Cmd.Flag("config", "Set a custom config file path").Short('c').String()
 

internal/oonitest/oonitest.go

@@ -0,0 +1,126 @@
+// Package oonitest contains code used for testing.
+package oonitest
+
+import (
+	"sync"
+
+	"github.com/apex/log"
+	"github.com/ooni/probe-cli/internal/config"
+	"github.com/ooni/probe-cli/internal/ooni"
+	"upper.io/db.v3/lib/sqlbuilder"
+)
+
+// FakeOutput allows to fake the output package.
+type FakeOutput struct {
+	FakeSectionTitle []string
+	mu               sync.Mutex
+}
+
+// SectionTitle writes the section title.
+func (fo *FakeOutput) SectionTitle(s string) {
+	fo.mu.Lock()
+	defer fo.mu.Unlock()
+	fo.FakeSectionTitle = append(fo.FakeSectionTitle, s)
+}
+
+// FakeProbeCLI fakes ooni.ProbeCLI
+type FakeProbeCLI struct {
+	FakeConfig         *config.Config
+	FakeDB             sqlbuilder.Database
+	FakeIsBatch        bool
+	FakeHome           string
+	FakeTempDir        string
+	FakeProbeEnginePtr ooni.ProbeEngine
+	FakeProbeEngineErr error
+}
+
+// Config implements ProbeCLI.Config
+func (cli *FakeProbeCLI) Config() *config.Config {
+	return cli.FakeConfig
+}
+
+// DB implements ProbeCLI.DB
+func (cli *FakeProbeCLI) DB() sqlbuilder.Database {
+	return cli.FakeDB
+}
+
+// IsBatch implements ProbeCLI.IsBatch
+func (cli *FakeProbeCLI) IsBatch() bool {
+	return cli.FakeIsBatch
+}
+
+// Home implements ProbeCLI.Home
+func (cli *FakeProbeCLI) Home() string {
+	return cli.FakeHome
+}
+
+// TempDir implements ProbeCLI.TempDir
+func (cli *FakeProbeCLI) TempDir() string {
+	return cli.FakeTempDir
+}
+
+// NewProbeEngine implements ProbeCLI.NewProbeEngine
+func (cli *FakeProbeCLI) NewProbeEngine() (ooni.ProbeEngine, error) {
+	return cli.FakeProbeEnginePtr, cli.FakeProbeEngineErr
+}
+
+var _ ooni.ProbeCLI = &FakeProbeCLI{}
+
+// FakeProbeEngine fakes ooni.ProbeEngine
+type FakeProbeEngine struct {
+	FakeClose               error
+	FakeMaybeLookupLocation error
+	FakeProbeASNString      string
+	FakeProbeCC             string
+	FakeProbeIP             string
+	FakeProbeNetworkName    string
+}
+
+// Close implements ProbeEngine.Close
+func (eng *FakeProbeEngine) Close() error {
+	return eng.FakeClose
+}
+
+// MaybeLookupLocation implements ProbeEngine.MaybeLookupLocation
+func (eng *FakeProbeEngine) MaybeLookupLocation() error {
+	return eng.FakeMaybeLookupLocation
+}
+
+// ProbeASNString implements ProbeEngine.ProbeASNString
+func (eng *FakeProbeEngine) ProbeASNString() string {
+	return eng.FakeProbeASNString
+}
+
+// ProbeCC implements ProbeEngine.ProbeCC
+func (eng *FakeProbeEngine) ProbeCC() string {
+	return eng.FakeProbeCC
+}
+
+// ProbeIP implements ProbeEngine.ProbeIP
+func (eng *FakeProbeEngine) ProbeIP() string {
+	return eng.FakeProbeIP
+}
+
+// ProbeNetworkName implements ProbeEngine.ProbeNetworkName
+func (eng *FakeProbeEngine) ProbeNetworkName() string {
+	return eng.FakeProbeNetworkName
+}
+
+var _ ooni.ProbeEngine = &FakeProbeEngine{}
+
+// FakeLoggerHandler fakes apex.log.Handler.
+type FakeLoggerHandler struct {
+	FakeEntries []*log.Entry
+	FakeErr     error
+	mu          sync.Mutex
+}
+
+// HandleLog implements Handler.HandleLog.
+func (handler *FakeLoggerHandler) HandleLog(entry *log.Entry) error {
+	handler.mu.Lock()
+	defer handler.mu.Unlock()
+	handler.FakeEntries = append(handler.FakeEntries, entry)
+	return handler.FakeErr
+}
+
+var _ log.Handler = &FakeLoggerHandler{}