Merge pull request #49 from Financial-Times/fix/UPPSF-1235-fix-tests

Remove error string comparisons from tests

.circleci/config.yml

@@ -1,52 +1,7 @@
-version: 2
+version: 2.1
+orbs:
+  ft-golang-ci: financial-times/golang-ci@1
 jobs:
-  build:
-    working_directory: /go/src/github.com/Financial-Times/draft-annotations-api
-    docker:
-      - image: golang:1
-        environment:
-          GOPATH: /go
-          CIRCLE_TEST_REPORTS: /tmp/test-results
-          CIRCLE_COVERAGE_REPORT: /tmp/coverage-results
-    steps:
-      - checkout
-      - run:
-          name: External Dependencies
-          command: |
-           GO111MODULE=off go get -u github.com/mattn/goveralls
-           GO111MODULE=off go get -u github.com/jstemmer/go-junit-report
-           curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.18.0
-           wget https://raw.githubusercontent.com/Financial-Times/upp-coding-standard/v1.0.0/golangci-config/.golangci.yml
-      - run:
-          name: Test Results
-          command: |
-            mkdir -p ${CIRCLE_TEST_REPORTS}
-            mkdir -p ${CIRCLE_COVERAGE_REPORT}
-      - run:
-          name: Go Build
-          command: go build -mod=readonly -v
-      - run:
-          name: Run linters
-          command: golangci-lint run --new-from-rev=master --config .golangci.yml
-      - run:
-          name: Run Tests
-          command: |
-            go test -mod=readonly -race -cover -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out ./... | go-junit-report > ${CIRCLE_TEST_REPORTS}/junit.xml
-      - run:
-          name: Upload Coverage
-          command: goveralls -coverprofile=${CIRCLE_COVERAGE_REPORT}/coverage.out -service=circle-ci -repotoken=${COVERALLS_TOKEN}
-      - store_test_results:
-          path: /tmp/test-results
-  dockerfile:
-    working_directory: /draft-content-api
-    docker:
-      - image: docker:18
-    steps:
-      - checkout
-      - setup_docker_engine
-      - run:
-          name: Build Dockerfile
-          command: docker build .
   dredd:
     working_directory: /go/src/github.com/Financial-Times/draft-annotations-api
     docker:
@@ -60,12 +15,12 @@ jobs:
       - image: peteclarkft/ersatz:stable
     steps:
       - checkout
-      - run:
-          name: Go Build
-          command: go build -mod=readonly -v
       - run:
           name: Load ersatz-fixtures.yml to ersatz image
           command: "curl -X POST --data-binary @_ft/ersatz-fixtures.yml -H \"Content-type: text/x-yaml\" http://localhost:9000/__configure"
+      - run:
+          name: Go Build
+          command: go build -mod=readonly -v
       - run:
           name: Download dredd
           command: |
@@ -77,11 +32,17 @@ jobs:
           name: Dredd API Testing
           command: dredd
 workflows:
-  version: 2
   test-and-build-docker:
     jobs:
-      - build
+      - ft-golang-ci/build-and-test:
+          name: build-and-test-project
       - dredd
-      - dockerfile:
+      - ft-golang-ci/docker-build:
+          name: build-docker-image
           requires:
-            - build
+            - build-and-test-project
+  snyk-scanning:
+    jobs:
+      - ft-golang-ci/scan:
+          name: scan-dependencies
+          context: cm-team-snyk

annotations/annotations_api.go

