From 7ebbd79ef53ab26f85e39637afb7b1b0628314b0 Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Wed, 1 Dec 2021 13:52:12 -0500 Subject: [PATCH 1/6] adding variables API --- sdk/internal/recording/recording.go | 35 ++++++++++++++- sdk/internal/recording/recording_test.go | 55 ++++++++++++++++++------ 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/sdk/internal/recording/recording.go b/sdk/internal/recording/recording.go index 418211de1c3b..adc8e7b19a4b 100644 --- a/sdk/internal/recording/recording.go +++ b/sdk/internal/recording/recording.go @@ -10,6 +10,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/json" "errors" "fmt" "io/ioutil" @@ -492,6 +493,7 @@ const ( type recordedTest struct { recordingId string liveOnly bool + variables map[string]interface{} } var testSuite = map[string]recordedTest{} @@ -505,6 +507,7 @@ var client = http.Client{ type RecordingOptions struct { UseHTTPS bool GroupForReplace string + Variables map[string]interface{} } func defaultOptions() *RecordingOptions { @@ -589,7 +592,21 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err val.recordingId = recId testSuite[t.Name()] = val } else { - testSuite[t.Name()] = recordedTest{recordingId: recId, liveOnly: false} + var m map[string]interface{} + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + err = json.Unmarshal(body, &m) + if err != nil { + return err + } + + testSuite[t.Name()] = recordedTest{ + recordingId: recId, + liveOnly: false, + variables: m, + } } return nil } @@ -614,6 +631,15 @@ func Stop(t *testing.T, options *RecordingOptions) error { if err != nil { return err } + if options.Variables != nil { + req.Header.Set("Content-Type", "application/json") + marshalled, err := json.Marshal(options.Variables) + if err != nil { + return err + } + req.Body = ioutil.NopCloser(bytes.NewReader(marshalled)) + } + var recTest recordedTest var ok bool if recTest, ok = testSuite[t.Name()]; !ok { @@ -721,3 +747,10 @@ func IsLiveOnly(t *testing.T) bool { } return false } + +func GetVariables(t *testing.T) map[string]interface{} { + if s, ok := testSuite[t.Name()]; ok { + return s.variables + } + return nil +} diff --git a/sdk/internal/recording/recording_test.go b/sdk/internal/recording/recording_test.go index b75f577b0599..11348812bbb7 100644 --- a/sdk/internal/recording/recording_test.go +++ b/sdk/internal/recording/recording_test.go @@ -569,22 +569,51 @@ func TestFindProxyCertLocation(t *testing.T) { require.Contains(t, location, filepath.Join("eng", "common", "testproxy", "dotnet-devcert.crt")) } -func TestModeNotSet(t *testing.T) { - proxyMode, _ := os.LookupEnv("AZURE_RECORD_MODE") - defer os.Setenv("AZURE_RECORD_MODE", proxyMode) +func TestVariables(t *testing.T) { + temp := recordMode + recordMode = RecordingMode + defer func() { recordMode = temp }() - os.Unsetenv("AZURE_RECORD_MODE") - require.Equal(t, PlaybackMode, GetRecordMode()) -} + err := Start(t, packagePath, nil) + require.NoError(t, err) -func TestModeNotSetStartStop(t *testing.T) { - proxyMode, _ := os.LookupEnv("AZURE_RECORD_MODE") - defer os.Setenv("AZURE_RECORD_MODE", proxyMode) + client, err := NewRecordingHTTPClient(t, nil) + require.NoError(t, err) - os.Unsetenv("AZURE_RECORD_MODE") - err := Start(t, packagePath, nil) + req, err := http.NewRequest("POST", "https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags", nil) require.NoError(t, err) - require.Equal(t, PlaybackMode, GetRecordMode()) - err = Stop(t, nil) + + resp, err := client.Do(req) + require.NoError(t, err) + require.NotNil(t, resp) + + require.NotNil(t, GetRecordingId(t)) + + err = Stop(t, &RecordingOptions{Variables: map[string]interface{}{"key1": "value1", "key2": 1}}) require.NoError(t, err) + + err = Start(t, packagePath, nil) + require.NoError(t, err) + + variables := GetVariables(t) + require.Equal(t, variables["key1"], "value1") + require.Equal(t, variables["key2"], 1) + + // // Make sure the file is there + // jsonFile, err := os.Open(fmt.Sprintf("./testdata/recordings/%s.json", t.Name())) + // require.NoError(t, err) + // defer func() { + // err = jsonFile.Close() + // require.NoError(t, err) + // err = os.Remove(jsonFile.Name()) + // require.NoError(t, err) + // }() + + // var data RecordingFileStruct + // byteValue, err := ioutil.ReadAll(jsonFile) + // require.NoError(t, err) + // err = json.Unmarshal(byteValue, &data) + // require.NoError(t, err) + // require.Equal(t, "https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags", data.Entries[0].RequestURI) + // require.Equal(t, req.URL.String(), "https://localhost:5001/acr/v1/some_registry/_tags") } From d0eae3f8b87a46cb1b5b3975141fe948e840de5d Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Wed, 1 Dec 2021 16:14:08 -0500 Subject: [PATCH 2/6] update changelog, fixing body --- sdk/internal/CHANGELOG.md | 4 +++ sdk/internal/recording/recording.go | 35 ++++++++++++++++-------- sdk/internal/recording/recording_test.go | 34 ++++++++++------------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/sdk/internal/CHANGELOG.md b/sdk/internal/CHANGELOG.md index 51d757092dd8..c4c391a407d9 100644 --- a/sdk/internal/CHANGELOG.md +++ b/sdk/internal/CHANGELOG.md @@ -1,5 +1,9 @@ # Release History +## 0.8.4 (Unreleased) +### Features Added +* Added variables storage to the `Stop` function. Pass in a `map[string]interface{}` to the `Stop` method options and the values can be retrieved with the `GetVariables(t *testing.T)` function + ## 0.8.3 (2021-11-30) ### Features Added diff --git a/sdk/internal/recording/recording.go b/sdk/internal/recording/recording.go index adc8e7b19a4b..a3d3392cdbfb 100644 --- a/sdk/internal/recording/recording.go +++ b/sdk/internal/recording/recording.go @@ -588,20 +588,25 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err } return fmt.Errorf("Recording ID was not returned by the response. Response body: %s", b) } - if val, ok := testSuite[t.Name()]; ok { - val.recordingId = recId - testSuite[t.Name()] = val - } else { - var m map[string]interface{} - body, err := ioutil.ReadAll(resp.Body) - if err != nil { - return err - } + + // Unmarshal any variables returned by the proxy + var m map[string]interface{} + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return err + } + if len(body) > 0 { err = json.Unmarshal(body, &m) if err != nil { return err } + } + if val, ok := testSuite[t.Name()]; ok { + val.recordingId = recId + val.variables = m + testSuite[t.Name()] = val + } else { testSuite[t.Name()] = recordedTest{ recordingId: recId, liveOnly: false, @@ -631,13 +636,14 @@ func Stop(t *testing.T, options *RecordingOptions) error { if err != nil { return err } - if options.Variables != nil { + if options.Variables != nil && len(options.Variables) > 0 { req.Header.Set("Content-Type", "application/json") marshalled, err := json.Marshal(options.Variables) if err != nil { return err } req.Body = ioutil.NopCloser(bytes.NewReader(marshalled)) + req.ContentLength = int64(len(marshalled)) } var recTest recordedTest @@ -646,7 +652,14 @@ func Stop(t *testing.T, options *RecordingOptions) error { return errors.New("Recording ID was never set. Did you call StartRecording?") } req.Header.Set("x-recording-id", recTest.recordingId) - _, err = client.Do(req) + resp, err := client.Do(req) + if resp.StatusCode != 200 { + b, err := ioutil.ReadAll(resp.Body) + if err == nil { + return fmt.Errorf("proxy did not stop the recording properly: %s", string(b)) + } + return errors.New("proxy did not stop the recording properly") + } return err } diff --git a/sdk/internal/recording/recording_test.go b/sdk/internal/recording/recording_test.go index 11348812bbb7..d4b53bf0f746 100644 --- a/sdk/internal/recording/recording_test.go +++ b/sdk/internal/recording/recording_test.go @@ -589,31 +589,25 @@ func TestVariables(t *testing.T) { require.NotNil(t, GetRecordingId(t)) - err = Stop(t, &RecordingOptions{Variables: map[string]interface{}{"key1": "value1", "key2": 1}}) + err = Stop(t, &RecordingOptions{Variables: map[string]interface{}{"key1": "value1", "key2": "1"}}) require.NoError(t, err) + recordMode = PlaybackMode err = Start(t, packagePath, nil) + defer Stop(t, nil) require.NoError(t, err) variables := GetVariables(t) require.Equal(t, variables["key1"], "value1") - require.Equal(t, variables["key2"], 1) - - // // Make sure the file is there - // jsonFile, err := os.Open(fmt.Sprintf("./testdata/recordings/%s.json", t.Name())) - // require.NoError(t, err) - // defer func() { - // err = jsonFile.Close() - // require.NoError(t, err) - // err = os.Remove(jsonFile.Name()) - // require.NoError(t, err) - // }() - - // var data RecordingFileStruct - // byteValue, err := ioutil.ReadAll(jsonFile) - // require.NoError(t, err) - // err = json.Unmarshal(byteValue, &data) - // require.NoError(t, err) - // require.Equal(t, "https://azsdkengsys.azurecr.io/acr/v1/some_registry/_tags", data.Entries[0].RequestURI) - // require.Equal(t, req.URL.String(), "https://localhost:5001/acr/v1/some_registry/_tags") + require.Equal(t, variables["key2"], "1") + + // Make sure the file is there + jsonFile, err := os.Open(fmt.Sprintf("./testdata/recordings/%s.json", t.Name())) + require.NoError(t, err) + defer func() { + err = jsonFile.Close() + require.NoError(t, err) + err = os.Remove(jsonFile.Name()) + require.NoError(t, err) + }() } From 7186535225230a66214053f6d138ae8c1b88dd9b Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Wed, 1 Dec 2021 16:25:26 -0500 Subject: [PATCH 3/6] checking error value on stop --- sdk/internal/recording/recording_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/internal/recording/recording_test.go b/sdk/internal/recording/recording_test.go index d4b53bf0f746..ea75e52dff38 100644 --- a/sdk/internal/recording/recording_test.go +++ b/sdk/internal/recording/recording_test.go @@ -594,13 +594,15 @@ func TestVariables(t *testing.T) { recordMode = PlaybackMode err = Start(t, packagePath, nil) - defer Stop(t, nil) require.NoError(t, err) variables := GetVariables(t) require.Equal(t, variables["key1"], "value1") require.Equal(t, variables["key2"], "1") + err = Stop(t, nil) + require.NoError(t, err) + // Make sure the file is there jsonFile, err := os.Open(fmt.Sprintf("./testdata/recordings/%s.json", t.Name())) require.NoError(t, err) From e481ee3c85e8683ed54a470c7e9fb09d54e4c708 Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Wed, 1 Dec 2021 17:03:10 -0500 Subject: [PATCH 4/6] adding Close to response bodies, doc comments --- sdk/internal/recording/recording.go | 9 ++++++++- sdk/internal/recording/sanitizer.go | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/internal/recording/recording.go b/sdk/internal/recording/recording.go index a3d3392cdbfb..53b07e939540 100644 --- a/sdk/internal/recording/recording.go +++ b/sdk/internal/recording/recording.go @@ -551,6 +551,7 @@ func getTestId(pathToRecordings string, t *testing.T) string { return path.Join(pathToRecordings, "recordings", t.Name()+".json") } +// Start tells the test proxy to begin accepting requests for a given test func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) error { if options == nil { options = defaultOptions() @@ -583,6 +584,7 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err recId := resp.Header.Get(IDHeader) if recId == "" { b, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err != nil { return err } @@ -592,6 +594,7 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err // Unmarshal any variables returned by the proxy var m map[string]interface{} body, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err != nil { return err } @@ -616,6 +619,7 @@ func Start(t *testing.T, pathToRecordings string, options *RecordingOptions) err return nil } +// Stop tells the test proxy to stop accepting requests for a given test func Stop(t *testing.T, options *RecordingOptions) error { if options == nil { options = defaultOptions() @@ -636,7 +640,7 @@ func Stop(t *testing.T, options *RecordingOptions) error { if err != nil { return err } - if options.Variables != nil && len(options.Variables) > 0 { + if len(options.Variables) > 0 { req.Header.Set("Content-Type", "application/json") marshalled, err := json.Marshal(options.Variables) if err != nil { @@ -655,6 +659,7 @@ func Stop(t *testing.T, options *RecordingOptions) error { resp, err := client.Do(req) if resp.StatusCode != 200 { b, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err == nil { return fmt.Errorf("proxy did not stop the recording properly: %s", string(b)) } @@ -726,6 +731,7 @@ func (c RecordingHTTPClient) Do(req *http.Request) (*http.Response, error) { return c.defaultClient.Do(req) } +// NewRecordingHTTPClient returns a type that implements `azcore.Transporter`. This will automatically route tests on the `Do` call. func NewRecordingHTTPClient(t *testing.T, options *RecordingOptions) (*RecordingHTTPClient, error) { if options == nil { options = &RecordingOptions{UseHTTPS: true} @@ -761,6 +767,7 @@ func IsLiveOnly(t *testing.T) bool { return false } +// GetVariables returns access to the variables stored by the test proxy for a specific test func GetVariables(t *testing.T) map[string]interface{} { if s, ok := testSuite[t.Name()]; ok { return s.variables diff --git a/sdk/internal/recording/sanitizer.go b/sdk/internal/recording/sanitizer.go index 2074726b691b..b765a36aa18d 100644 --- a/sdk/internal/recording/sanitizer.go +++ b/sdk/internal/recording/sanitizer.go @@ -102,6 +102,7 @@ func handleProxyResponse(resp *http.Response, err error) error { } body, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() if err != nil { return err } From 46f5e58762a97d39ea65429ce7db782e26c89d4c Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Wed, 1 Dec 2021 17:05:49 -0500 Subject: [PATCH 5/6] fixing close on Stop method --- sdk/internal/recording/recording.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/internal/recording/recording.go b/sdk/internal/recording/recording.go index 53b07e939540..b8e8363cf2c2 100644 --- a/sdk/internal/recording/recording.go +++ b/sdk/internal/recording/recording.go @@ -663,8 +663,9 @@ func Stop(t *testing.T, options *RecordingOptions) error { if err == nil { return fmt.Errorf("proxy did not stop the recording properly: %s", string(b)) } - return errors.New("proxy did not stop the recording properly") + return fmt.Errorf("proxy did not stop the recording properly: %s", err.Error()) } + _ = resp.Body.Close() return err } From 1ea7f57ee41383fd1914ee5a208ace694bb60d99 Mon Sep 17 00:00:00 2001 From: seankane-msft Date: Tue, 7 Dec 2021 10:20:47 -0500 Subject: [PATCH 6/6] updating for release --- sdk/internal/CHANGELOG.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sdk/internal/CHANGELOG.md b/sdk/internal/CHANGELOG.md index c4c391a407d9..945e6a76e902 100644 --- a/sdk/internal/CHANGELOG.md +++ b/sdk/internal/CHANGELOG.md @@ -1,16 +1,13 @@ # Release History -## 0.8.4 (Unreleased) -### Features Added -* Added variables storage to the `Stop` function. Pass in a `map[string]interface{}` to the `Stop` method options and the values can be retrieved with the `GetVariables(t *testing.T)` function - -## 0.8.3 (2021-11-30) +## 0.8.3 (2021-12-07) ### Features Added * If `AZURE_RECORD_MODE` is not set, default to `playback` * If `PROXY_CERT` is not set, try to find it based on the `GOPATH` environment variable and the path to `eng/common/testproxy/dotnet-devcert.crt` * Adds `NewRecordingHTTPClient()` method which returns an `azcore.Transporter` interface that routes requests to the test proxy [#16221](https://github.com/Azure/azure-sdk-for-go/pull/16221). * Adds the `SetBodilessMatcher` method [#16256](https://github.com/Azure/azure-sdk-for-go/pull/16256) +* Added variables storage to the `Stop` function. Pass in a `map[string]interface{}` to the `Stop` method options and the values can be retrieved with the `GetVariables(t *testing.T)` function [#16375](https://github.com/Azure/azure-sdk-for-go/pull/16375). ### Breaking Changes * Renames `ResetSanitizers` to `ResetProxy` [#16256](https://github.com/Azure/azure-sdk-for-go/pull/16256)