Storage test enhancements (Azure#6918)

Storage test enhancements:

- Change test recordings to not require a local config
- Add an initial CONTRIBUTING.md
- Add a test to garbage collect any resources in our test tenants
- Add Storage specific record matching and update test recordings
- Change the diagnostic attribute name
- Use the same buffer size across platforms so recordings are in sync
- Separate excluded headers from volatile headers in recording matches
- Be a little more careful about waiting for progress events
- Fix Azure#6806 to remove excess serialization whitespace

sdk/appconfiguration/Azure.ApplicationModel.Configuration/tests/ConfigurationRecordMatcher.cs

@@ -20,7 +20,7 @@ public class ConfigurationRecordMatcher : RecordMatcher
 
         public ConfigurationRecordMatcher(RecordedTestSanitizer sanitizer) : base(sanitizer)
         {
-            ExcludeResponseHeaders.Add("Sync-Token");
+            VolatileResponseHeaders.Add("Sync-Token");
         }
 
         protected override bool IsBodyEquivalent(RecordEntry record, RecordEntry otherRecord)

sdk/core/Azure.Core/tests/TestFramework/DiagnosticScopeValidatingInterceptor.cs

@@ -24,7 +24,7 @@ public void Intercept(IInvocation invocation)
                 TestDiagnosticListener diagnosticListener = new TestDiagnosticListener("Azure.Clients");
                 invocation.Proceed();
 
-                bool strict = !invocation.Method.GetCustomAttributes(true).Any(a => a.GetType().FullName == "Azure.Core.ConvenienceMethodAttribute");
+                bool strict = !invocation.Method.GetCustomAttributes(true).Any(a => a.GetType().FullName == "Azure.Core.ForwardsClientCallsAttribute");
                 if (invocation.Method.ReturnType.Name.Contains("AsyncCollection") ||
                     invocation.Method.ReturnType.Name.Contains("IAsyncEnumerable"))
                 {

sdk/core/Azure.Core/tests/TestFramework/RecordEntry.cs

@@ -273,6 +273,7 @@ public void Sanitize(RecordedTestSanitizer sanitizer)
             RequestUri = sanitizer.SanitizeUri(RequestUri);
             if (RequestBody != null)
             {
+                int contentLength = RequestBody.Length;
                 TryGetContentType(RequestHeaders, out string contentType);
                 if (IsTextContentType(RequestHeaders, out Encoding encoding))
                 {
@@ -282,12 +283,14 @@ public void Sanitize(RecordedTestSanitizer sanitizer)
                 {
                     RequestBody = sanitizer.SanitizeBody(contentType, RequestBody);
                 }
+                UpdateSanitizedContentLength(RequestHeaders, contentLength, RequestBody?.Length ?? 0);
             }
 
             sanitizer.SanitizeHeaders(RequestHeaders);
 
             if (ResponseBody != null)
             {
+                int contentLength = ResponseBody.Length;
                 TryGetContentType(ResponseHeaders, out string contentType);
                 if (IsTextContentType(ResponseHeaders, out Encoding encoding))
                 {
@@ -297,9 +300,32 @@ public void Sanitize(RecordedTestSanitizer sanitizer)
                 {
                     ResponseBody = sanitizer.SanitizeBody(contentType, ResponseBody);
                 }
+                UpdateSanitizedContentLength(ResponseHeaders, contentLength, ResponseBody?.Length ?? 0);
             }
 
             sanitizer.SanitizeHeaders(ResponseHeaders);
         }
+
+        /// <summary>
+        /// Optionally update the Content-Length header if we've sanitized it
+        /// and the new value is a different length from the original
+        /// Content-Length header.  We don't add a Content-Length header if it
+        /// wasn't already present.
+        /// </summary>
+        /// <param name="headers">The Request or Response headers</param>
+        /// <param name="originalLength">THe original Content-Length</param>
+        /// <param name="sanitizedLength">The sanitized Content-Length</param>
+        private static void UpdateSanitizedContentLength(IDictionary<string, string[]> headers, int originalLength, int sanitizedLength)
+        {
+            // Note: If the RequestBody/ResponseBody was set to null by our
+            // sanitizer, we'll pass 0 as the sanitizedLength and use that as
+            // our new Content-Length.  That's fine for all current scenarios
+            // (i.e., we never do that), but it's possible we may want to
+            // remove the Content-Length header in the future.
+            if (originalLength != sanitizedLength && headers.ContainsKey("Content-Length"))
+            {
+                headers["Content-Length"] = new string[] { sanitizedLength.ToString() };
+            }
+        }
     }
 }

sdk/core/Azure.Core/tests/TestFramework/RecordMatcher.cs

@@ -29,7 +29,22 @@ public RecordMatcher(RecordedTestSanitizer sanitizer)
             "Request-Id"
         };
 
