More cleanup and refactoring

chat.go

@@ -15,28 +15,47 @@ type ChatAPI struct {
 	fromStdout chan []byte
 	fromListen chan []byte
 	errors     chan error
+	kbLoc      string
 }
 
-func NewChatAPI(ctx context.Context, opts *Options) (api *ChatAPI, err error) {
+func NewChatAPI(ctx context.Context, opts Options) (api *ChatAPI, err error) {
+	if opts.ChannelBufferSize <= 0 {
+		opts.ChannelBufferSize = 10
+	}
+
 	// set up the channels
-	api.toStdin = make(chan []byte, opts.ChannelBufferSize)
-	api.fromStdout = make(chan []byte, opts.ChannelBufferSize)
-	api.fromListen = make(chan []byte, opts.ChannelBufferSize)
-	api.errors = make(chan error, opts.ChannelBufferSize)
+	api = &ChatAPI{
+		toStdin:    make(chan []byte, opts.ChannelBufferSize),
+		fromStdout: make(chan []byte, opts.ChannelBufferSize),
+		fromListen: make(chan []byte, opts.ChannelBufferSize),
+		errors:     make(chan error, opts.ChannelBufferSize),
+	}
 
 	// get the basics
-	kbCmd := opts.locateKeybase()
-	chatApiArgs := opts.buildArgs("chat", "api")
-	chatApiListenArgs := opts.buildArgs("chat", "api-listen")
+	if opts.KeybaseLocation == "" {
+		opts.KeybaseLocation, err = locateKeybase()
+		if err != nil {
+			return nil, fmt.Errorf("failed to locate keybase: %v", err)
+		}
+	}
+	api.kbLoc = opts.KeybaseLocation
+	chatApiArgs, err := buildArgs(opts, "chat", "api")
+	if err != nil {
+		return nil, fmt.Errorf("failed to build api args: %v", err)
+	}
+	chatApiListenArgs, err := buildArgs(opts, "chat", "api-listen")
+	if err != nil {
+		return nil, fmt.Errorf("failed to build api-listen args: %v", err)
+	}
 
 	// build the commands
-	chatApi, err := newApiCmd(ctx, kbCmd, chatApiArgs...)
+	chatApi, err := newApiCmd(ctx, api.kbLoc, chatApiArgs...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to build api command")
 	}
-	chatListen, err := newApiCmd(ctx, kbCmd, chatApiListenArgs...)
+	chatListen, err := newApiCmd(ctx, api.kbLoc, chatApiListenArgs...)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to build api-listen command")
 	}
 
 	// create the goroutines
@@ -95,10 +114,6 @@ func NewChatAPI(ctx context.Context, opts *Options) (api *ChatAPI, err error) {
 	return
 }
 
-func (c *ChatAPI) Listen() chan []byte {
-	return c.fromListen
-}
-
 func (c *ChatAPI) SendRaw(msg []byte) ([]byte, error) {
 	c.Lock()
 	defer c.Unlock()

keybase.go

@@ -1,52 +1,55 @@
 package keybase
 
 import (
-	"log"
+	"fmt"
 	"os/exec"
 )
 
 // Options holds... run... options...
 type Options struct {
-	KeybaseLoction    string // Optional, but required if keybase is not in your path
-	HomeDir           string // Only use this if you know what you're doing
-	EnableTyping      bool   // Show others a typing notification while the bot is working on a command
-	BotLiteMode       bool   // Defaults to true - only disable if you need to.
-	ChannelBufferSize int    // The size of the channel buffers, may vary on rate of message ingestion
-}
+	// Optional, but required if keybase is not in your path
+	KeybaseLocation string
 
-// NewOptions returns a new instance of *Options with sensible defaults
-func NewOptions() *Options {
-	return &Options{
-		BotLiteMode:       true,
-		ChannelBufferSize: 10,
-	}
+	// Only use this if you know what you're doing
+	HomeDir string
+
+	// Show others a typing notification while the bot is working on a command
+	EnableTyping bool
+
+	// Turn off Keybase's 'bot lite' mode. Only disable if you need to.
+	DisableLiteMode bool
+
+	// The size of the channel buffers, may vary on rate of message ingestion
+	ChannelBufferSize int
 }
 
 // locateKeybase attempts to find the location of the keybase binary in the following order:
 // 1. What the user has specified as the location [user specified]
 // 2. Looks up the binary location using exec.LookPath [default]
 // 3. Returns "keybase" and hopes its pathed on the users system [fallback]
-func (opt *Options) locateKeybase() string {
-	if opt.KeybaseLoction != "" {
-		return opt.KeybaseLoction
-	}
+func locateKeybase() (string, error) {
 	path, err := exec.LookPath("keybase")
 	if err != nil {
-		log.Println("INFO: Could not detect keybase in path")
-		return "keybase"
+		return "", fmt.Errorf("could not determine path for keybase binary: %v", err)
 	}
-	return path
+	return path, nil
 }
 
 // buildBaseCommand adds the homedirectory before the args, when required
-func (opt *Options) buildArgs(args ...string) []string {
-	var cmd []string
-	if opt.HomeDir != "" {
-		cmd = append(cmd, "--home", opt.HomeDir)
+func buildArgs(opts Options, args ...string) ([]string, error) {
+	if len(args) == 0 {
+		return nil, fmt.Errorf("no arguments")
+	}
+
+	// initialize slice so we never return a nil object
+	cmd := make([]string, 0)
+	if opts.HomeDir != "" {
+		cmd = append(cmd, "--home", opts.HomeDir)
 	}
-	if opt.BotLiteMode {
+	if !opts.DisableLiteMode {
 		cmd = append(cmd, "--enable-bot-lite-mode")
 	}
 	cmd = append(cmd, args...)
-	return cmd
+
+	return cmd, nil
 }

keybase_test.go

@@ -2,41 +2,39 @@ package keybase
 
 import (
 	"reflect"
-	"strings"
 	"testing"
 )
 
-func TestNewOptions(t *testing.T) {
-	want := &Options{
-		KeybaseLoction: "",
-		HomeDir:        "",
-		EnableTyping:   false,
-		BotLiteMode:    true,
+func TestBuildArgs(t *testing.T) {
+	cases := []struct {
+		Opts     Options
+		Args     []string
+		Expected []string
+	}{
+		{
+			Opts:     Options{HomeDir: "/home/foo"},
+			Args:     []string{"arg1"},
+			Expected: []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg1"},
+		},
+		{
+			Opts:     Options{HomeDir: "/home/foo"},
+			Args:     []string{"arg1", "arg2"},
+			Expected: []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg1", "arg2"},
+		},
+		{
+			Opts:     Options{HomeDir: "/home/foo", DisableLiteMode: true},
+			Args:     []string{"arg1"},
+			Expected: []string{"--home", "/home/foo", "arg1"},
+		},
 	}
-	got := NewOptions()
-	if !reflect.DeepEqual(want, got) {
-		t.Errorf("NewOptions returned non-equal structs")
-	}
-}
-
-func TestLocateKeybase(t *testing.T) {
-	opts := NewOptions()
-	opts.KeybaseLoction = "/usr/bin/keybase"
-	if opts.locateKeybase() != "/usr/bin/keybase" {
-		t.Errorf("locateKeybase returned non-specified keybase")
-	}
-	opts2 := NewOptions()
-	if !strings.Contains(opts2.locateKeybase(), "keybase") {
-		t.Errorf("locateKeybase did not return a keybase path")
-	}
-}
 
-func TestBaseCommand(t *testing.T) {
-	opts := NewOptions()
-	opts.HomeDir = "/home/foo"
-	want := []string{"--home", "/home/foo", "--enable-bot-lite-mode", "arg"}
-	got := opts.buildArgs("arg")
-	if !reflect.DeepEqual(want, got) {
-		t.Errorf("buildBaseCommand returned invalid command set")
+	for i, c := range cases {
+		actual, err := buildArgs(c.Opts, c.Args...)
+		if err != nil {
+			t.Errorf("[case %d] returned error: %v", i, err)
+		}
+		if !reflect.DeepEqual(actual, c.Expected) {
+			t.Errorf("[case %d] expected: %#v, got: %#v", i, c.Expected, actual)
+		}
 	}
 }