go vet yourself (#397)

go vet caught some nifty bugs. so fixed those here, and also made it so that
we vet everything from now on since the robots seem to do a better job of
vetting than we have managed to.

also adds gofmt check to circle. could move this to the test.sh script (didn't
want a script calling a script, because $reasons) and it's nice and isolated
in its own little land as it is. side note, changed the script so it runs in
100ms instead of 3s, i think find is a lot faster than go list.

attempted some minor cleanup of various scripts

.circleci/config.yml

@@ -19,11 +19,7 @@ jobs:
           cd tmp
           sudo rm -rf /usr/local/go
           wget https://storage.googleapis.com/golang/go$GOVERSION.$OS-$ARCH.tar.gz
-          # mkdir -p $HOME/golang
-          # tar -C $HOME/golang -xzf go1.8.3.linux-amd64.tar.gz
           sudo tar -C /usr/local -xzf go$GOVERSION.$OS-$ARCH.tar.gz
-          # go get -u github.com/golang/dep/...
-          # go get -u github.com/Masterminds/glide
       - run: go version
       # update Docker
       - run: |
@@ -31,11 +27,10 @@ jobs:
           sudo service docker stop
           curl -fsSL https://get.docker.com/ | sudo sh
       - run: docker version
-      # login here for tests
-      # - run: docker login -u $DOCKER_USER -p $DOCKER_PASS
       - run: make docker-build
       - run: make install
       - run: make test
+      - run: ./go-fmt.sh
       # TODO these should be inside test.sh file ?
       - run: ./api_test.sh mysql 4
       - run: ./api_test.sh postgres 4

Makefile

@@ -16,9 +16,6 @@ install:
 test:
 	./test.sh
 
-fmt:
-	./go-fmt.sh
-
 test-datastore:
 	cd api/datastore && go test -v ./...
 

api/agent/drivers/docker/docker_test.go

@@ -112,6 +112,6 @@ func TestRegistry(t *testing.T) {
 	}
 
 	if size <= 0 {
-		t.Fatalf("expected positive size for image that exists, got size:", size)
+		t.Fatal("expected positive size for image that exists, got size:", size)
 	}
 }

api/agent/func_logger.go

@@ -185,7 +185,7 @@ func (l *limitWriter) Write(b []byte) (int, error) {
 	l.n += n
 	if l.n >= l.max {
 		// write in truncation message to log once
-		l.Writer.Write([]byte(fmt.Sprintf("\n-----max log size %d bytes exceeded, truncating log-----\n")))
+		l.Writer.Write([]byte(fmt.Sprintf("\n-----max log size %d bytes exceeded, truncating log-----\n", l.max)))
 	}
 	return n, err
 }

api/datastore/internal/datastoretest/test.go

@@ -171,7 +171,7 @@ func Test(t *testing.T, dsf func() models.Datastore) {
 			t.Fatalf("Test GetCalls(ctx, filter): unexpected length `%v`", len(calls))
 		} else if calls[0].ID != c2.ID {
 			t.Log(buf.String())
-			t.Fatalf("Test GetCalls: call id not expected", calls[0].ID, c2.ID)
+			t.Fatalf("Test GetCalls: call id not expected %s vs %s", calls[0].ID, c2.ID)
 		}
 	})
 
