From 5184fad18c4b7ba72e0b1b314d2f0caf3f2b9867 Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Wed, 29 Nov 2023 17:21:08 -0800 Subject: [PATCH] Minor retry handler test refactor (#4752) Switch to configurable retries/timeouts/backoffs. Set up PR for https://github.com/alcionai/corso/pull/4750/files testing. With configurable retries, we can cut down the `TestRetryMiddleware_RetryResponse_maintainBodyAfter503` runtime from ~40seconds to around 1-2 seconds in the following PR #4750 --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../m365/api/graph/middleware_test.go | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/pkg/services/m365/api/graph/middleware_test.go b/src/pkg/services/m365/api/graph/middleware_test.go index 98e3f6494..b943dc9f7 100644 --- a/src/pkg/services/m365/api/graph/middleware_test.go +++ b/src/pkg/services/m365/api/graph/middleware_test.go @@ -98,7 +98,7 @@ func (mw *testMW) Intercept( func mockAdapter( creds account.M365Config, mw khttp.Middleware, - timeout time.Duration, + cc *clientConfig, ) (*msgraphsdkgo.GraphRequestAdapter, error) { auth, err := GetAuth( creds.AzureTenantID, @@ -110,13 +110,10 @@ func mockAdapter( var ( clientOptions = msgraphsdkgo.GetDefaultClientOptions() - cc = populateConfig(MinimumBackoff(10 * time.Millisecond)) middlewares = append(kiotaMiddlewares(&clientOptions, cc, count.New()), mw) httpClient = msgraphgocore.GetDefaultClient(&clientOptions, middlewares...) ) - httpClient.Timeout = timeout - cc.apply(httpClient) return msgraphsdkgo.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient( @@ -146,6 +143,11 @@ func (suite *RetryMWIntgSuite) SetupSuite() { err error ) + ctx, flush := tester.NewContext(suite.T()) + defer flush() + + InitializeConcurrencyLimiter(ctx, false, -1) + suite.creds, err = a.M365Config() require.NoError(suite.T(), err, clues.ToCore(err)) } @@ -239,7 +241,12 @@ func (suite *RetryMWIntgSuite) TestRetryMiddleware_Intercept_byStatusCode() { newMWReturns(test.status, nil, test.providedErr)) mw.repeatReturn0 = true - adpt, err := mockAdapter(suite.creds, mw, 25*time.Second) + cc := populateConfig( + MinimumBackoff(10*time.Millisecond), + Timeout(25*time.Second), + MaxRetries(test.expectRetryCount)) + + adpt, err := mockAdapter(suite.creds, mw, cc) require.NoError(t, err, clues.ToCore(err)) // url doesn't fit the builder, but that shouldn't matter @@ -283,7 +290,11 @@ func (suite *RetryMWIntgSuite) TestRetryMiddleware_RetryRequest_resetBodyAfter50 newMWReturns(http.StatusInternalServerError, nil, nil), newMWReturns(http.StatusOK, nil, nil)) - adpt, err := mockAdapter(suite.creds, mw, 15*time.Second) + cc := populateConfig( + MinimumBackoff(10*time.Millisecond), + Timeout(15*time.Second)) + + adpt, err := mockAdapter(suite.creds, mw, cc) require.NoError(t, err, clues.ToCore(err)) // no api package needed here, this is a mocked request that works @@ -303,8 +314,6 @@ func (suite *RetryMWIntgSuite) TestRetryMiddleware_RetryResponse_maintainBodyAft ctx, flush := tester.NewContext(t) defer flush() - InitializeConcurrencyLimiter(ctx, false, -1) - odem := graphTD.ODataErrWithMsg("SystemDown", "The System, Is Down, bah-dup-da-woo-woo!") m := graphTD.ParseableToMap(t, odem) @@ -319,7 +328,11 @@ func (suite *RetryMWIntgSuite) TestRetryMiddleware_RetryResponse_maintainBodyAft newMWReturns(http.StatusServiceUnavailable, body, nil), newMWReturns(http.StatusServiceUnavailable, body, nil)) - adpt, err := mockAdapter(suite.creds, mw, 55*time.Second) + cc := populateConfig( + MinimumBackoff(10*time.Millisecond), + Timeout(55*time.Second)) + + adpt, err := mockAdapter(suite.creds, mw, cc) require.NoError(t, err, clues.ToCore(err)) // no api package needed here,