-        public HashSet<string> ExcludeResponseHeaders = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+        // Headers that don't indicate meaningful changes between updated recordings
+        public HashSet<string> VolatileHeaders = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
+        {
+            "Date",
+            "x-ms-date",
+            "x-ms-client-request-id",
+            "User-Agent",
+            "Request-Id",
+            "If-Match",
+            "If-None-Match",
+            "If-Modified-Since",
+            "If-Unmodified-Since"
+        };
+
+        // Headers that don't indicate meaningful changes between updated recordings
+        public HashSet<string> VolatileResponseHeaders = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
         {
             "Date",
             "ETag",
@@ -59,7 +74,8 @@ public virtual RecordEntry FindMatch(Request request, IList<RecordEntry> entries
             foreach (RecordEntry entry in entries)
             {
                 int score = 0;
-                if (entry.RequestUri != uri)
+
+                if (!AreUrisSame(entry.RequestUri, uri))
                 {
                     score++;
                 }
@@ -69,7 +85,7 @@ public virtual RecordEntry FindMatch(Request request, IList<RecordEntry> entries
                     score++;
                 }
 
-                score += CompareHeaderDictionaries(headers, entry.RequestHeaders);
+                score += CompareHeaderDictionaries(headers, entry.RequestHeaders, ExcludeHeaders);
 
                 if (score == 0)
                 {
@@ -83,14 +99,31 @@ public virtual RecordEntry FindMatch(Request request, IList<RecordEntry> entries
                 }
             }
 
-
             throw new InvalidOperationException(GenerateException(request.Method, uri, headers, bestScoreEntry));
         }
 
-        public virtual bool IsEquivalentResponse(RecordEntry entry, RecordEntry otherEntry)
+        public virtual bool IsEquivalentRecord(RecordEntry entry, RecordEntry otherEntry) =>
+            IsEquivalentRequest(entry, otherEntry) &&
+            IsEquivalentResponse(entry, otherEntry);
+
+        protected virtual bool IsEquivalentRequest(RecordEntry entry, RecordEntry otherEntry) =>
+            entry.RequestMethod == otherEntry.RequestMethod &&
+            IsEquivalentUri(entry.RequestUri, otherEntry.RequestUri) &&
+            CompareHeaderDictionaries(entry.RequestHeaders, otherEntry.RequestHeaders, VolatileHeaders) == 0;
+
+        private static bool AreUrisSame(string entryUri, string otherEntryUri) =>
+            // Some versions of .NET behave differently when calling new Uri("...")
+            // so we'll normalize the recordings (which may have been against
+            // a different .NET version) to be safe
+            new Uri(entryUri).ToString() == new Uri(otherEntryUri).ToString();
+
+        protected virtual bool IsEquivalentUri(string entryUri, string otherEntryUri) =>
+            AreUrisSame(entryUri, otherEntryUri);
+
+        protected virtual bool IsEquivalentResponse(RecordEntry entry, RecordEntry otherEntry)
         {
-            IEnumerable<KeyValuePair<string, string[]>> entryHeaders = entry.ResponseHeaders.Where(h => !ExcludeResponseHeaders.Contains(h.Key));
-            IEnumerable<KeyValuePair<string, string[]>> otherEntryHeaders = otherEntry.ResponseHeaders.Where(h => !ExcludeResponseHeaders.Contains(h.Key));
+            IEnumerable<KeyValuePair<string, string[]>> entryHeaders = entry.ResponseHeaders.Where(h => !VolatileResponseHeaders.Contains(h.Key));
+            IEnumerable<KeyValuePair<string, string[]>> otherEntryHeaders = otherEntry.ResponseHeaders.Where(h => !VolatileResponseHeaders.Contains(h.Key));
 
             return
                 entry.StatusCode == otherEntry.StatusCode &&
@@ -162,7 +195,7 @@ private string JoinHeaderValues(string[] values)
             return string.Join(",", values);
         }
 
-        private int CompareHeaderDictionaries(SortedDictionary<string, string[]> headers, SortedDictionary<string, string[]> entryHeaders)
+        private int CompareHeaderDictionaries(SortedDictionary<string, string[]> headers, SortedDictionary<string, string[]> entryHeaders, HashSet<string> ignoredHeaders)
         {
             int difference = 0;
             var remaining = new SortedDictionary<string, string[]>(entryHeaders, entryHeaders.Comparer);
@@ -171,13 +204,13 @@ private int CompareHeaderDictionaries(SortedDictionary<string, string[]> headers
                 if (remaining.TryGetValue(header.Key, out string[] values))
                 {
                     remaining.Remove(header.Key);
-                    if (!ExcludeHeaders.Contains(header.Key) &&
+                    if (!ignoredHeaders.Contains(header.Key) &&
                         !values.SequenceEqual(header.Value))
                     {
                         difference++;
                     }
                 }
-                else if (!ExcludeHeaders.Contains(header.Key))
+                else if (!ignoredHeaders.Contains(header.Key))
                 {
                     difference++;
                 }

sdk/core/Azure.Core/tests/TestFramework/RecordSession.cs

@@ -93,7 +93,11 @@ public bool IsEquivalent(RecordSession session, RecordMatcher matcher)
                 return false;
             }
 
-            return session.Variables.SequenceEqual(Variables) &&
+            // The DateTimeOffsetNow variable is updated any time it's used so
+            // we only care that both sessions use it or both sessions don't.
+            var now = TestRecording.DateTimeOffsetNowVariableKey;
+            return session.Variables.TryGetValue(now, out string _) == Variables.TryGetValue(now, out string _) &&
+                   session.Variables.Where(v => v.Key != now).SequenceEqual(Variables.Where(v => v.Key != now)) &&
                    session.Entries.SequenceEqual(Entries, new EntryEquivalentComparer(matcher));
         }
 
@@ -108,7 +112,7 @@ public EntryEquivalentComparer(RecordMatcher matcher)
 
             public bool Equals(RecordEntry x, RecordEntry y)
             {
-                return _matcher.IsEquivalentResponse(x, y);
+                return _matcher.IsEquivalentRecord(x, y);
             }
 
             public int GetHashCode(RecordEntry obj)

sdk/core/Azure.Core/tests/TestFramework/TestRecording.cs

@@ -13,7 +13,7 @@ namespace Azure.Core.Testing
     public class TestRecording : IDisposable
     {
         private const string RandomSeedVariableKey = "RandomSeed";
-        private const string DateTimeOffsetNowVariableKey = "DateTimeOffsetNow";
+        internal const string DateTimeOffsetNowVariableKey = "DateTimeOffsetNow";
 
         public TestRecording(RecordedTestMode mode, string sessionFile, RecordedTestSanitizer sanitizer, RecordMatcher matcher)
         {

sdk/storage/Azure.Storage.Blobs/README.md

@@ -168,6 +168,9 @@ Get started with our [Blob samples][samples]:
 
 ## Contributing
 
+See the [Storage CONTRIBUTING.md][storage_contrib] for details on building,
+testing, and contributing to this library.
+
 This project welcomes contributions and suggestions.  Most contributions require
 you to agree to a Contributor License Agreement (CLA) declaring that you have
 the right to, and actually do, grant us the rights to use your contribution. For
@@ -199,6 +202,7 @@ additional questions or comments.
 [StorageRequestFailedException]: https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/storage/Azure.Storage.Common/src/StorageRequestFailedException.cs
 [error_codes]: https://docs.microsoft.com/en-us/rest/api/storageservices/blob-service-error-codes
 [samples]: samples/
+[storage_contrib]: ../CONTRIBUTING.md
 [cla]: https://cla.microsoft.com
 [coc]: https://opensource.microsoft.com/codeofconduct/
 [coc_faq]: https://opensource.microsoft.com/codeofconduct/faq/

sdk/storage/Azure.Storage.Blobs/src/Generated/BlobRestClient.cs

@@ -116,7 +116,7 @@ internal static Azure.Core.Http.Request SetPropertiesAsync_CreateRequest(
 
                 // Create the body
                 System.Xml.Linq.XElement _body = Azure.Storage.Blobs.Models.BlobServiceProperties.ToXml(blobServiceProperties, "StorageServiceProperties", "");
-                string _text = _body.ToString();
+                string _text = _body.ToString(System.Xml.Linq.SaveOptions.DisableFormatting);
                 _request.Headers.SetValue("Content-Type", "application/xml");
                 _request.Headers.SetValue("Content-Length", _text.Length.ToString(System.Globalization.CultureInfo.InvariantCulture));
                 _request.Content = Azure.Core.Pipeline.HttpPipelineRequestContent.Create(System.Text.Encoding.UTF8.GetBytes(_text));
@@ -653,7 +653,7 @@ internal static Azure.Core.Http.Request GetUserDelegationKeyAsync_CreateRequest(
 
                 // Create the body
                 System.Xml.Linq.XElement _body = Azure.Storage.Blobs.Models.KeyInfo.ToXml(keyInfo, "KeyInfo", "");
-                string _text = _body.ToString();
+                string _text = _body.ToString(System.Xml.Linq.SaveOptions.DisableFormatting);
                 _request.Headers.SetValue("Content-Type", "application/xml");
                 _request.Headers.SetValue("Content-Length", _text.Length.ToString(System.Globalization.CultureInfo.InvariantCulture));
                 _request.Content = Azure.Core.Pipeline.HttpPipelineRequestContent.Create(System.Text.Encoding.UTF8.GetBytes(_text));
@@ -1742,7 +1742,7 @@ internal static Azure.Core.Http.Request SetAccessPolicyAsync_CreateRequest(
                         _body.Add(Azure.Storage.Blobs.Models.SignedIdentifier.ToXml(_child));
                     }
                 }
-                string _text = _body.ToString();
+                string _text = _body.ToString(System.Xml.Linq.SaveOptions.DisableFormatting);
                 _request.Headers.SetValue("Content-Type", "application/xml");
                 _request.Headers.SetValue("Content-Length", _text.Length.ToString(System.Globalization.CultureInfo.InvariantCulture));
                 _request.Content = Azure.Core.Pipeline.HttpPipelineRequestContent.Create(System.Text.Encoding.UTF8.GetBytes(_text));
@@ -9629,7 +9629,7 @@ internal static Azure.Core.Http.Request CommitBlockListAsync_CreateRequest(
 
                 // Create the body
                 System.Xml.Linq.XElement _body = Azure.Storage.Blobs.Models.BlockLookupList.ToXml(blocks, "BlockList", "");
-                string _text = _body.ToString();
+                string _text = _body.ToString(System.Xml.Linq.SaveOptions.DisableFormatting);
                 _request.Headers.SetValue("Content-Type", "application/xml");
                 _request.Headers.SetValue("Content-Length", _text.Length.ToString(System.Globalization.CultureInfo.InvariantCulture));
                 _request.Content = Azure.Core.Pipeline.HttpPipelineRequestContent.Create(System.Text.Encoding.UTF8.GetBytes(_text));

sdk/storage/Azure.Storage.Blobs/tests/AppendBlobClientTests.cs

@@ -403,8 +403,8 @@ public async Task AppendBlockAsync_WithUnreliableConnection()
                     new BlobContainerClient(
                         container.Uri,
                         new StorageSharedKeyCredential(
-                            TestConfigurations.DefaultTargetTenant.AccountName,
-                            TestConfigurations.DefaultTargetTenant.AccountKey),
+                            this.TestConfigDefault.AccountName,
+                            this.TestConfigDefault.AccountKey),
                         this.GetFaultyBlobConnectionOptions()));
 
                 // Arrange
@@ -422,13 +422,7 @@ public async Task AppendBlockAsync_WithUnreliableConnection()
                 using (var stream = new FaultyStream(new MemoryStream(data), 256 * Constants.KB, 1, new Exception("Simulated stream fault")))
                 {
                     await blobFaulty.AppendBlockAsync(stream, progressHandler: progressHandler);
-
-                    var attempts = 0;
-                    while (attempts++ < 7 && progressList.Last().BytesTransferred < data.LongLength)
-                    {
-                        // wait to allow lingering progress events to execute
-                        await this.Delay(500, 100).ConfigureAwait(false);
-                    }
+                    await this.WaitForProgressAsync(progressList, data.LongLength);
                     Assert.IsTrue(progressList.Count > 1, "Too few progress received");
                     // Changing from Assert.AreEqual because these don't always update fast enough
                     Assert.GreaterOrEqual(data.LongLength, progressList.Last().BytesTransferred, "Final progress has unexpected value");

sdk/storage/Azure.Storage.Blobs/tests/BlobBaseClientTests.cs

@@ -82,8 +82,8 @@ public async Task DownloadAsync_WithUnreliableConnection()
             // Arrange
             var service = this.InstrumentClient(
                 new BlobServiceClient(
-                    new Uri(TestConfigurations.DefaultTargetTenant.BlobServiceEndpoint),
-                    new StorageSharedKeyCredential(TestConfigurations.DefaultTargetTenant.AccountName, TestConfigurations.DefaultTargetTenant.AccountKey),
+                    new Uri(this.TestConfigDefault.BlobServiceEndpoint),
+                    new StorageSharedKeyCredential(this.TestConfigDefault.AccountName, this.TestConfigDefault.AccountKey),
                     this.GetFaultyBlobConnectionOptions(
                         raiseAt: 256 * Constants.KB,
                         raise: new Exception("Unexpected"))));