@@ -387,7 +387,7 @@ func Test(t *testing.T, dsf func() models.Datastore) {
 		_, err := ds.InsertApp(ctx, testApp)
 		if err != nil && err != models.ErrAppsAlreadyExists {
 			t.Log(buf.String())
-			t.Fatalf("Test InsertRoute Prep: failed to insert app: ", err)
+			t.Fatal("Test InsertRoute Prep: failed to insert app: ", err)
 		}
 
 		// Testing insert route

api/id/id.go

@@ -26,10 +26,10 @@ func SetMachineId(id uint64) {
 // with an addition that net.IP must be a ipv4 address.
 func SetMachineIdHost(addr net.IP, port uint16) {
 	var machineId uint64 // 48 bits
-	machineId |= uint64(addr[0] << 40)
-	machineId |= uint64(addr[1] << 32)
-	machineId |= uint64(addr[2] << 24)
-	machineId |= uint64(addr[3] << 16)
+	machineId |= uint64(addr[0]) << 40
+	machineId |= uint64(addr[1]) << 32
+	machineId |= uint64(addr[2]) << 24
+	machineId |= uint64(addr[3]) << 16
 	machineId |= uint64(port)
 
 	SetMachineId(machineId)

api/models/route.go

@@ -27,7 +27,7 @@ type Route struct {
 	Memory      uint64  `json:"memory" db:"memory"`
 	Headers     Headers `json:"headers" db:"headers"`
 	Type        string  `json:"type" db:"type"`
-	Format      string  `json:"format" db":format"`
+	Format      string  `json:"format" db:"format"`
 	Timeout     int32   `json:"timeout" db:"timeout"`
 	IdleTimeout int32   `json:"idle_timeout" db:"idle_timeout"`
 	Config      Config  `json:"config" db:"config"`

api/server/server.go

@@ -109,7 +109,7 @@ func traceWrap(c *gin.Context) {
 	// Create the span referring to the RPC client if available.
 	// If wireContext == nil, a root span will be created.
 	// TODO we should add more tags?
-	serverSpan := opentracing.StartSpan("serve_http", ext.RPCServerOption(wireContext), opentracing.Tag{"path", c.Request.URL.Path})
+	serverSpan := opentracing.StartSpan("serve_http", ext.RPCServerOption(wireContext), opentracing.Tag{Key: "path", Value: c.Request.URL.Path})
 	defer serverSpan.Finish()
 
 	ctx := opentracing.ContextWithSpan(c.Request.Context(), serverSpan)

examples/tutorial/hello/go/func.go

@@ -22,6 +22,6 @@ func main() {
 	log.Println("---> stderr goes to the server logs.")
 	log.Println("---> LINE 2")
 	log.Println("---> LINE 3 with a break right here\nand LINE 4")
-	log.Println("---> LINE 5 with a double line break\n")
+	log.Printf("---> LINE 5 with a double line break\n\n")
 	log.Println("---> LINE 6")
 }

fnlb/lb/ch.go

@@ -200,6 +200,7 @@ func (ch *chRouter) besti(key string, i int, nodes []string) (string, error) {
 
 	for ; ; i++ {
 		// theoretically this could take infinite time, but practically improbable...
+		// TODO we need a way to add a node for a given key from down here if a node is overloaded.
 		node := f(nodes[i])
 		if node != "" {
 			return node, nil
@@ -207,10 +208,6 @@ func (ch *chRouter) besti(key string, i int, nodes []string) (string, error) {
 			i = -1 // reset i to 0
 		}
 	}
-
-	// TODO we need a way to add a node for a given key from down here if a node is overloaded.
-
-	panic("strange things are afoot at the circle k")
 }
 
 func translate(val, inFrom, inTo, outFrom, outTo int64) int {

fnlb/lb/proxy.go

@@ -160,7 +160,7 @@ func (p *proxy) startSpan(req *http.Request) (opentracing.Span, *http.Request) {
 	// Create the span referring to the RPC client if available.
 	// If wireContext == nil, a root span will be created.
 	// TODO we should add more tags?
-	serverSpan := opentracing.StartSpan("lb_serve", ext.RPCServerOption(wireContext), opentracing.Tag{"path", req.URL.Path})
+	serverSpan := opentracing.StartSpan("lb_serve", ext.RPCServerOption(wireContext), opentracing.Tag{Key: "path", Value: req.URL.Path})
 
 	ctx := opentracing.ContextWithSpan(req.Context(), serverSpan)
 	req = req.WithContext(ctx)

go-fmt.sh

@@ -1,23 +1,12 @@
-#! /bin/sh
+#!/bin/sh
+# find and output all Go files that are not correctly formatted
+
 set -e
 
-function listFilesExit () {
-	echo The following files need to have go fmt ran:
-	echo $NEED_TO_FORMAT
-	exit 1
-}
+# Find all .go files except those under vendor/ or .git, run gofmt -l on them
+OUT=$(find ! \( -path ./vendor -prune \) ! \( -path ./.git -prune \) -name '*.go' -exec gofmt -l {} +)
 
-FOLDERS=$(go list -f {{.Dir}} ./... | grep -v vendor)
-for i in $FOLDERS
-do
-cd $i
-FILES=$(ls *.go)
-	for j in $FILES
-	do
-	#echo $i/$j
-	ALL_FILES="$ALL_FILES $i/$j"
-	done
-done
-#echo $ALL_FILES
-NEED_TO_FORMAT="$(gofmt -l $ALL_FILES)"
-[[ -z $NEED_TO_FORMAT ]] || listFilesExit
+if [ -n "$OUT" ]; then
+  echo "$OUT"
+  exit 1
+fi

test.sh

@@ -35,7 +35,8 @@ container_ip)
     ;;
 esac
 
-go test -v $(go list ./... | grep -v vendor | grep -v examples | grep -v tool | grep -v cli | grep -v tmp/go/src | grep -v test/fn-api-tests)
+go test -v $(go list ./... | grep -v vendor | grep -v examples | grep -v test/fn-api-tests)
+go vet -v $(go list ./... | grep -v vendor)
 docker rm --force func-postgres-test 
 docker rm --force func-mysql-test