From 952a18f639eeef9f9d85506c5dc23be9549244f2 Mon Sep 17 00:00:00 2001
From: Igor Drozdov <idrozdov@gitlab.com>
Date: Wed, 25 Jan 2023 19:03:45 +0100
Subject: Stub retryable http values in tests

Currently, the default values are used for retryable http.
That's why a test waits 1 second minimun to retry a request.
Client test takes 25 seconds to execute as a result.
When we stub the value to 1 millisecond instead, we get 0.5s of
execution
---
 client/client_test.go      | 11 +++++++----
 client/httpclient.go       | 43 ++++++++++++++++++++++++++++++-------------
 client/httpsclient_test.go |  2 +-
 3 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/client/client_test.go b/client/client_test.go
index 9bea4fd..e48bb67 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -20,7 +20,10 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/testhelper"
 )
 
-var secret = "sssh, it's a secret"
+var (
+	secret          = "sssh, it's a secret"
+	defaultHttpOpts = []HTTPClientOpt{WithHTTPRetryOpts(time.Millisecond, time.Millisecond, 2)}
+)
 
 func TestClients(t *testing.T) {
 	testhelper.PrepareTestRootDir(t)
@@ -81,7 +84,7 @@ func TestClients(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			url := tc.server(t, buildRequests(t, tc.relativeURLRoot))
 
-			httpClient, err := NewHTTPClientWithOpts(url, tc.relativeURLRoot, tc.caFile, "", 1, nil)
+			httpClient, err := NewHTTPClientWithOpts(url, tc.relativeURLRoot, tc.caFile, "", 1, defaultHttpOpts)
 			require.NoError(t, err)
 
 			client, err := NewGitlabNetClient("", "", tc.secret, httpClient)
@@ -313,7 +316,7 @@ func TestRetryableHTTPFeatureToggle(t *testing.T) {
 		}))
 		defer srv.Close()
 
-		httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, nil)
+		httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
 		require.NoError(t, err)
 		require.NotNil(t, httpClient.HTTPClient)
 		require.Nil(t, httpClient.RetryableHTTP)
@@ -334,7 +337,7 @@ func TestRetryableHTTPFeatureToggle(t *testing.T) {
 		}))
 		defer srv.Close()
 
-		httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, nil)
+		httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
 		require.NoError(t, err)
 		require.Nil(t, httpClient.HTTPClient)
 		require.NotNil(t, httpClient.RetryableHTTP)
diff --git a/client/httpclient.go b/client/httpclient.go
index fe6e2e5..82b4b40 100644
--- a/client/httpclient.go
+++ b/client/httpclient.go
@@ -24,6 +24,9 @@ const (
 	httpProtocol              = "http://"
 	httpsProtocol             = "https://"
 	defaultReadTimeoutSeconds = 300
+	defaultRetryWaitMin       = time.Second
+	defaultRetryWaitMax       = 15 * time.Second
+	defaultRetryMax           = 2
 )
 
 var ErrCafileNotFound = errors.New("cafile not found")
@@ -35,8 +38,10 @@ type HttpClient struct {
 }
 
 type httpClientCfg struct {
-	keyPath, certPath string
-	caFile, caPath    string
+	keyPath, certPath          string
+	caFile, caPath             string
+	retryWaitMin, retryWaitMax time.Duration
+	retryMax                   int
 }
 
 func (hcc httpClientCfg) HaveCertAndKey() bool { return hcc.keyPath != "" && hcc.certPath != "" }
@@ -53,6 +58,14 @@ func WithClientCert(certPath, keyPath string) HTTPClientOpt {
 	}
 }
 
+func WithHTTPRetryOpts(waitMin, waitMax time.Duration, maxAttempts int) HTTPClientOpt {
+	return func(hcc *httpClientCfg) {
+		hcc.retryWaitMin = waitMin
+		hcc.retryWaitMax = waitMax
+		hcc.retryMax = maxAttempts
+	}
+}
+
 func validateCaFile(filename string) error {
 	if filename == "" {
 		return nil
@@ -71,6 +84,18 @@ func validateCaFile(filename string) error {
 
 // NewHTTPClientWithOpts builds an HTTP client using the provided options
 func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, readTimeoutSeconds uint64, opts []HTTPClientOpt) (*HttpClient, error) {
+	hcc := &httpClientCfg{
+		caFile:       caFile,
+		caPath:       caPath,
+		retryWaitMin: defaultRetryWaitMin,
+		retryWaitMax: defaultRetryWaitMax,
+		retryMax:     defaultRetryMax,
+	}
+
+	for _, opt := range opts {
+		opt(hcc)
+	}
+
 	var transport *http.Transport
 	var host string
 	var err error
@@ -84,15 +109,6 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri
 			return nil, err
 		}
 
-		hcc := &httpClientCfg{
-			caFile: caFile,
-			caPath: caPath,
-		}
-
-		for _, opt := range opts {
-			opt(hcc)
-		}
-
 		transport, host, err = buildHttpsTransport(*hcc, gitlabURL)
 		if err != nil {
 			return nil, err
@@ -110,8 +126,9 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri
 
 	if os.Getenv("FF_GITLAB_SHELL_RETRYABLE_HTTP") == "1" {
 		c := retryablehttp.NewClient()
-		c.RetryMax = 2
-		c.RetryWaitMax = 15 * time.Second
+		c.RetryMax = hcc.retryMax
+		c.RetryWaitMax = hcc.retryWaitMax
+		c.RetryWaitMin = hcc.retryWaitMin
 		c.Logger = nil
 		c.HTTPClient.Transport = correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(transport))
 		c.HTTPClient.Timeout = readTimeout(readTimeoutSeconds)
diff --git a/client/httpsclient_test.go b/client/httpsclient_test.go
index d220171..49152de 100644
--- a/client/httpsclient_test.go
+++ b/client/httpsclient_test.go
@@ -123,7 +123,7 @@ func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPat
 
 	url := testserver.StartHttpsServer(t, requests, clientCAPath)
 
-	var opts []HTTPClientOpt
+	opts := defaultHttpOpts
 	if clientCertPath != "" && clientKeyPath != "" {
 		opts = append(opts, WithClientCert(clientCertPath, clientKeyPath))
 	}
-- 
cgit v1.2.1