Merge pull request #9 from marshyski/master

Add tests and better error handling for configs

Author: kood1

.circleci/config.yml

@@ -4,9 +4,17 @@ jobs:
     docker:
       - image: circleci/golang:1.9.1
     working_directory: /go/src/github.com/FINRAOS/yum-nginx-api
+    environment:
+      TEST_RESULTS: /tmp/test-results
     steps:
       - setup_remote_docker
       - run: git clone https://github.com/FINRAOS/yum-nginx-api.git -b $CIRCLE_BRANCH .
+      - run:
+          shell: /bin/bash
+          command: |
+            mkdir -p $TEST_RESULTS
+            go get github.com/jstemmer/go-junit-report
+            make test | go-junit-report > ${TEST_RESULTS}/go-test-report.xml
       - run:
           shell: /bin/bash
           command: |
@@ -17,4 +25,6 @@ jobs:
           shell: /bin/bash
           command: |
             docker login -u $DOCKER_USER -p $DOCKER_PASS
-            docker push finraos/yum-nginx-api:latest
+            docker push finraos/yum-nginx-api:latest
+      - store_test_results:
+          path: /tmp/test-results

Makefile

@@ -1,22 +1,16 @@
 PACKAGE_NAME:='yumapi'
 BUILT_ON:=$(shell date)
 COMMIT_HASH:=$(shell git log -n 1 --pretty=format:"%H")
-PACKAGES:=$(shell go list ./... | sed -n '1!p' | grep -v /vendor/)
+PACKAGES:=$(shell go list ./... | grep -v /vendor/)
 LDFLAGS:='-X "main.builtOn=$(BUILT_ON)" -X "main.commitHash=$(COMMIT_HASH)"'
 
 default: docker
 
 test:
-	echo "mode: count" > coverage-all.out
-	$(foreach pkg,$(PACKAGES), \
-		go test -p=1 -cover -covermode=count -coverprofile=coverage.out ${pkg}; \
-		tail -n +2 coverage.out >> coverage-all.out;)
-
-cover: test
-	go tool cover -html=coverage-all.out
+	go test -cover -v $(PACKAGES)
 
 run: config
-	go run -ldflags $(LDFLAGS) *.go
+	go run -ldflags $(LDFLAGS) `find . | grep -v 'test\|vendor\|repo' | grep \.go`
 
 # Cross-compile from OS X to Linux using xgo
 cc:

config.go

@@ -12,14 +12,22 @@
 package main
 
 import (
-	"fmt"
+	"errors"
 	"os"
 	"path"
 
 	"github.com/FINRAOS/yum-nginx-api/repojson"
 	"github.com/spf13/viper"
 )
 
