tests(pkg): beef up unit tests

Author: Matthew Fisher

boot.go

@@ -20,6 +20,7 @@ import (
 	_ "github.com/docker/distribution/registry/storage/driver/gcs"
 	_ "github.com/docker/distribution/registry/storage/driver/s3-aws"
 	_ "github.com/docker/distribution/registry/storage/driver/swift"
+	"github.com/kelseyhightower/envconfig"
 	kcl "k8s.io/kubernetes/pkg/client/unversioned"
 )
 
@@ -48,7 +49,7 @@ func main() {
 			Usage:   "Run the git server",
 			Action: func(c *cli.Context) {
 				cnf := new(sshd.Config)
-				if err := conf.EnvConfig(serverConfAppName, cnf); err != nil {
+				if err := envconfig.Process(serverConfAppName, cnf); err != nil {
 					pkglog.Err("getting config for %s [%s]", serverConfAppName, err)
 					os.Exit(1)
 				}
@@ -118,7 +119,7 @@ func main() {
 			Usage:   "Run the git-receive hook",
 			Action: func(c *cli.Context) {
 				cnf := new(gitreceive.Config)
-				if err := conf.EnvConfig(gitReceiveConfAppName, cnf); err != nil {
+				if err := envconfig.Process(gitReceiveConfAppName, cnf); err != nil {
 					log.Printf("Error getting config for %s [%s]", gitReceiveConfAppName, err)
 					os.Exit(1)
 				}

glide.lock

@@ -16,7 +16,7 @@ import:
   version: eca94c41d994ae2215d455ce578ae6e2dc6ee516
 - package: github.com/pborman/uuid
 - package: github.com/deis/pkg
-  version: 189ed6bd6b6aa6629b72c2c5472095e176eec8a6
+  version: 00e55bded444eea7fadff398f93152e962a0c338
   subpackages:
   - time
   - log

glide.yaml

@@ -6,40 +6,25 @@ import (
 	"strings"
 
 	"github.com/deis/builder/pkg/sys"
-	"github.com/kelseyhightower/envconfig"
 )
 
 const (
-	BuilderKeyLocation  = "/var/run/secrets/api/auth/builder-key"
 	storageCredLocation = "/var/run/secrets/deis/objectstore/creds/"
 	minioHostEnvVar     = "DEIS_MINIO_SERVICE_HOST"
 	minioPortEnvVar     = "DEIS_MINIO_SERVICE_PORT"
 	gcsKey              = "key.json"
 )
 
+var BuilderKeyLocation string = "/var/run/secrets/api/auth/builder-key"
+
 // Parameters is map which contains storage params
 type Parameters map[string]interface{}
 
-// EnvConfig is a convenience function to process the envconfig (
-// https://github.com/kelseyhightower/envconfig) based configuration environment variables into
-// conf. Additional notes:
-//
-// - appName will be passed as the first parameter to envconfig.Process
-// - conf should be a pointer to an envconfig compatible struct. If you'd like to use struct
-// 	 	tags to customize your struct, see
-// 		https://github.com/kelseyhightower/envconfig#struct-tag-support
-func EnvConfig(appName string, conf interface{}) error {
-	if err := envconfig.Process(appName, conf); err != nil {
-		return err
-	}
-	return nil
-}
-
 // GetBuilderKey returns the key to be used as token to interact with deis-controller
-func GetBuilderKey(builderKeyPath string) (string, error) {
-	builderKeyBytes, err := ioutil.ReadFile(builderKeyPath)
+func GetBuilderKey() (string, error) {
+	builderKeyBytes, err := ioutil.ReadFile(BuilderKeyLocation)
 	if err != nil {
-		return "", fmt.Errorf("couldn't get builder key from %s (%s)", builderKeyPath, err)
+		return "", fmt.Errorf("couldn't get builder key from %s (%s)", BuilderKeyLocation, err)
 	}
 	builderKey := strings.Trim(string(builderKeyBytes), "\n")
 	return builderKey, nil

pkg/conf/config.go

@@ -4,6 +4,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/user"
+	"path/filepath"
 	"testing"
 
 	"github.com/arschles/assert"
@@ -64,28 +65,47 @@ func TestGetStorageParams(t *testing.T) {
 	if err != nil {
 		t.Errorf("received error while retrieving storage params: %v", err)
 	}
+
+	env := sys.NewFakeEnv()
+	env.Envs = map[string]string{
+		"BUILDER_STORAGE":         "minio",
+		"DEIS_MINIO_SERVICE_HOST": "localhost",
+		"DEIS_MINIO_SERVICE_PORT": "8088",
+	}
+	params, err = GetStorageParams(env)
+	if err != nil {
+		t.Errorf("received error while retrieving storage params: %v", err)
+	}
+	assert.Equal(t, params["regionendpoint"], "http://localhost:8088", "region endpoint")
+	assert.Equal(t, params["secure"], false, "secure")
+	assert.Equal(t, params["region"], "us-east-1", "region")
+	assert.Equal(t, params["bucket"], "git", "bucket")
 }
 
 func TestGetControllerClient(t *testing.T) {
 	tmpDir, err := ioutil.TempDir("", "tmpdir")
 	if err != nil {
 		t.Fatalf("error creating temp directory (%s)", err)
 	}
-	data := []byte("testbuilderkey")
-	if err := ioutil.WriteFile(tmpDir+"/builder-key", data, 0644); err != nil {
-		t.Fatalf("error creating %s/builder-key (%s)", tmpDir, err)
-	}
+
 	defer func() {
 		if err := os.RemoveAll(tmpDir); err != nil {
 			t.Fatalf("failed to remove builder-key from %s (%s)", tmpDir, err)
 		}
 	}()
-	key, err := GetBuilderKey(tmpDir + "/builder-key")
+
+	BuilderKeyLocation = filepath.Join(tmpDir, "builder-key")
+	data := []byte("testbuilderkey")
+	if err := ioutil.WriteFile(BuilderKeyLocation, data, 0644); err != nil {
+		t.Fatalf("error creating %s (%s)", BuilderKeyLocation, err)
+	}
+
+	key, err := GetBuilderKey()
 	assert.NoErr(t, err)
 	assert.Equal(t, key, string(data), "data")
 }
 
 func TestGetBuilderKeyError(t *testing.T) {
-	_, err := GetBuilderKey("/builder-key")
+	_, err := GetBuilderKey()
 	assert.True(t, err != nil, "no error received when there should have been")
 }

pkg/conf/config_test.go

@@ -9,15 +9,15 @@ import (
 )
 
 // New creates a new SDK client configured as the builder.
-func New(host, port, builderKeyPath string) (*deis.Client, error) {
+func New(host, port string) (*deis.Client, error) {
 
 	client, err := deis.New(true, fmt.Sprintf("http://%s:%s/", host, port), "")
 	if err != nil {
 		return client, err
 	}
 	client.UserAgent = "deis-builder"
 
-	builderKey, err := conf.GetBuilderKey(builderKeyPath)
+	builderKey, err := conf.GetBuilderKey()
 	if err != nil {
 		return client, err
 	}

pkg/controller/utils.go

@@ -1,39 +1,68 @@
 package controller
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
+	builderconf "github.com/deis/builder/pkg/conf"
+
 	"github.com/arschles/assert"
+	deis "github.com/deis/controller-sdk-go"
 )
 
-func TestGetControllerClient(t *testing.T) {
+func TestNew(t *testing.T) {
 	tmpDir, err := ioutil.TempDir("", "tmpdir")
 	if err != nil {
 		t.Fatalf("error creating temp directory (%s)", err)
 	}
-	data := []byte("testbuilderkey")
-	if err := ioutil.WriteFile(tmpDir+"/builder-key", data, 0644); err != nil {
-		t.Fatalf("error creating %s/builder-key (%s)", tmpDir, err)
-	}
+
 	defer func() {
 		if err := os.RemoveAll(tmpDir); err != nil {
 			t.Fatalf("failed to remove builder-key from %s (%s)", tmpDir, err)
 		}
 	}()
+
+	builderconf.BuilderKeyLocation = filepath.Join(tmpDir, "builder-key")
+	data := []byte("testbuilderkey")
+	if err := ioutil.WriteFile(builderconf.BuilderKeyLocation, data, 0644); err != nil {
+		t.Fatalf("error creating %s (%s)", builderconf.BuilderKeyLocation, err)
+	}
+
 	host := "127.0.0.1"
 	port := "80"
-	cli, err := New(host, port, tmpDir+"/builder-key")
+	cli, err := New(host, port)
 	assert.NoErr(t, err)
 	assert.Equal(t, cli.ControllerURL.String(), fmt.Sprintf("http://%s:%s/", host, port), "data")
 	assert.Equal(t, cli.HooksToken, string(data), "data")
+	assert.Equal(t, cli.UserAgent, "deis-builder", "user-agent")
+
+	port = "invalid-port-number"
+	if _, err = New(host, port); err == nil {
+		t.Errorf("expected error with invalid port number, got nil")
+	}
 }
 
-func TestGetControllerClientError(t *testing.T) {
+func TestNewWithInvalidBuilderKeyPath(t *testing.T) {
 	host := "127.0.0.1"
 	port := "80"
-	_, err := New(host, port, "/builder-key")
+	_, err := New(host, port)
 	assert.True(t, err != nil, "no error received when there should have been")
 }
+
+func TestCheckAPICompat(t *testing.T) {
+	client := &deis.Client{ControllerAPIVersion: deis.APIVersion}
+	err := deis.ErrAPIMismatch
+
+	if apiErr := CheckAPICompat(client, err); apiErr != nil {
+		t.Errorf("api errors are non-fatal and should return nil, got '%v'", apiErr)
+	}
+
+	err = errors.New("random error")
+	if apiErr := CheckAPICompat(client, err); apiErr == nil {
+		t.Error("expected error to be returned, got nil")
+	}
+}

pkg/controller/utils_test.go

@@ -11,7 +11,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	builderconf "github.com/deis/builder/pkg/conf"
 	"github.com/deis/builder/pkg/controller"
 	"github.com/deis/builder/pkg/git"
 	"github.com/deis/builder/pkg/k8s"
@@ -62,7 +61,7 @@ func build(
 
 	slugBuilderImagePullPolicy, err := k8s.PullPolicyFromString(conf.SlugBuilderImagePullPolicy)
 	if err != nil {
-		return nil
+		return err
 	}
 
 	repo := conf.Repository
@@ -91,7 +90,7 @@ func build(
 		}
 	}()
 
-	client, err := controller.New(conf.ControllerHost, conf.ControllerPort, builderconf.BuilderKeyLocation)
+	client, err := controller.New(conf.ControllerHost, conf.ControllerPort)
 	if err != nil {
 		return err
 	}