From 11540067c546c53d652d6c3099f9c5411452810f Mon Sep 17 00:00:00 2001 From: Laurentiu Badea Date: Wed, 21 Oct 2020 18:04:58 +0000 Subject: [PATCH] Address feedback Bail out if http endpoint isn't parseable. Add custom messages to test assertions. Check error assertions before the data. --- .../backends/http_fetcher/fetcher.go | 4 +- .../backends/http_fetcher/fetcher_test.go | 38 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/stored_requests/backends/http_fetcher/fetcher.go b/stored_requests/backends/http_fetcher/fetcher.go index 4a148377664..203150c0689 100644 --- a/stored_requests/backends/http_fetcher/fetcher.go +++ b/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 } diff --git a/stored_requests/backends/http_fetcher/fetcher_test.go b/stored_requests/backends/http_fetcher/fetcher_test.go index 8e2375a362c..dc8efe5cccf 100644 --- a/stored_requests/backends/http_fetcher/fetcher_test.go +++ b/stored_requests/backends/http_fetcher/fetcher_test.go @@ -18,9 +18,9 @@ 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) { @@ -28,9 +28,9 @@ func TestSeveralReqs(t *testing.T) { 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) { @@ -38,9 +38,9 @@ func TestSingleImp(t *testing.T) { 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) { @@ -48,9 +48,9 @@ func TestSeveralImps(t *testing.T) { 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) { @@ -58,9 +58,9 @@ func TestReqsAndImps(t *testing.T) { 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) { @@ -68,9 +68,9 @@ func TestMissingValues(t *testing.T) { 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) { @@ -78,7 +78,7 @@ func TestFetchAccounts(t *testing.T) { 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,8 +87,8 @@ 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) { @@ -96,8 +96,8 @@ func TestFetchAccountsBadJSON(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 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) {