Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ignore on tests that were failing due to faulted tasks in interceptor #20341

Merged
merged 4 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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)
markcowl marked this conversation as resolved.
Show resolved Hide resolved
return;

var taskResultType = type.GetGenericArguments()[0];
if (taskResultType.Name.StartsWith("ArmResponse") || taskResultType.Name.StartsWith("ArmOperation"))
{
Expand All @@ -44,6 +47,9 @@ public void Intercept(IInvocation invocation)
}
else if (type.Name.StartsWith("ValueTask"))
{
if ((bool)type.GetProperty("IsFaulted").GetValue(result))
markcowl marked this conversation as resolved.
Show resolved Hide resolved
return;

var taskResultType = type.GetGenericArguments()[0];
if (taskResultType.Name.StartsWith("Response") || taskResultType.Name.StartsWith("ArmResponse"))
{
Expand Down
48 changes: 48 additions & 0 deletions sdk/core/Azure.Core/tests/ManagementRecordedTestBaseTests.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
{
}
}
}
}
17 changes: 16 additions & 1 deletion sdk/core/Azure.Core/tests/TestClients/ArmOperationTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
}
}
Expand Down
72 changes: 68 additions & 4 deletions sdk/core/Azure.Core/tests/TestClients/TestResourceOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument validation should happen before the _diagnostic.CreateScope

}
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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
markcowl marked this conversation as resolved.
Show resolved Hide resolved
throw new ArgumentException("resourceGroupName cannot be null or a whitespace.", nameof(resourceGroupName));

using var scope = Diagnostics.CreateScope("ResourceGroupContainer.Get");
scope.Start();

Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
<ItemGroup>
<Folder Include="SessionRecords\ContainerTryGetTest\" />
<Folder Include="SessionRecords\ResourceGroupOperationsTests\" />
<Folder Include="SessionRecords\SubscriptionOperationsTests\" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)
{
}

Expand All @@ -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()
Expand Down
Loading