@@ -167,23 +167,23 @@ func (api *UPPAnnotationsAPI) GTG() error {
 	apiReqURI := fmt.Sprintf(api.endpointTemplate, syntheticContentUUID)
 	apiReq, err := http.NewRequest("GET", apiReqURI, nil)
 	if err != nil {
-		return fmt.Errorf("gtg request error: %v", err.Error())
+		return fmt.Errorf("GTG: %w", err)
 	}
 
 	apiReq.Header.Set(apiKeyHeader, api.apiKey)
 
 	apiResp, err := api.httpClient.Do(apiReq)
 	if err != nil {
-		return fmt.Errorf("gtg call error: %v", err.Error())
+		return fmt.Errorf("GTG: %w", err)
 	}
 	defer apiResp.Body.Close()
 
 	if apiResp.StatusCode != http.StatusOK {
 		errMsgBody, err := ioutil.ReadAll(apiResp.Body)
 		if err != nil {
-			return fmt.Errorf("gtg returned a non-200 HTTP status [%v]", apiResp.StatusCode)
+			return fmt.Errorf("status %d: %w", apiResp.StatusCode, ErrGTGNotOK)
 		}
-		return fmt.Errorf("gtg returned a non-200 HTTP status [%v]: %v", apiResp.StatusCode, string(errMsgBody))
+		return fmt.Errorf("status %d %s: %w", apiResp.StatusCode, string(errMsgBody), ErrGTGNotOK)
 	}
 	return nil
 }

annotations/annotations_api_test.go

@@ -2,16 +2,18 @@ package annotations
 
 import (
 	"context"
+	"errors"
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
 	"github.com/Financial-Times/go-ft-http/fthttp"
 	tidUtils "github.com/Financial-Times/transactionid-utils-go"
 	"github.com/husobee/vestigo"
-	"github.com/satori/go.uuid"
+	uuid "github.com/satori/go.uuid"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -34,7 +36,7 @@ func TestUnhappyAnnotationsAPIGTG(t *testing.T) {
 
 	annotationsAPI := NewUPPAnnotationsAPI(testClient, annotationsServerMock.URL+"/content/%v/annotations", testAPIKey)
 	err := annotationsAPI.GTG()
-	assert.EqualError(t, err, "gtg returned a non-200 HTTP status [503]: I am not happy!")
+	assert.True(t, errors.Is(err, ErrGTGNotOK))
 }
 
 func TestAnnotationsAPIGTGWrongAPIKey(t *testing.T) {
@@ -43,13 +45,15 @@ func TestAnnotationsAPIGTGWrongAPIKey(t *testing.T) {
 
 	annotationsAPI := NewUPPAnnotationsAPI(testClient, annotationsServerMock.URL+"/content/%v/annotations", "a-non-existing-key")
 	err := annotationsAPI.GTG()
-	assert.EqualError(t, err, "gtg returned a non-200 HTTP status [401]: unauthorized")
+	assert.True(t, errors.Is(err, ErrGTGNotOK))
 }
 
 func TestAnnotationsAPIGTGInvalidURL(t *testing.T) {
 	annotationsAPI := NewUPPAnnotationsAPI(testClient, ":#", testAPIKey)
 	err := annotationsAPI.GTG()
-	assert.EqualError(t, err, "gtg request error: parse :: missing protocol scheme")
+	var urlErr *url.Error
+	assert.True(t, errors.As(err, &urlErr))
+	assert.Equal(t, urlErr.Op, "parse")
 }
 
 func TestAnnotationsAPIGTGConnectionError(t *testing.T) {

annotations/annotations_rw.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -32,6 +33,10 @@ func NewRW(client *http.Client, endpoint string) RW {
 	return &annotationsRW{endpoint, client}
 }
 
+var ErrUnexpectedStatusRead = errors.New("annotations RW returned an unexpected HTTP status code in read operation")
+var ErrUnexpectedStatusWrite = errors.New("annotations RW returned an unexpected HTTP status code in write operation")
+var ErrGTGNotOK = errors.New("gtg returned a non-200 HTTP status")
+
 func (rw *annotationsRW) Read(ctx context.Context, contentUUID string) (*Annotations, string, bool, error) {
 	tid, err := tidUtils.GetTransactionIDFromContext(ctx)
 
@@ -72,7 +77,7 @@ func (rw *annotationsRW) Read(ctx context.Context, contentUUID string) (*Annotat
 	case http.StatusNotFound:
 		return nil, "", false, nil
 	default:
-		return nil, "", false, fmt.Errorf("annotations RW returned an unexpected HTTP status code in read operation: %v", resp.StatusCode)
+		return nil, "", false, fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedStatusRead)
 	}
 }
 
@@ -116,7 +121,7 @@ func (rw *annotationsRW) Write(ctx context.Context, contentUUID string, annotati
 		newHash := resp.Header.Get(DocumentHashHeader)
 		return newHash, nil
 	default:
-		return "", fmt.Errorf("annotations RW returned an unexpected HTTP status code in write operation: %v", resp.StatusCode)
+		return "", fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedStatusWrite)
 	}
 }
 
