Address feedback

Bail out if http endpoint isn't parseable.
Add custom messages to test assertions.
Check error assertions before the data.

stored_requests/backends/http_fetcher/fetcher.go

@@ -54,7 +54,7 @@ func NewFetcher(client *http.Client, endpoint string) *HttpFetcher {
 	// `&request-ids=...&imp-ids=...`.
 
 	if url, err := url.Parse(endpoint); err != nil {
-		glog.Errorf(`Invalid endpoint "%s": %v`, endpoint, err)
+		glog.Fatalf(`Invalid endpoint "%s": %v`, endpoint, err)
 	} else {
 		glog.Infof("Making http_fetcher for endpoint %v", url)
 	}
@@ -142,7 +142,7 @@ func (fetcher *HttpFetcher) FetchAccounts(ctx context.Context, accountIDs []stri
 			fmt.Errorf(`Error fetching accounts %v via http: failed to parse response: %v`, accountIDs, err),
 		}
 	}
-	errs := convertNullsToErrs(responseData.Accounts, "Account", nil)
+	errs := convertNullsToErrs(responseData.Accounts, "Account", []error{})
 	return responseData.Accounts, errs
 }
 

stored_requests/backends/http_fetcher/fetcher_test.go

@@ -18,67 +18,67 @@ func TestSingleReq(t *testing.T) {
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), []string{"req-1"}, nil)
+	assert.Empty(t, errs, "Unexpected errors fetching known requests")
 	assertMapKeys(t, reqData, "req-1")
-	assert.Empty(t, impData)
-	assert.Empty(t, errs)
+	assert.Empty(t, impData, "Unexpected imps returned fetching just requests")
 }
 
 func TestSeveralReqs(t *testing.T) {
 	fetcher, close := newTestFetcher(t, []string{"req-1", "req-2"}, nil)
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), []string{"req-1", "req-2"}, nil)
+	assert.Empty(t, errs, "Unexpected errors fetching known requests")
 	assertMapKeys(t, reqData, "req-1", "req-2")
-	assert.Empty(t, impData)
-	assert.Empty(t, errs)
+	assert.Empty(t, impData, "Unexpected imps returned fetching just requests")
 }
 
 func TestSingleImp(t *testing.T) {
 	fetcher, close := newTestFetcher(t, nil, []string{"imp-1"})
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), nil, []string{"imp-1"})
-	assert.Empty(t, reqData)
+	assert.Empty(t, errs, "Unexpected errors fetching known imps")
+	assert.Empty(t, reqData, "Unexpected requests returned fetching just imps")
 	assertMapKeys(t, impData, "imp-1")
-	assert.Empty(t, errs)
 }
 
 func TestSeveralImps(t *testing.T) {
 	fetcher, close := newTestFetcher(t, nil, []string{"imp-1", "imp-2"})
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), nil, []string{"imp-1", "imp-2"})
-	assert.Empty(t, reqData)
+	assert.Empty(t, errs, "Unexpected errors fetching known imps")
+	assert.Empty(t, reqData, "Unexpected requests returned fetching just imps")
 	assertMapKeys(t, impData, "imp-1", "imp-2")
-	assert.Empty(t, errs)
 }
 
 func TestReqsAndImps(t *testing.T) {
 	fetcher, close := newTestFetcher(t, []string{"req-1"}, []string{"imp-1"})
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), []string{"req-1"}, []string{"imp-1"})
+	assert.Empty(t, errs, "Unexpected errors fetching known reqs and imps")
 	assertMapKeys(t, reqData, "req-1")
 	assertMapKeys(t, impData, "imp-1")
-	assert.Empty(t, errs)
 }
 
 func TestMissingValues(t *testing.T) {
 	fetcher, close := newEmptyFetcher(t, []string{"req-1", "req-2"}, []string{"imp-1"})
 	defer close()
 
 	reqData, impData, errs := fetcher.FetchRequests(context.Background(), []string{"req-1", "req-2"}, []string{"imp-1"})
-	assert.Empty(t, reqData)
-	assert.Empty(t, impData)
-	assert.Len(t, errs, 3)
+	assert.Empty(t, reqData, "Fetching unknown reqs should return no reqs")
+	assert.Empty(t, impData, "Fetching unknown imps should return no imps")
+	assert.Len(t, errs, 3, "Fetching 3 unknown reqs+imps should return 3 errors")
 }
 
 func TestFetchAccounts(t *testing.T) {
 	fetcher, close := newTestAccountFetcher(t, []string{"acc-1", "acc-2"})
 	defer close()
 
 	accData, errs := fetcher.FetchAccounts(context.Background(), []string{"acc-1", "acc-2"})
-	assert.Empty(t, errs)
+	assert.Empty(t, errs, "Unexpected error fetching known accounts")
 	assertMapKeys(t, accData, "acc-1", "acc-2")
 }
 
@@ -87,17 +87,17 @@ func TestFetchAccountsNoData(t *testing.T) {
 	defer close()
 
 	accData, errs := fetcher.FetchAccounts(context.Background(), []string{"req-1"})
-	assert.Len(t, errs, 1)
-	assert.Nil(t, accData)
+	assert.Len(t, errs, 1, "Fetching unknown account should have returned an error")
+	assert.Nil(t, accData, "Fetching unknown account should return nil account map")
 }
 
 func TestFetchAccountsBadJSON(t *testing.T) {
 	fetcher, close := newFetcherBadJSON()
 	defer close()
 
 	accData, errs := fetcher.FetchAccounts(context.Background(), []string{"req-1"})
-	assert.Len(t, errs, 1)
-	assert.Nil(t, accData)
+	assert.Len(t, errs, 1, "Fetching account with broken json should have returned an error")
+	assert.Nil(t, accData, "Fetching account with broken json should return nil account map")
 }
 
 func TestFetchAccount(t *testing.T) {
@@ -106,7 +106,7 @@ func TestFetchAccount(t *testing.T) {
 
 	account, errs := fetcher.FetchAccount(context.Background(), "acc-1")
 	assert.Empty(t, errs, "Unexpected error fetching existing account")
-	assert.JSONEq(t, `"acc-1"`, string(account), "Unexpected account data returned")
+	assert.JSONEq(t, `"acc-1"`, string(account), "Unexpected account data fetching existing account")
 }
 
 func TestFetchAccountNoData(t *testing.T) {
@@ -115,7 +115,7 @@ func TestFetchAccountNoData(t *testing.T) {
 
 	unknownAccount, errs := fetcher.FetchAccount(context.Background(), "unknown-acc")
 	assert.NotEmpty(t, errs, "Retrieving unknown account should have returned an error")
-	assert.Nil(t, unknownAccount)
+	assert.Nil(t, unknownAccount, "Retrieving unknown account should return nil json.RawMessage")
 }
 
 func TestErrResponse(t *testing.T) {