Skip to content

Commit

Permalink
feat: improve fetching event for APItest (#788)
Browse files Browse the repository at this point in the history
* Improve fetching event for APItest

* fix lint err

* Update internal/http/apitester.go

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>

* tune rate

* refine test

* move limiter to client

* use cmp

* fix unit test

* Update internal/http/apitester_test.go

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>

* create test retryable http client

* Update internal/http/apitester_test.go

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>

* Update internal/http/apitester_test.go

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>

---------

Co-authored-by: Alex Plischke <alex.plischke@saucelabs.com>
  • Loading branch information
tianfeng92 and alexplischke authored May 26, 2023
1 parent bcf1640 commit 24c4ef8
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 42 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ require (
golang.org/x/sys v0.6.0 // indirect
golang.org/x/term v0.5.0 // indirect
golang.org/x/text v0.7.0
golang.org/x/time v0.3.0
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,8 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.3.0 h1:rg5rLMjNzMS1RkNLzCG38eapWhnYLFYXDXj2gOlr8j4=
golang.org/x/time v0.3.0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190114222345-bf090417da8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20190226205152-f727befe758c/go.mod h1:9Yl7xja0Znq3iFh3HoIrodX9oNMXvdceNzlUR8zjMvY=
Expand Down
55 changes: 32 additions & 23 deletions internal/http/apitester.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,25 @@ import (
"net/url"
"time"

"github.com/hashicorp/go-retryablehttp"
"github.com/saucelabs/saucectl/internal/apitest"
"golang.org/x/time/rate"

"github.com/saucelabs/saucectl/internal/config"
"github.com/saucelabs/saucectl/internal/msg"
)

// Query rate is queryRequestRate per second.
var queryRequestRate = 1
var rateLimitTokenBucket = 10

// APITester describes an interface to the api-testing rest endpoints.
type APITester struct {
HTTPClient *http.Client
URL string
Username string
AccessKey string
HTTPClient *retryablehttp.Client
URL string
Username string
AccessKey string
RequestRateLimiter *rate.Limiter
}

// PublishedTest describes a published test.
Expand All @@ -48,20 +55,18 @@ type vaultErr struct {
// NewAPITester a new instance of APITester.
func NewAPITester(url string, username string, accessKey string, timeout time.Duration) APITester {
return APITester{
HTTPClient: &http.Client{
Timeout: timeout,
Transport: &http.Transport{Proxy: http.ProxyFromEnvironment},
},
URL: url,
Username: username,
AccessKey: accessKey,
HTTPClient: NewRetryableClient(timeout),
URL: url,
Username: username,
AccessKey: accessKey,
RequestRateLimiter: rate.NewLimiter(rate.Every(time.Duration(1/queryRequestRate)*time.Second), rateLimitTokenBucket),
}
}

// GetProject returns Project metadata for a given hookID.
func (c *APITester) GetProject(ctx context.Context, hookID string) (apitest.ProjectMeta, error) {
url := fmt.Sprintf("%s/api-testing/rest/v4/%s", c.URL, hookID)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return apitest.ProjectMeta{}, err
}
Expand Down Expand Up @@ -90,8 +95,12 @@ func (c *APITester) GetProject(ctx context.Context, hookID string) (apitest.Proj
}

func (c *APITester) GetEventResult(ctx context.Context, hookID string, eventID string) (apitest.TestResult, error) {
if err := c.RequestRateLimiter.Wait(ctx); err != nil {
return apitest.TestResult{}, err
}

url := fmt.Sprintf("%s/api-testing/rest/v4/%s/insights/events/%s", c.URL, hookID, eventID)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return apitest.TestResult{}, err
}
Expand Down Expand Up @@ -121,7 +130,7 @@ func (c *APITester) GetEventResult(ctx context.Context, hookID string, eventID s

func (c *APITester) GetTest(ctx context.Context, hookID string, testID string) (apitest.Test, error) {
url := fmt.Sprintf("%s/api-testing/rest/v4/%s/tests/%s", c.URL, hookID, testID)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return apitest.Test{}, err
}
Expand Down Expand Up @@ -185,7 +194,7 @@ func (c *APITester) composeURL(path string, buildID string, format string, tunne
// GetProjects returns the list of Project available.
func (c *APITester) GetProjects(ctx context.Context) ([]apitest.ProjectMeta, error) {
url := fmt.Sprintf("%s/api-testing/api/project", c.URL)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return []apitest.ProjectMeta{}, err
}
Expand Down Expand Up @@ -216,7 +225,7 @@ func (c *APITester) GetProjects(ctx context.Context) ([]apitest.ProjectMeta, err
// GetHooks returns the list of hooks available.
func (c *APITester) GetHooks(ctx context.Context, projectID string) ([]apitest.Hook, error) {
url := fmt.Sprintf("%s/api-testing/api/project/%s/hook", c.URL, projectID)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return []apitest.Hook{}, err
}
Expand Down Expand Up @@ -254,7 +263,7 @@ func (c *APITester) RunAllAsync(ctx context.Context, hookID string, buildID stri
}
payloadReader := bytes.NewReader(payload)

req, err := NewRequestWithContext(ctx, http.MethodPost, url, payloadReader)
req, err := NewRetryableRequestWithContext(ctx, http.MethodPost, url, payloadReader)
if err != nil {
return apitest.AsyncResponse{}, err
}
Expand All @@ -278,7 +287,7 @@ func (c *APITester) RunEphemeralAsync(ctx context.Context, hookID string, buildI
}
payloadReader := bytes.NewReader(payload)

req, err := NewRequestWithContext(ctx, http.MethodPost, url, payloadReader)
req, err := NewRetryableRequestWithContext(ctx, http.MethodPost, url, payloadReader)
if err != nil {
return apitest.AsyncResponse{}, err
}
Expand All @@ -302,7 +311,7 @@ func (c *APITester) RunTestAsync(ctx context.Context, hookID string, testID stri
}
payloadReader := bytes.NewReader(payload)

req, err := NewRequestWithContext(ctx, http.MethodPost, url, payloadReader)
req, err := NewRetryableRequestWithContext(ctx, http.MethodPost, url, payloadReader)
if err != nil {
return apitest.AsyncResponse{}, err
}
Expand All @@ -327,7 +336,7 @@ func (c *APITester) RunTagAsync(ctx context.Context, hookID string, testTag stri
}
payloadReader := bytes.NewReader(payload)

req, err := NewRequestWithContext(ctx, http.MethodPost, url, payloadReader)
req, err := NewRetryableRequestWithContext(ctx, http.MethodPost, url, payloadReader)
if err != nil {
return apitest.AsyncResponse{}, err
}
Expand All @@ -341,7 +350,7 @@ func (c *APITester) RunTagAsync(ctx context.Context, hookID string, testTag stri
return resp, nil
}

func (c *APITester) doAsyncRun(client *http.Client, request *http.Request) (apitest.AsyncResponse, error) {
func (c *APITester) doAsyncRun(client *retryablehttp.Client, request *retryablehttp.Request) (apitest.AsyncResponse, error) {
request.Header.Set("Content-Type", "application/json")

resp, err := client.Do(request)
Expand Down Expand Up @@ -370,7 +379,7 @@ func (c *APITester) doAsyncRun(client *http.Client, request *http.Request) (apit
// GetVault returns the vault for the project identified by hookID
func (c *APITester) GetVault(ctx context.Context, hookID string) (apitest.Vault, error) {
url := fmt.Sprintf("%s/api-testing/rest/v4/%s/vault", c.URL, hookID)
req, err := NewRequestWithContext(ctx, http.MethodGet, url, nil)
req, err := NewRetryableRequestWithContext(ctx, http.MethodGet, url, nil)
if err != nil {
return apitest.Vault{}, err
}
Expand Down Expand Up @@ -408,7 +417,7 @@ func (c *APITester) PutVault(ctx context.Context, hookID string, vault apitest.V
return err
}

req, err := NewRequestWithContext(ctx, http.MethodPut, url, &b)
req, err := NewRetryableRequestWithContext(ctx, http.MethodPut, url, &b)
if err != nil {
return err
}
Expand Down
58 changes: 39 additions & 19 deletions internal/http/apitester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,31 @@ import (
"net/http/httptest"
"reflect"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/go-retryablehttp"
"github.com/saucelabs/saucectl/internal/apitest"
"github.com/saucelabs/saucectl/internal/config"
"github.com/stretchr/testify/assert"
"golang.org/x/time/rate"
)

func createTestRetryableHTTPClient(t *testing.T) *retryablehttp.Client {
return &retryablehttp.Client{
HTTPClient: &http.Client{
Timeout: 10 * time.Second,
Transport: &http.Transport{Proxy: http.ProxyFromEnvironment},
},
RetryWaitMin: 0 * time.Second,
RetryWaitMax: 0 * time.Second,
RetryMax: 1,
CheckRetry: retryablehttp.DefaultRetryPolicy,
Backoff: retryablehttp.DefaultBackoff,
ErrorHandler: retryablehttp.PassthroughErrorHandler,
}
}

func TestAPITester_GetEventResult(t *testing.T) {
type args struct {
ctx context.Context
Expand Down Expand Up @@ -91,10 +110,11 @@ func TestAPITester_GetEventResult(t *testing.T) {
defer ts.Close()

c := &APITester{
HTTPClient: ts.Client(),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
RequestRateLimiter: rate.NewLimiter(rate.Inf, 0),
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -103,7 +123,7 @@ func TestAPITester_GetEventResult(t *testing.T) {
t.Errorf("GetEventResult() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("GetEventResult() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -154,7 +174,7 @@ func TestAPITester_GetProject(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
Expand All @@ -167,7 +187,7 @@ func TestAPITester_GetProject(t *testing.T) {
t.Errorf("GetProject() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("GetProject() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -219,7 +239,7 @@ func TestAPITester_GetTest(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
Expand All @@ -232,7 +252,7 @@ func TestAPITester_GetTest(t *testing.T) {
t.Errorf("GetProject() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("GetProject() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -358,7 +378,7 @@ func TestAPITester_GetProjects(t *testing.T) {
defer ts.Close()

c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
Expand Down Expand Up @@ -445,7 +465,7 @@ func TestAPITester_GetHooks(t *testing.T) {
defer ts.Close()

c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummy",
AccessKey: "accesskey",
Expand Down Expand Up @@ -511,7 +531,7 @@ func TestAPITester_RunAllAsync(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummyUser",
AccessKey: "dummyAccesKey",
Expand All @@ -524,7 +544,7 @@ func TestAPITester_RunAllAsync(t *testing.T) {
t.Errorf("RunAllAsync() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("RunAllAsync() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -589,7 +609,7 @@ func TestAPITester_RunEphemeralAsync(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummyUser",
AccessKey: "dummyAccesKey",
Expand All @@ -600,7 +620,7 @@ func TestAPITester_RunEphemeralAsync(t *testing.T) {
t.Errorf("RunAllAsync() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("RunAllAsync() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -657,7 +677,7 @@ func TestAPITester_RunTestAsync(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummyUser",
AccessKey: "dummyAccesKey",
Expand All @@ -670,7 +690,7 @@ func TestAPITester_RunTestAsync(t *testing.T) {
t.Errorf("RunAllAsync() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("RunAllAsync() got = %v, want %v", got, tt.want)
}
})
Expand Down Expand Up @@ -727,7 +747,7 @@ func TestAPITester_RunTagAsync(t *testing.T) {
}))
defer ts.Close()
c := &APITester{
HTTPClient: ts.Client(),
HTTPClient: createTestRetryableHTTPClient(t),
URL: ts.URL,
Username: "dummyUser",
AccessKey: "dummyAccesKey",
Expand All @@ -740,7 +760,7 @@ func TestAPITester_RunTagAsync(t *testing.T) {
t.Errorf("RunAllAsync() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if !cmp.Equal(got, tt.want) {
t.Errorf("RunAllAsync() got = %v, want %v", got, tt.want)
}
})
Expand Down

0 comments on commit 24c4ef8

Please sign in to comment.