Remove ignore on tests that were failing due to faulted tasks in inte…

…rceptor (#20341)

* Change the accessbility to virtual for Resource.Id

* remove ignore on tests and update intrumentation to handle faulted tasks

Co-authored-by: YalinLi0312 <yall@microsoft.com>
Co-authored-by: m-nash <micnash@microsoft.com>

common/ManagementTestShared/Redesign/ManagementRecordedTestBase.cs

@@ -4,6 +4,7 @@
 using Azure.Core;
 using Azure.Core.TestFramework;
 using Azure.ResourceManager.Core;
+using Castle.DynamicProxy;
 using NUnit.Framework;
 using System;
 using System.Collections.Generic;
@@ -51,6 +52,8 @@ private ArmClient GetCleanupClient()
             return null;
         }
 
+        protected TClient InstrumentClientExtension<TClient>(TClient client) => (TClient)InstrumentClient(typeof(TClient), client, new IInterceptor[] { new ManagementInterceptor(this) });
+
         protected ArmClient GetArmClient(ArmClientOptions clientOptions = default)
         {
             var options = InstrumentClientOptions(clientOptions ?? new ArmClientOptions());

sdk/core/Azure.Core.TestFramework/src/ManagementInterceptor.cs

@@ -8,7 +8,7 @@
 
 namespace Azure.Core.TestFramework
 {
-    internal class ManagementInterceptor : IInterceptor
+    public class ManagementInterceptor : IInterceptor
     {
         private readonly ClientTestBase _testBase;
         private static readonly ProxyGenerator s_proxyGenerator = new ProxyGenerator();
@@ -31,6 +31,9 @@ public void Intercept(IInvocation invocation)
             var type = result.GetType();
             if (type.Name.StartsWith("Task"))
             {
+                if (((Task)result).IsFaulted || ((Task)result).Status == TaskStatus.WaitingForActivation)
+                    return;
+
                 var taskResultType = type.GetGenericArguments()[0];
                 if (taskResultType.Name.StartsWith("ArmResponse") || taskResultType.Name.StartsWith("ArmOperation"))
                 {
@@ -44,6 +47,9 @@ public void Intercept(IInvocation invocation)
             }
             else if (type.Name.StartsWith("ValueTask"))
             {
+                if ((bool)type.GetProperty("IsFaulted").GetValue(result))
+                    return;
+
                 var taskResultType = type.GetGenericArguments()[0];
                 if (taskResultType.Name.StartsWith("Response") || taskResultType.Name.StartsWith("ArmResponse"))
                 {

sdk/core/Azure.Core/tests/ManagementRecordedTestBaseTests.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
 using System.Threading.Tasks;
 using Azure.Core.TestFramework;
 using Azure.ResourceManager.Core;
@@ -134,5 +135,52 @@ public async Task ValidateWaitForCompletion()
             Assert.AreEqual("TestResourceProxy", testResource.GetType().Name);
             Assert.AreEqual("success", testResource.Method());
         }
+
+        [Test]
+        public async Task ValidateExceptionResponse()
+        {
+            ManagementTestClient client = InstrumentClient(new ManagementTestClient());
+            TestResourceOperations rgOp = client.GetTestResourceOperations();
+            try
+            {
+                var testResourceOp = await rgOp.GetArmResponseExceptionAsync();
+                Assert.Fail("Argument exception wasn't thrown");
+            }
+            catch (ArgumentException)
+            {
+            }
+        }
+
+        [Test]
+        public async Task ValidateExceptionOperation()
+        {
+            ManagementTestClient client = InstrumentClient(new ManagementTestClient());
+            TestResourceOperations rgOp = client.GetTestResourceOperations();
+            try
+            {
+                var testResourceOp = await rgOp.GetArmOperationExceptionAsync();
+                var testResource = await testResourceOp.WaitForCompletionAsync();
+                Assert.Fail("Argument exception wasn't thrown");
+            }
+            catch (ArgumentException)
+            {
+            }
+        }
+
+        [Test]
+        public async Task ValidateExceptionOperationWaitForCompletion()
+        {
+            ManagementTestClient client = InstrumentClient(new ManagementTestClient());
+            TestResourceOperations rgOp = client.GetTestResourceOperations();
+            var testResourceOp = await rgOp.GetArmOperationAsync(true);
+            try
+            {
+                var testResource = await testResourceOp.WaitForCompletionAsync();
+                Assert.Fail("Argument exception wasn't thrown");
+            }
+            catch (ArgumentException)
+            {
+            }
+        }
     }
 }

sdk/core/Azure.Core/tests/TestClients/ArmOperationTest.cs

@@ -12,13 +12,16 @@ public class ArmOperationTest<T> : ArmOperation<T>
         where T : class
     {
         private T _value;
+        private bool _exceptionOnWait;
+
         protected ArmOperationTest()
         {
         }
 
-        public ArmOperationTest(T value)
+        public ArmOperationTest(T value, bool exceptionOnWait = false)
         {
             _value = value;
+            _exceptionOnWait = exceptionOnWait;
         }
 
         public override string Id => "testId";
@@ -36,21 +39,33 @@ public override Response GetRawResponse()
 
         public override ValueTask<Response<T>> WaitForCompletionAsync(CancellationToken cancellationToken = default)
         {
+            if (_exceptionOnWait)
+                throw new ArgumentException("FakeArg");
+
             return new ValueTask<Response<T>>(Response.FromValue(_value, null));
         }
 
         public override ValueTask<Response<T>> WaitForCompletionAsync(TimeSpan pollingInterval, CancellationToken cancellationToken)
         {
+            if (_exceptionOnWait)
+                throw new ArgumentException("FakeArg");
+
             return new ValueTask<Response<T>>(Response.FromValue(_value, null));
         }
 
         public override ValueTask<Response> UpdateStatusAsync(CancellationToken cancellationToken = default)
         {
+            if (_exceptionOnWait)
+                throw new ArgumentException("FakeArg");
+
             return new ValueTask<Response>(Response.FromValue(_value, null) as Response);
         }
 
         public override Response UpdateStatus(CancellationToken cancellationToken = default)
         {
+            if (_exceptionOnWait)
+                throw new ArgumentException("FakeArg");
+
             return Response.FromValue(_value, null) as Response;
         }
     }

sdk/core/Azure.Core/tests/TestClients/TestResourceOperations.cs

@@ -17,14 +17,14 @@ public virtual TestResourceOperations GetAnotherOperations()
             return new TestResource();
         }
 
-        public virtual ArmOperationTest<TestResource> GetArmOperation(CancellationToken cancellationToken = default)
+        public virtual ArmOperationTest<TestResource> GetArmOperation(bool exceptionOnWait = false, CancellationToken cancellationToken = default)
         {
             using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmOperation");
             scope.Start();
 
             try
             {
-                return new ArmOperationTest<TestResource>(new TestResource());
+                return new ArmOperationTest<TestResource>(new TestResource(), exceptionOnWait);
             }
             catch (Exception e)
             {
@@ -33,14 +33,14 @@ public virtual ArmOperationTest<TestResource> GetArmOperation(CancellationToken
             }
         }
 
-        public virtual Task<ArmOperationTest<TestResource>> GetArmOperationAsync(CancellationToken cancellationToken = default)
+        public virtual Task<ArmOperationTest<TestResource>> GetArmOperationAsync(bool exceptionOnWait = false, CancellationToken cancellationToken = default)
         {
             using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmOperation");
             scope.Start();
 
             try
             {
-                return Task.FromResult(new ArmOperationTest<TestResource>(new TestResource()));
+                return Task.FromResult(new ArmOperationTest<TestResource>(new TestResource(), exceptionOnWait));
             }
             catch (Exception e)
             {
@@ -145,6 +145,70 @@ public virtual Task<ArmResponseTest<TestResource>> GetPhArmResponseAsync(Cancell
             }
         }
 
+        public virtual ArmResponseTest<TestResource> GetArmResponseException(CancellationToken cancellationToken = default)
+        {
+            using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmResponseException");
+            scope.Start();
+
+            try
+            {
+                throw new ArgumentException("FakeArg");
+            }
+            catch (Exception e)
+            {
+                scope.Failed(e);
+                throw;
+            }
+        }
+
+        public virtual Task<ArmResponseTest<TestResource>> GetArmResponseExceptionAsync(CancellationToken cancellationToken = default)
+        {
+            using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmResponseException");
+            scope.Start();
+
+            try
+            {
+                throw new ArgumentException("FakeArg");
+            }
+            catch (Exception e)
+            {
+                scope.Failed(e);
+                throw;
+            }
+        }
+
+        public virtual ArmOperationTest<TestResource> GetArmOperationException(CancellationToken cancellationToken = default)
+        {
+            using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmOperationException");
+            scope.Start();
+
+            try
+            {
+                throw new ArgumentException("FakeArg");
+            }
+            catch (Exception e)
+            {
+                scope.Failed(e);
+                throw;
+            }
+        }
+
+        public virtual Task<ArmOperationTest<TestResource>> GetArmOperationExceptionAsync(CancellationToken cancellationToken = default)
+        {
+            using var scope = _diagnostic.CreateScope("TestResourceOperations.GetArmOperationException");
+            scope.Start();
+
+            try
+            {
+                throw new ArgumentException("FakeArg");
+            }
+            catch (Exception e)
+            {
+                scope.Failed(e);
+                throw;
+            }
+        }
+
         public virtual string Method()
         {
             return "success";

sdk/resourcemanager/Azure.ResourceManager.Core/src/ArmClient.cs

@@ -106,17 +106,17 @@ private ArmClient(
         /// <summary>
         /// Gets the Azure Resource Manager client options.
         /// </summary>
-        private ArmClientOptions ClientOptions;
+        protected virtual ArmClientOptions ClientOptions { get; private set; }
 
         /// <summary>
         /// Gets the Azure credential.
         /// </summary>
-        private TokenCredential Credential;
+        protected virtual TokenCredential Credential { get; private set; }
 
         /// <summary>
         /// Gets the base URI of the service.
         /// </summary>
-        private Uri BaseUri;
+        protected virtual Uri BaseUri { get; private set; }
 
         /// <summary>
         /// Gets the Azure subscriptions.

sdk/resourcemanager/Azure.ResourceManager.Core/src/ResourceGroupContainer.cs

@@ -219,9 +219,6 @@ public virtual AsyncPageable<ResourceGroup> ListAsync(CancellationToken cancella
         /// <inheritdoc />
         public override ArmResponse<ResourceGroup> Get(string resourceGroupName, CancellationToken cancellationToken = default)
         {
-            if (string.IsNullOrWhiteSpace(resourceGroupName))
-                throw new ArgumentException("resourceGroupName cannot be null or a whitespace.", nameof(resourceGroupName));
-
             using var scope = Diagnostics.CreateScope("ResourceGroupContainer.Get");
             scope.Start();
 
@@ -242,16 +239,14 @@ public override ArmResponse<ResourceGroup> Get(string resourceGroupName, Cancell
         /// <inheritdoc/>
         public override async Task<ArmResponse<ResourceGroup>> GetAsync(string resourceGroupName, CancellationToken cancellationToken = default)
         {
-            if (string.IsNullOrWhiteSpace(resourceGroupName))
-                throw new ArgumentException("resourceGroupName cannot be null or a whitespace.", nameof(resourceGroupName));
-
             using var scope = Diagnostics.CreateScope("ResourceGroupContainer.Get");
             scope.Start();
 
             try
             {
+                var result = await Operations.GetAsync(resourceGroupName, cancellationToken).ConfigureAwait(false);
                 return new PhArmResponse<ResourceGroup, ResourceManager.Resources.Models.ResourceGroup>(
-                await Operations.GetAsync(resourceGroupName, cancellationToken).ConfigureAwait(false),
+                result,
                 g =>
                 {
                     return new ResourceGroup(Parent, new ResourceGroupData(g));

sdk/resourcemanager/Azure.ResourceManager.Core/src/ResourceOperationsBase.cs

@@ -67,7 +67,7 @@ protected ResourceOperationsBase(OperationsBase parentOperations, ResourceIdenti
         /// </summary>
         /// <param name="clientContext"></param>
         /// <param name="id"></param>
-        internal ResourceOperationsBase(ClientContext clientContext, string id)
+        internal ResourceOperationsBase(ClientContext clientContext, ResourceIdentifier id)
             : base(clientContext, id)
         {
         }

sdk/resourcemanager/Azure.ResourceManager.Core/tests/Azure.ResourceManager.Core.Tests.csproj

@@ -30,6 +30,7 @@
   <ItemGroup>
     <Folder Include="SessionRecords\ContainerTryGetTest\" />
     <Folder Include="SessionRecords\ResourceGroupOperationsTests\" />
+    <Folder Include="SessionRecords\SubscriptionOperationsTests\" />
   </ItemGroup>
 
 </Project>

sdk/resourcemanager/Azure.ResourceManager.Core/tests/Scenario/ResourceGroupOperationsTests.cs

@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
+using System.Threading;
 using System.Threading.Tasks;
 using Azure.Core.TestFramework;
 using NUnit.Framework;
@@ -10,7 +12,7 @@ namespace Azure.ResourceManager.Core.Tests
     public class ResourceGroupOperationsTests : ResourceManagerTestBase
     {
         public ResourceGroupOperationsTests(bool isAsync)
-            : base(isAsync) //, RecordedTestMode.Record)
+            : base(isAsync)//, RecordedTestMode.Record)
         {
         }
 
@@ -32,6 +34,22 @@ public async Task StartDeleteRg()
             await deleteOp.WaitForCompletionAsync();
         }
 
+        [TestCase]
+        [RecordedTest]
+        public async Task StartDeleteNonExistantRg()
+        {
+            var rgOp = InstrumentClientExtension(Client.GetResourceGroupOperations($"/subscriptions/{TestEnvironment.SubscriptionId}/resourceGroups/fake"));
+            var deleteOp = rgOp.StartDeleteAsync();
+            try
+            {
+                var response = await (await deleteOp).WaitForCompletionAsync();
+                Assert.Fail("RequestFailedException was not thrown");
+            }
+            catch (RequestFailedException e) when (e.Status == 404)
+            {
+            }
+        }
+
         [TestCase]
         [RecordedTest]
         public async Task Get()