Fix some misbehaving tests (Azure#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

@@ -6,7 +6,11 @@
         },
         {
             "Name": "azcore",
-            "CoverageGoal": 0.68
+            "CoverageGoal": 0.85
+        },
+        {
+            "Name": "internal",
+            "CoverageGoal": 0.90
         }
     ]
-}
+}

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
 

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

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

sdk/internal/ci.yml

@@ -15,3 +15,4 @@ stages:
 - template: ../../eng/pipelines/templates/jobs/archetype-sdk-client.yml
   parameters:
     ServiceDirectory: 'internal'
+    RunTests: true

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) {

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")
+	}
+}

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()

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)
+			}
+		})
+	}
+}