+const (
+	crError = "config: createrepo_workers is less than 1"
+	mlError = "config: max_content_length is less than 1MB"
+	upError = "config: upload_directory does not exist"
+	ptError = "config: port is not above port 80"
+	mxError = "config: max_retries is less than 1"
+)
+
 // Some vars are used with different types in handlers vs validation
 var (
 	builtOn    string
@@ -36,14 +44,15 @@ var (
 	crBin      string
 )
 
-func init() {
+// Validate configurations and if createrepo binary is present in path
+func configValidate() error {
 	viper.SetConfigName("yumapi")
 	viper.AddConfigPath("/opt/yum-nginx-api/yumapi/")
 	viper.AddConfigPath("/etc/yumapi/")
 	viper.AddConfigPath(".")
 	err := viper.ReadInConfig()
 	if err != nil {
-		panic(fmt.Errorf("Fatal error config file: %s \n", err))
+		return errors.New("Fatal error config file: " + err.Error())
 	}
 
 	viper.SetDefault("createrepo_workers", 1)
@@ -52,10 +61,7 @@ func init() {
 	viper.SetDefault("port", 8080)
 	viper.SetDefault("dev_mode", false)
 	viper.SetDefault("max_retries", 3)
-}
 
-// Validate configurations and if createrepo binary is present in path
-func configValidate() {
 	createRepo = viper.GetString("createrepo_workers")
 	maxLength = viper.GetInt64("max_content_length")
 	uploadDir = path.Clean(viper.GetString("upload_dir")) + "/"
@@ -64,16 +70,19 @@ func configValidate() {
 	maxRetries = viper.GetInt("max_retries")
 
 	if viper.GetInt64("createrepo_workers") < 1 {
-		panic(fmt.Errorf("createrepo_workers is less than 1"))
+		return errors.New(crError)
 	}
 	if maxLength < 1000000 {
-		panic(fmt.Errorf("max_content_length is less than 1MB"))
+		return errors.New(mlError)
 	}
 	if _, err := os.Stat(uploadDir); os.IsNotExist(err) {
-		panic(fmt.Errorf("upload_directory %s does not exist", uploadDir))
+		return errors.New(upError)
 	}
 	if viper.GetInt64("port") < 80 {
-		panic(fmt.Errorf("port is not above port 80"))
+		return errors.New(ptError)
+	}
+	if maxRetries < 1 {
+		return errors.New(mxError)
 	}
 	if !devMode {
 		for _, cr := range crPaths {
@@ -83,10 +92,8 @@ func configValidate() {
 			}
 		}
 		if crBin == "" {
-			panic(fmt.Errorf("createrepo binary not found in path"))
+			return errors.New(crError)
 		}
 	}
-	if maxRetries < 1 {
-		panic(fmt.Errorf("max_retries is less than 1"))
-	}
+	return nil
 }

config_test.go

@@ -0,0 +1,88 @@
+package main
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+)
+
+const testFile = "./yumapi.yaml"
+
+// TestConfigValidate validates all configuration
+// options in yumapi config file
+func TestConfigValidate(t *testing.T) {
+	b := []byte("upload_dir: .\ncreaterepo_workers: 2\ndev_mode: true\nmax_content_length: 220000000\nport: 80\nmax_retries: 1")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err != nil {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}
+
+// TestConfigCR verifies error returned for zero workers
+func TestConfigCR(t *testing.T) {
+	b := []byte("createrepo_workers: 0")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err.Error() != crError {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}
+
+// TestConfigML verifies max_content_length is not too low
+func TestConfigML(t *testing.T) {
+	b := []byte("max_content_length: 900000")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err.Error() != mlError {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}
+
+// TestConfigUp verifies upload_dir does not exist
+func TestConfigUP(t *testing.T) {
+	b := []byte("upload_dir: /supp")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err.Error() != upError {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}
+
+// TestConfigPT verifies ports under 80 are not allowed
+func TestConfigPT(t *testing.T) {
+	b := []byte("port: 22")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err.Error() != ptError {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}
+
+// TestConfigMX verifies max retries are not less than 1
+func TestConfigMX(t *testing.T) {
+	b := []byte("max_retries: 0")
+	if err := ioutil.WriteFile(testFile, b, 0644); err != nil {
+		t.Fatalf("Error writing file")
+	}
+	if err := configValidate(); err.Error() != mxError {
+		os.Remove(testFile)
+		t.Fatal(err)
+	}
+	os.Remove(testFile)
+}

handlers_test.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/go-ozzo/ozzo-routing"
+	"github.com/go-ozzo/ozzo-routing/content"
+)
+
+func newRouter() *routing.Router {
+	rtr := routing.New()
+	rtr.Group("/api", content.TypeNegotiator(content.JSON))
+	return rtr
+}
+func testHealthHandler(t *testing.T, rtr *routing.Router) {
+	rtr.Get("/health", healthRoute)
+	req, err := http.NewRequest("GET", "/health", nil)
+	req.Header.Set("Content-Type", "application/json")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rr := httptest.NewRecorder()
+	rtr.ServeHTTP(rr, req)
+	if status := rr.Code; status != http.StatusOK {
+		t.Errorf("handler returned wrong status code: got %v want %v",
+			status, http.StatusOK)
+	}
+
+	expected := `OK`
+	if rr.Body.String() != expected {
+		t.Errorf("handler returned unexpected body: got %v want %v",
+			rr.Body.String(), expected)
+	}
+}
+
+func testRepoHandler(t *testing.T, rtr *routing.Router) {
+	rtr.Get("/repo", repoRoute)
+	req, err := http.NewRequest("GET", "/repo", nil)
+	req.Header.Set("Content-Type", "application/json")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	rr := httptest.NewRecorder()
+	rtr.ServeHTTP(rr, req)
+	if status := rr.Code; status != http.StatusOK {
+		t.Errorf("handler returned wrong status code: got %v want %v",
+			status, http.StatusOK)
+	}
+
+	expected := `[]`
+	if rr.Body.String() != expected {
+		t.Errorf("handler returned unexpected body: got %v want %v",
+			rr.Body.String(), expected)
+	}
+}
+
+func TestHealth(t *testing.T) {
+	testHealthHandler(t, newRouter())
+}
+
+func TestRepo(t *testing.T) {
+	testRepoHandler(t, newRouter())
+}

main.go

@@ -61,15 +61,15 @@ func uploadRoute(c *routing.Context) error {
 	file, handler, err := c.Request.FormFile("file")
 	if err != nil {
 		c.Response.WriteHeader(http.StatusInternalServerError)
-		c.Write("Upload Failed")
+		c.Write("Upload Failed " + err.Error())
 		return err
 	}
 	defer file.Close()
 	filePath := uploadDir + handler.Filename
 	f, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE, 0644)
 	if err != nil {
 		c.Response.WriteHeader(http.StatusInternalServerError)
-		c.Write("Upload Failed")
+		c.Write("Upload Failed " + err.Error())
 		return err
 	}
 	defer f.Close()
@@ -104,7 +104,9 @@ func repoRoute(c *routing.Context) error {
 }
 
 func main() {
-	configValidate()
+	if err := configValidate(); err != nil {
+		log.Fatalln(err.Error())
+	}
 	go crRoutine()
 	rtr := routing.New()