@@ -128,19 +133,22 @@ func (rw *annotationsRW) GTG() error {
 	req, err := http.NewRequest("GET", rw.endpoint+"/__gtg", nil)
 	if err != nil {
 		log.WithError(err).Error("Error in creating the HTTP request to annotations RW GTG")
-		return fmt.Errorf("gtg HTTP request error: %v", err)
+		return fmt.Errorf("GTG: %w", err)
 	}
 
 	resp, err := rw.httpClient.Do(req)
 	if err != nil {
 		log.WithError(err).Error("Error making the HTTP request to annotations RW GTG")
-		return fmt.Errorf("gtg HTTP call error: %v", err)
+		return fmt.Errorf("GTG: %w", err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		body, _ := ioutil.ReadAll(resp.Body)
-		return fmt.Errorf("gtg returned unexpected status %v: %v", resp.StatusCode, string(body))
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			return fmt.Errorf("status %d: %w", resp.StatusCode, ErrGTGNotOK)
+		}
+		return fmt.Errorf("status %d %s: %w", resp.StatusCode, string(body), ErrGTGNotOK)
 	}
 
 	return nil

annotations/annotations_rw_test.go

@@ -2,10 +2,13 @@ package annotations
 
 import (
 	"context"
+	"encoding/json"
+	"errors"
 	"io/ioutil"
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 	"time"
 
@@ -72,15 +75,15 @@ func TestReadAnnotationsNotFound(t *testing.T) {
 	assert.False(t, found)
 }
 
-func TestRead500Error(t *testing.T) {
+func TestUnhappyReadStatus500(t *testing.T) {
 	tid := tidUtils.NewTransactionID()
 	s := newAnnotationsRWServerMock(t, http.MethodGet, http.StatusInternalServerError, "", "", "", tid)
 	defer s.Close()
 
 	rw := NewRW(testClient, s.URL)
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, _, found, err := rw.Read(ctx, testContentUUID)
-	assert.EqualError(t, err, "annotations RW returned an unexpected HTTP status code in read operation: 500")
+	assert.True(t, errors.Is(err, ErrUnexpectedStatusRead))
 	assert.False(t, found)
 }
 
@@ -90,7 +93,10 @@ func TestReadHTTPRequestError(t *testing.T) {
 	rw := NewRW(testClient, ":#")
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, _, found, err := rw.Read(ctx, testContentUUID)
-	assert.EqualError(t, err, "parse :: missing protocol scheme")
+
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "parse")
 	assert.False(t, found)
 }
 
@@ -100,7 +106,10 @@ func TestReadHTTPCallError(t *testing.T) {
 	rw := NewRW(testClient, "")
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, _, found, err := rw.Read(ctx, testContentUUID)
-	assert.EqualError(t, err, "Get /drafts/content/db4daee0-2b84-465a-addb-fc8938a608db/annotations: unsupported protocol scheme \"\"")
+
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "Get")
 	assert.False(t, found)
 }
 
@@ -112,7 +121,10 @@ func TestReadInvalidBodyError(t *testing.T) {
 	rw := NewRW(testClient, s.URL)
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, _, found, err := rw.Read(ctx, testContentUUID)
-	assert.EqualError(t, err, "invalid character 'i' looking for beginning of object key string")
+	var jsonErr *json.SyntaxError
+	if assert.Error(t, err) {
+		assert.True(t, errors.As(err, &jsonErr))
+	}
 	assert.False(t, found)
 }
 
@@ -171,7 +183,7 @@ func TestUnhappyWriteStatus500(t *testing.T) {
 	rw := NewRW(testClient, s.URL)
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash)
-	assert.EqualError(t, err, "annotations RW returned an unexpected HTTP status code in write operation: 500")
+	assert.True(t, errors.Is(err, ErrUnexpectedStatusWrite))
 }
 
 func TestWriteHTTPRequestError(t *testing.T) {
@@ -180,7 +192,10 @@ func TestWriteHTTPRequestError(t *testing.T) {
 	rw := NewRW(testClient, ":#")
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash)
-	assert.EqualError(t, err, "parse :: missing protocol scheme")
+
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "parse")
 }
 
 func TestWriteHTTPCallError(t *testing.T) {
@@ -189,7 +204,10 @@ func TestWriteHTTPCallError(t *testing.T) {
 	rw := NewRW(testClient, "")
 	ctx := tidUtils.TransactionAwareContext(context.Background(), tid)
 	_, err := rw.Write(ctx, testContentUUID, &expectedCanonicalizedAnnotations, oldHash)
-	assert.EqualError(t, err, "Put /drafts/content/db4daee0-2b84-465a-addb-fc8938a608db/annotations: unsupported protocol scheme \"\"")
+
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "Put")
 }
 
 func TestWriteMissingTID(t *testing.T) {
@@ -264,21 +282,25 @@ func TestRWHappyGTG(t *testing.T) {
 func TestRWHTTPRequestErrorGTG(t *testing.T) {
 	rw := NewRW(testClient, ":#")
 	err := rw.GTG()
-	assert.EqualError(t, err, "gtg HTTP request error: parse :: missing protocol scheme")
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "parse")
 }
 
 func TestRWHTTPCallErrorGTG(t *testing.T) {
 	rw := NewRW(testClient, "")
 	err := rw.GTG()
-	assert.EqualError(t, err, "gtg HTTP call error: Get /__gtg: unsupported protocol scheme \"\"")
+	var urlError *url.Error
+	assert.True(t, errors.As(err, &urlError))
+	assert.Equal(t, urlError.Op, "Get")
 }
 
 func TestRW503GTG(t *testing.T) {
 	s := newAnnotationsRWGTGServerMock(t, http.StatusServiceUnavailable, "service unavailable")
 	defer s.Close()
 	rw := NewRW(testClient, s.URL)
 	err := rw.GTG()
-	assert.EqualError(t, err, "gtg returned unexpected status 503: service unavailable")
+	assert.True(t, errors.Is(err, ErrGTGNotOK))
 }
 
 func newAnnotationsRWGTGServerMock(t *testing.T, status int, body string) *httptest.Server {

concept/read_api.go

@@ -3,7 +3,9 @@ package concept
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io/ioutil"
 	"net/http"
 
 	tidUtils "github.com/Financial-Times/transactionid-utils-go"
@@ -32,6 +34,8 @@ func NewReadAPI(client *http.Client, endpoint string, apiKey string, batchSize i
 	}
 }
 
+var ErrUnexpectedResponse = errors.New("concept search API returned a non-200 HTTP status code")
+
 func (search *internalConcordancesAPI) GetConceptsByIDs(ctx context.Context, conceptIDs []string) (map[string]Concept, error) {
 	tid, err := tidUtils.GetTransactionIDFromContext(ctx)
 	if err != nil {
@@ -92,7 +96,12 @@ func (search *internalConcordancesAPI) searchConceptBatch(ctx context.Context, c
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		err = fmt.Errorf("concept search API returned a non-200 HTTP status code: %v", resp.StatusCode)
+		body, e := ioutil.ReadAll(resp.Body)
+		if e != nil {
+			err = fmt.Errorf("status %d: %w", resp.StatusCode, ErrUnexpectedResponse)
+		} else {
+			err = fmt.Errorf("status %d %s: %w", resp.StatusCode, string(body), ErrUnexpectedResponse)
+		}
 		batchConceptsLog.WithError(err).Error("Error received from concept search API")
 		return nil, err
 	}