Fixed duplicates in finds + work on unit tests + vendor cleanup

Gopkg.toml

@@ -105,10 +105,6 @@
   name = "github.com/spf13/viper"
   version = "1.0.2"
 
-[[constraint]]
-  name = "github.com/stretchr/testify"
-  version = "1.2.2"
-
 [[constraint]]
   branch = "master"
   name = "github.com/wangjohn/quickselect"

Makefile

@@ -40,7 +40,9 @@ vet:
 lint:
 	gometalinter --vendor --deadline=150s --cyclo-over=15 --exclude="\bexported \w+ (\S*['.]*)([a-zA-Z'.*]*) should have comment or be unexported\b" ./...
 
-check:
+check: test vet
+
+test:
 	$(PKGCONF) $(GO) test ./... -race -coverprofile=coverage.txt -covermode=atomic
 
 clean:

app/carbonapi/app_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/bookingcom/carbonapi/pkg/types/encoding/json"
 
 	"github.com/lomik/zapwriter"
-	"github.com/stretchr/testify/assert"
 )
 
 var testApp *App
@@ -142,13 +141,10 @@ func TestRenderHandler(t *testing.T) {
 
 	expected := `[{"target":"foo.bar","datapoints":[[null,1510913280],[1510913759,1510913340],[1510913818,1510913400]]}]`
 
-	// Check the status code is what we expect.
-	r := assert.Equal(t, rr.Code, http.StatusOK, "HttpStatusCode should be 200 OK.")
-	if !r {
+	if rr.Code != http.StatusOK {
 		t.Error("HttpStatusCode should be 200 OK.")
 	}
-	r = assert.Equal(t, expected, rr.Body.String(), "Http response should be same.")
-	if !r {
+	if expected != rr.Body.String() {
 		t.Error("Http response should be same.")
 	}
 }
@@ -159,13 +155,13 @@ func TestFindHandler(t *testing.T) {
 
 	body := rr.Body.String()
 	// LOL this test is so fragile
+	// TODO (grzkv): It can be made not-fragile by unmarshalling first
+	// ...or using JSONEq, but this would be bloat
 	expected := "[{\"allowChildren\":0,\"context\":{},\"expandable\":0,\"id\":\"foo.bar\",\"leaf\":1,\"text\":\"bar\"}]"
-	r := assert.Equal(t, rr.Code, http.StatusOK, "HttpStatusCode should be 200 OK.")
-	if !r {
+	if rr.Code != http.StatusOK {
 		t.Error("HttpStatusCode should be 200 OK.")
 	}
-	r = assert.Equal(t, string(expected), body, "Http response should be same.")
-	if !r {
+	if body != expected {
 		t.Error("Http response should be same.")
 	}
 }
@@ -177,13 +173,11 @@ func TestFindHandlerCompleter(t *testing.T) {
 		testApp.findHandler(rr, req)
 		body := rr.Body.String()
 		expectedValue, _ := findCompleter(getMetricGlobResponse(getCompleterQuery(testMetric)))
-		r := assert.Equal(t, rr.Code, http.StatusOK, "HttpStatusCode should be 200 OK.")
-		if !r {
+		if rr.Code != http.StatusOK {
 			t.Error("HttpStatusCode should be 200 OK.")
 		}
-		r = assert.Equal(t, string(expectedValue), body, "Http response should be same.")
-		if !r {
-			t.Error("Http response should be same.")
+		if string(expectedValue) != body {
+			t.Error("HTTP response should be same.")
 		}
 	}
 }
@@ -195,17 +189,15 @@ func TestInfoHandler(t *testing.T) {
 	body := rr.Body.String()
 	expected := getMockInfoResponse()
 	expectedJSON, err := json.InfoEncoder(expected)
-	r := assert.Nil(t, err)
-	if !r {
+	if err != nil {
 		t.Errorf("err should be nil, %v instead", err)
 	}
 
-	r = assert.Equal(t, rr.Code, http.StatusOK, "HttpStatusCode should be 200 OK.")
-	if !r {
+	if rr.Code != http.StatusOK {
 		t.Error("Http response should be same.")
 	}
-	r = assert.Equal(t, string(expectedJSON), body, "Http response should be same.")
-	if !r {
+	// TODO (grzkv): Unmarshal for reliablility
+	if string(expectedJSON) != body {
 		t.Error("Http response should be same.")
 	}
 }

app/carbonapi/http_handlers_test.go

@@ -5,8 +5,6 @@ import (
 	"testing"
 
 	"github.com/bookingcom/carbonapi/pkg/types"
-
-	"github.com/stretchr/testify/assert"
 )
 
 func TestShouldNotBlock(t *testing.T) {
@@ -31,7 +29,9 @@ func TestShouldNotBlockWithoutRule(t *testing.T) {
 
 	req.Header.Add("foo", "bar")
 	// no rules are set
-	assert.Equal(t, false, shouldBlockRequest(req, []Rule{}), "Req should not be blocked")
+	if shouldBlockRequest(req, []Rule{}) {
+		t.Error("Req should not be blocked")
+	}
 }
 
 func TestShouldBlock(t *testing.T) {
@@ -42,7 +42,9 @@ func TestShouldBlock(t *testing.T) {
 
 	req.Header.Add("foo", "bar")
 	rule := Rule{"foo": "bar"}
-	assert.Equal(t, true, shouldBlockRequest(req, []Rule{rule}), "Req should be blocked")
+	if !shouldBlockRequest(req, []Rule{rule}) {
+		t.Error("Req should be blocked")
+	}
 }
 
 func TestGetCompleterQuery(t *testing.T) {
@@ -51,7 +53,9 @@ func TestGetCompleterQuery(t *testing.T) {
 
 	for i, metricTestCase := range metricTestCases {
 		response := getCompleterQuery(metricTestCase)
-		assert.Equal(t, metricCompleterResponse[i], response, "should be same")
+		if metricCompleterResponse[i] != response {
+			t.Error("should be same")
+		}
 	}
 }
 
@@ -69,7 +73,9 @@ func TestFindCompleter(t *testing.T) {
 
 	for i, metricTestCase := range metricTestCases {
 		response, _ := findCompleter(metricTestCase)
-		assert.Equal(t, string(metricFindCompleterResponse[i]), string(response), "should be same")
+		if metricFindCompleterResponse[i] != string(response) {
+			t.Error("should be same")
+		}
 	}
 
 }

pkg/types/encoding/json/encoding.go

@@ -23,14 +23,16 @@ type jsonMatch struct {
 	Text          string         `json:"text"`
 }
 
+// FindEncoder converts matches to JSON data
 func FindEncoder(matches types.Matches) ([]byte, error) {
 	jms := matchesToJSONMatches(matches)
 
 	return json.Marshal(jms)
 }
 
 func matchesToJSONMatches(matches types.Matches) []jsonMatch {
-	jms := make([]jsonMatch, 0, len(matches.Matches))
+	// values are stored in the map first to remove duplicates by ID
+	ms := make(map[string]jsonMatch)
 
 	var basepath string
 	if i := strings.LastIndex(matches.Name, "."); i != -1 {
@@ -61,9 +63,13 @@ func matchesToJSONMatches(matches types.Matches) []jsonMatch {
 
 		// jm.Context not set on purpose; seems to always be empty map?
 
-		jms = append(jms, jm)
+		ms[jm.ID] = jm
 	}
 
+	jms := make([]jsonMatch, 0, len(ms))
+	for _, jm := range ms {
+		jms = append(jms, jm)
+	}
 	return jms
 }
 
@@ -87,6 +93,7 @@ type jsonRet struct {
 	NumberOfPoints  int32 `json:"numberOfPoints"`
 }
 
+// InfoEncoder converts acquired info data to JSON string
 func InfoEncoder(infos []types.Info) ([]byte, error) {
 	jsonInfos := make(map[string]jsonInfo)
 
@@ -111,6 +118,7 @@ func InfoEncoder(infos []types.Info) ([]byte, error) {
 	return json.Marshal(jsonInfos)
 }
 
+// InfoDecoder converts JSON string to metrics info
 func InfoDecoder(blob []byte) ([]types.Info, error) {
 	jsonInfos := make(map[string]jsonInfo)
 	if err := json.Unmarshal(blob, &jsonInfos); err != nil {
@@ -145,6 +153,7 @@ type jsonMetric struct {
 	Datapoints [][]interface{} `json:"datapoints"`
 }
 
+// RenderEncoder converts metrics data to JSON format
 func RenderEncoder(metrics []types.Metric) ([]byte, error) {
 	jms := make([]jsonMetric, 0, len(metrics))
 
@@ -177,6 +186,7 @@ func RenderEncoder(metrics []types.Metric) ([]byte, error) {
 	return json.Marshal(jms)
 }
 
+// RenderDecoder converts JSON string to metrics data
 func RenderDecoder(blob []byte) ([]types.Metric, error) {
 	jms := make([]jsonMetric, 0)
 	if err := json.Unmarshal(blob, &jms); err != nil {