From 1eb47c16ddfd8feeb2cb4ff1698e1a3f3e1765ab Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Wed, 18 Aug 2021 07:59:47 -0700 Subject: [PATCH] Fix some misbehaving tests (#15309) * Fix some misbehaving tests Enable tests in CI * add missing tests and lower CC threshold * add a bit more coverage * increase code coverage goal for azcore added move coverage to azcore and fixed bug in retry policy. fixed test publishing to include sub-directories. * add missing EOL --- eng/config.json | 8 ++++++-- eng/pipelines/templates/steps/build-test.yml | 2 +- sdk/azcore/policy_retry.go | 2 +- sdk/azcore/poller_test.go | 2 +- sdk/internal/ci.yml | 1 + sdk/internal/log/log.go | 5 +++++ sdk/internal/log/log_test.go | 10 ++++++++++ sdk/internal/recording/recording_test.go | 5 ++--- sdk/internal/uuid/uuid_test.go | 15 +++++++++++++++ 9 files changed, 42 insertions(+), 8 deletions(-) diff --git a/eng/config.json b/eng/config.json index d3a9dbdc69cd..711b2f4c3575 100644 --- a/eng/config.json +++ b/eng/config.json @@ -6,7 +6,11 @@ }, { "Name": "azcore", - "CoverageGoal": 0.68 + "CoverageGoal": 0.85 + }, + { + "Name": "internal", + "CoverageGoal": 0.90 } ] -} \ No newline at end of file +} diff --git a/eng/pipelines/templates/steps/build-test.yml b/eng/pipelines/templates/steps/build-test.yml index 6a79c86628e2..3fb124547094 100644 --- a/eng/pipelines/templates/steps/build-test.yml +++ b/eng/pipelines/templates/steps/build-test.yml @@ -57,7 +57,7 @@ steps: condition: succeededOrFailed() inputs: testRunner: JUnit - testResultsFiles: '${{parameters.GoWorkspace}}sdk/${{parameters.ServiceDirectory}}/report.xml' + testResultsFiles: '${{parameters.GoWorkspace}}sdk/${{parameters.ServiceDirectory}}/**/report.xml' testRunTitle: 'Go ${{ parameters.GoVersion }} on ${{ parameters.Image }}' failTaskOnFailedTests: true diff --git a/sdk/azcore/policy_retry.go b/sdk/azcore/policy_retry.go index 24fb07dce08f..b916f28fbadd 100644 --- a/sdk/azcore/policy_retry.go +++ b/sdk/azcore/policy_retry.go @@ -136,7 +136,7 @@ func (p *retryPolicy) Do(req *Request) (resp *http.Response, err error) { if req.body != nil { // wrap the body so we control when it's actually closed rwbody := &retryableRequestBody{body: req.body} - req.Body = rwbody + req.body = rwbody req.Request.GetBody = func() (io.ReadCloser, error) { _, err := rwbody.Seek(0, io.SeekStart) // Seek back to the beginning of the stream return rwbody, err diff --git a/sdk/azcore/poller_test.go b/sdk/azcore/poller_test.go index ef1ef814fcf2..36da6e548048 100644 --- a/sdk/azcore/poller_test.go +++ b/sdk/azcore/poller_test.go @@ -113,7 +113,7 @@ func TestOpPollerSimple(t *testing.T) { func TestOpPollerWithWidgetPUT(t *testing.T) { srv, close := mock.NewServer() - srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted), mock.WithBody([]byte(`{"status": "InProgress"}`))) + srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted), mock.WithBody([]byte(`{"status": "InProgress"}`)), mock.WithHeader("Retry-After", "1")) srv.AppendResponse(mock.WithStatusCode(http.StatusAccepted), mock.WithBody([]byte(`{"status": "InProgress"}`))) srv.AppendResponse(mock.WithStatusCode(http.StatusOK), mock.WithBody([]byte(`{"status": "Succeeded"}`))) // PUT and PATCH state that a final GET will happen diff --git a/sdk/internal/ci.yml b/sdk/internal/ci.yml index 5baaaff4f3b6..31ed8cc20a1a 100644 --- a/sdk/internal/ci.yml +++ b/sdk/internal/ci.yml @@ -15,3 +15,4 @@ stages: - template: ../../eng/pipelines/templates/jobs/archetype-sdk-client.yml parameters: ServiceDirectory: 'internal' + RunTests: true diff --git a/sdk/internal/log/log.go b/sdk/internal/log/log.go index cdd57980f2f7..2ac1dc3376db 100644 --- a/sdk/internal/log/log.go +++ b/sdk/internal/log/log.go @@ -97,6 +97,11 @@ func TestResetClassifications() { var log logger func init() { + initLogging() +} + +// split out for testing purposes +func initLogging() { if cls := os.Getenv("AZURE_SDK_GO_LOGGING"); cls == "all" { // cls could be enhanced to support a comma-delimited list of log classifications log.lst = func(cls Classification, msg string) { diff --git a/sdk/internal/log/log_test.go b/sdk/internal/log/log_test.go index d0391080ff23..8987acae7c33 100644 --- a/sdk/internal/log/log_test.go +++ b/sdk/internal/log/log_test.go @@ -6,6 +6,7 @@ package log import ( "fmt" "net/http" + "os" "testing" ) @@ -50,3 +51,12 @@ func TestLoggingClassification(t *testing.T) { t.Fatalf("unexpected log entry: %s", log[Request]) } } + +func TestEnvironment(t *testing.T) { + os.Setenv("AZURE_SDK_GO_LOGGING", "all") + defer os.Unsetenv("AZURE_SDK_GO_LOGGING") + initLogging() + if log.lst == nil { + t.Fatal("unexpected nil listener") + } +} diff --git a/sdk/internal/recording/recording_test.go b/sdk/internal/recording/recording_test.go index d1e83c54da4b..b7d39956bb18 100644 --- a/sdk/internal/recording/recording_test.go +++ b/sdk/internal/recording/recording_test.go @@ -74,14 +74,14 @@ func (s *recordingTests) TestRecordedVariables() { require.Equal(expectedVariableValue, target.GetOptionalEnvVar(nonExistingEnvVar, expectedVariableValue, NoSanitization)) // non existent variables return an error - val, err := target.GetEnvVar(nonExistingEnvVar, NoSanitization) + _, err = target.GetEnvVar(nonExistingEnvVar, NoSanitization) // mark test as succeeded require.Equal(envNotExistsError(nonExistingEnvVar), err.Error()) // now create the env variable and check that it can be fetched os.Setenv(nonExistingEnvVar, expectedVariableValue) defer os.Unsetenv(nonExistingEnvVar) - _, err = target.GetEnvVar(nonExistingEnvVar, NoSanitization) + val, err := target.GetEnvVar(nonExistingEnvVar, NoSanitization) require.NoError(err) require.Equal(expectedVariableValue, val) @@ -361,7 +361,6 @@ func (s *recordingTests) TestRecordRequestsAndFailMatchingForMissingRecording() // playback the request _, err = target.Do(req) - require.NoError(err) require.Equal(missingRequestError(req), err.Error()) // mark succeeded err = target.Stop() diff --git a/sdk/internal/uuid/uuid_test.go b/sdk/internal/uuid/uuid_test.go index 9e14d53c07f0..8d2fd8804107 100644 --- a/sdk/internal/uuid/uuid_test.go +++ b/sdk/internal/uuid/uuid_test.go @@ -54,3 +54,18 @@ func TestParse(t *testing.T) { }) } } + +func TestParseFail(t *testing.T) { + testCases := []string{ + "72d0f24f-82be-4016-729d-31fd13bd681", + "{72d0f24f-82be+4016-729d-31fd13bd681e}", + } + for _, input := range testCases { + t.Run(input, func(t *testing.T) { + _, err := Parse(input) + if err == nil { + t.Fatalf("unexpected nil error for: %s", input) + } + }) + } +}