Reduce graph adapter & http wrapper test runtime (#5002)

<!-- PR description-->
Another retry related optimization. This PR reduces `TestGraphIntgSuite/TestAdapterWrap_retriesConnectionClose` runtime from 150sec to 0.7sec.


---

#### Does this PR need a docs update or release note?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Abhishek Pandey 2024-01-10 16:25:44 -08:00 committed by GitHub
parent 72cfd3c4e5
commit 26e851ed01
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 20 deletions

View File

@ -17,6 +17,14 @@ import (
"github.com/alcionai/corso/src/pkg/logger"
)
// ---------------------------------------------------------------------------
// HTTP wrapper config
// ---------------------------------------------------------------------------
const (
httpWrapperRetryDelay = 3 * time.Second
)
// ---------------------------------------------------------------------------
// constructors
// ---------------------------------------------------------------------------
@ -60,7 +68,11 @@ func NewHTTPWrapper(
cc.apply(hc)
return &httpWrapper{hc, cc}
return &httpWrapper{
client: hc,
config: cc,
retryDelay: httpWrapperRetryDelay,
}
}
// NewNoTimeoutHTTPWrapper constructs a http wrapper with no context timeout.
@ -114,7 +126,7 @@ func (hw httpWrapper) Request(
// a common expectation.
for i := 0; i < hw.config.maxConnectionRetries+1; i++ {
if i > 0 {
time.Sleep(3 * time.Second)
time.Sleep(hw.retryDelay)
}
ctx = clues.Add(ctx, "request_retry_iter", i)
@ -158,6 +170,7 @@ type (
httpWrapper struct {
client *http.Client
config *clientConfig
retryDelay time.Duration
}
customTransport struct {

View File

@ -49,6 +49,9 @@ func (suite *HTTPWrapperIntgSuite) TestNewHTTPWrapper() {
require.NotNil(t, resp)
require.Equal(t, http.StatusOK, resp.StatusCode)
// Test http wrapper config
assert.Equal(t, httpWrapperRetryDelay, hw.retryDelay)
}
type mwForceResp struct {
@ -180,6 +183,11 @@ func (suite *HTTPWrapperUnitSuite) TestNewHTTPWrapper_http2StreamErrorRetries()
appendMiddleware(&mwResp),
MaxConnectionRetries(test.retries))
// Configure retry delay to reduce test time. Retry delay doesn't
// really matter here since all requests will be intercepted by
// the test middleware.
hw.retryDelay = 0
_, err := hw.Request(ctx, http.MethodGet, url, nil, nil)
require.ErrorAs(t, err, &http2.StreamError{}, clues.ToCore(err))
require.Equal(t, test.expectRetries, tries, "count of retries")

View File

@ -226,7 +226,6 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesConnectionClose() {
ctx, flush := tester.NewContext(t)
defer flush()
url := "https://graph.microsoft.com/fnords/beaux/regard"
retryInc := 0
// the panics should get caught and returned as errors
@ -243,24 +242,21 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesConnectionClose() {
suite.credentials.AzureClientID,
suite.credentials.AzureClientSecret,
count.New(),
appendMiddleware(&alwaysECONNRESET))
appendMiddleware(&alwaysECONNRESET),
// Configure retry middlewares so that they don't retry on connection reset.
// Those middlewares have their own tests to verify retries.
MaxRetries(-1))
require.NoError(t, err, clues.ToCore(err))
// Set retry delay to a low value to reduce test runtime.
// Retry delay doesn't really matter here since all requests will be intercepted
// by the test middleware. Set it to 0 to reduce test runtime.
aw := adpt.(*adapterWrap)
aw.retryDelay = 10 * time.Millisecond
// the query doesn't matter
_, err = users.NewItemCalendarsItemEventsDeltaRequestBuilder(url, adpt).Get(ctx, nil)
require.ErrorIs(t, err, syscall.ECONNRESET, clues.ToCore(err))
require.Equal(t, 16, retryInc, "number of retries")
retryInc = 0
aw.retryDelay = 0
// the query doesn't matter
_, err = NewService(adpt).Client().Users().Get(ctx, nil)
require.ErrorIs(t, err, syscall.ECONNRESET, clues.ToCore(err))
require.Equal(t, 16, retryInc, "number of retries")
require.Equal(t, 4, retryInc, "number of retries")
}
func (suite *GraphIntgSuite) TestAdapterWrap_retriesBadJWTToken() {
@ -307,9 +303,10 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesBadJWTToken() {
appendMiddleware(&alwaysBadJWT))
require.NoError(t, err, clues.ToCore(err))
// Set retry delay to a low value to reduce test runtime.
// Retry delay doesn't really matter here since all requests will be intercepted
// by the test middleware. Set it to 0 to reduce test runtime.
aw := adpt.(*adapterWrap)
aw.retryDelay = 10 * time.Millisecond
aw.retryDelay = 0
// When run locally this may fail. Not sure why it works in github but not locally.
// Pester keepers if it bothers you.
@ -384,9 +381,10 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesInvalidRequest() {
appendMiddleware(&returnsGraphResp))
require.NoError(t, err, clues.ToCore(err))
// Set retry delay to a low value to reduce test runtime.
// Retry delay doesn't really matter here since all requests will be intercepted
// by the test middleware. Set it to 0 to reduce test runtime.
aw := adpt.(*adapterWrap)
aw.retryDelay = 10 * time.Millisecond
aw.retryDelay = 0
table := []struct {
name string