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

Refactor unnecessary try/catch #4057

Merged
merged 11 commits into from
Nov 19, 2024
15 changes: 7 additions & 8 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -414,23 +414,22 @@ private static bool ProcessITestDataSourceTests(UnitTestElement test, Reflection

// This code is to discover tests. To run the tests code is in TestMethodRunner.ExecuteDataSourceBasedTests.
// Any change made here should be reflected in TestMethodRunner.ExecuteDataSourceBasedTests as well.
try
{
data = dataSource.GetData(methodInfo);
data = dataSource.GetData(methodInfo);

if (!data.Any())
if (!data.Any())
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
{
if (!MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive)
{
throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, FrameworkMessages.DynamicDataIEnumerableEmpty, "GetData", dataSource.GetType().Name));
throw dataSource.GetExceptionForEmptyDataSource(methodInfo);
}
}
catch (ArgumentException) when (MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive)
{

UnitTestElement discoveredTest = test.Clone();
// Make the test not data driven, because it had no data.
discoveredTest.TestMethod.DataType = DynamicDataType.None;
discoveredTest.DisplayName = dataSource.GetDisplayName(methodInfo, null) ?? discoveredTest.DisplayName;

tests.Add(discoveredTest);

continue;
}

Expand Down
10 changes: 0 additions & 10 deletions src/Adapter/MSTest.TestAdapter/DynamicDataOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,6 @@ public IEnumerable<object[]> GetData(Type? _dynamicDataDeclaringType, DynamicDat
_dynamicDataDeclaringType.FullName));
}

if (!data.Any())
{
throw new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
FrameworkMessages.DynamicDataIEnumerableEmpty,
_dynamicDataSourceName,
_dynamicDataDeclaringType.FullName));
}

// Data is valid, return it.
return data;
}
Expand Down
19 changes: 9 additions & 10 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,18 @@ private bool ExecuteDataSourceBasedTests(List<TestResult> results)
{
isDataDriven = true;
IEnumerable<object?[]>? dataSource;
try
{
// This code is to execute tests. To discover the tests code is in AssemblyEnumerator.ProcessTestDataSourceTests.
// Any change made here should be reflected in AssemblyEnumerator.ProcessTestDataSourceTests as well.
dataSource = testDataSource.GetData(_testMethodInfo.MethodInfo);

if (!dataSource.Any())
// This code is to execute tests. To discover the tests code is in AssemblyEnumerator.ProcessTestDataSourceTests.
// Any change made here should be reflected in AssemblyEnumerator.ProcessTestDataSourceTests as well.
dataSource = testDataSource.GetData(_testMethodInfo.MethodInfo);

if (!dataSource.Any())
{
if (!MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive)
{
throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, FrameworkMessages.DynamicDataIEnumerableEmpty, "GetData", testDataSource.GetType().Name));
throw testDataSource.GetExceptionForEmptyDataSource(_testMethodInfo.MethodInfo);
}
}
catch (Exception ex) when (ex is ArgumentException && MSTestSettings.CurrentSettings.ConsiderEmptyDataSourceAsInconclusive)
{

var inconclusiveResult = new TestResult
{
Outcome = UTF.UnitTestOutcome.Inconclusive,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public enum DynamicDataSourceType
/// Attribute to define dynamic data for a test method.
/// </summary>
[AttributeUsage(AttributeTargets.Method, AllowMultiple = true)]
public sealed class DynamicDataAttribute : Attribute, ITestDataSource
public sealed class DynamicDataAttribute : Attribute, ITestDataSource, ITestDataSourceEmptyDataSourceExceptionInfo
{
private readonly string _dynamicDataSourceName;
private readonly DynamicDataSourceType _dynamicDataSourceType;
Expand Down Expand Up @@ -155,4 +155,14 @@ private static bool TryGetData(object dataSource, [NotNullWhen(true)] out IEnume
data = null;
return false;
}

string? ITestDataSourceEmptyDataSourceExceptionInfo.GetPropertyOrMethodNameForEmptyDataSourceException()
=> _dynamicDataSourceName;

string? ITestDataSourceEmptyDataSourceExceptionInfo.GetPropertyOrMethodContainerTypeNameForEmptyDataSourceException(MethodInfo testMethodInfo)
{
Type? type = _dynamicDataDeclaringType ?? testMethodInfo.DeclaringType;
DebugEx.Assert(type is not null, "Declaring type of test data cannot be null.");
return type.FullName;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Globalization;
using System.Reflection;

namespace Microsoft.VisualStudio.TestTools.UnitTesting;

internal static class EmptyDataSourceExceptionInfoExtensions
{
internal static ArgumentException GetExceptionForEmptyDataSource(this ITestDataSource dataSource, MethodInfo testMethodInfo)
=> dataSource is ITestDataSourceEmptyDataSourceExceptionInfo info
? info.GetExceptionForEmptyDataSource(testMethodInfo)
: new ArgumentException(
string.Format(
CultureInfo.InvariantCulture,
FrameworkMessages.DynamicDataIEnumerableEmpty,
"GetData",
dataSource.GetType().Name));

private static ArgumentException GetExceptionForEmptyDataSource(this ITestDataSourceEmptyDataSourceExceptionInfo info, MethodInfo testMethodInfo)
=> new(
string.Format(
CultureInfo.InvariantCulture,
FrameworkMessages.DynamicDataIEnumerableEmpty,
info.GetPropertyOrMethodNameForEmptyDataSourceException(),
info.GetPropertyOrMethodContainerTypeNameForEmptyDataSourceException(testMethodInfo)));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Reflection;

namespace Microsoft.VisualStudio.TestTools.UnitTesting;

internal interface ITestDataSourceEmptyDataSourceExceptionInfo
{
/// <summary>
/// Returns the method/property name accessed by this data source.
/// For example, for DynamicDataAttribute, that will be attribute argument.
/// </summary>
string? GetPropertyOrMethodNameForEmptyDataSourceException();

/// <summary>
/// Returns the type name on which the method/property accessed by this data source exists.
/// </summary>
string? GetPropertyOrMethodContainerTypeNameForEmptyDataSourceException(MethodInfo testMethodInfo);
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,15 @@ public void GetDataShouldThrowExceptionIfPropertyReturnsNull() =>
_dynamicDataAttribute.GetData(methodInfo);
});

public void GetDataShouldThrowExceptionIfPropertyReturnsEmpty() =>
VerifyThrows<ArgumentException>(() =>
{
MethodInfo methodInfo = _dummyTestClass.GetType().GetTypeInfo().GetDeclaredMethod("TestMethod5");
_dynamicDataAttribute = new DynamicDataAttribute("EmptyProperty", typeof(DummyTestClass));
_dynamicDataAttribute.GetData(methodInfo);
});
public void GetDataShouldNotThrowExceptionIfPropertyReturnsEmpty()
{
MethodInfo methodInfo = _dummyTestClass.GetType().GetTypeInfo().GetDeclaredMethod("TestMethod5");
_dynamicDataAttribute = new DynamicDataAttribute("EmptyProperty", typeof(DummyTestClass));
IEnumerable<object[]> data = _dynamicDataAttribute.GetData(methodInfo);
// The callers in AssemblyEnumerator and TestMethodRunner are responsible
// for throwing an exception if data is empty and ConsiderEmptyDataSourceAsInconclusive is false.
Verify(!data.Any());
}

public void GetDataShouldThrowExceptionIfPropertyDoesNotReturnCorrectType() =>
VerifyThrows<ArgumentNullException>(() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Reflection;
using System.Text;

Expand All @@ -11,6 +12,7 @@
using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.UnitTests.TestableImplementations;
using Microsoft.VisualStudio.TestTools.UnitTesting;

using Moq;

Expand Down Expand Up @@ -40,16 +42,12 @@ public class TestMethodRunnerTests : TestContainer

public TestMethodRunnerTests()
{
ConstructorInfo constructorInfo = typeof(DummyTestClass).GetConstructor([])!;
_methodInfo = typeof(DummyTestClass).GetMethods().Single(m => m.Name.Equals("DummyTestMethod", StringComparison.Ordinal));
var classAttribute = new UTF.TestClassAttribute();
_testMethodAttribute = new UTF.TestMethodAttribute();
PropertyInfo testContextProperty = typeof(DummyTestClass).GetProperty("TestContext");

var testAssemblyInfo = new TestAssemblyInfo(typeof(DummyTestClass).Assembly);
_testMethod = new TestMethod("dummyTestName", "dummyClassName", "dummyAssemblyName", false);
_testContextImplementation = new TestContextImplementation(_testMethod, new ThreadSafeStringWriter(null, "test"), new Dictionary<string, object>());
_testClassInfo = new TestClassInfo(typeof(DummyTestClass), constructorInfo, true, testContextProperty, classAttribute, testAssemblyInfo);
_testClassInfo = GetTestClassInfo<DummyTestClass>();

_testMethodOptions = new TestMethodOptions(TimeoutInfo.FromTimeout(200), null, _testContextImplementation, false, _testMethodAttribute);

Expand All @@ -68,6 +66,15 @@ public TestMethodRunnerTests()
ReflectHelper.Instance.ClearCache();
}

private static TestClassInfo GetTestClassInfo<T>()
{
ConstructorInfo constructorInfo = typeof(T).GetConstructor([])!;
PropertyInfo testContextProperty = typeof(T).GetProperty("TestContext");
var classAttribute = new UTF.TestClassAttribute();
var testAssemblyInfo = new TestAssemblyInfo(typeof(T).Assembly);
return new TestClassInfo(typeof(T), constructorInfo, isParameterlessConstructor: true, testContextProperty, classAttribute, testAssemblyInfo);
}

protected override void Dispose(bool disposing)
{
if (!IsDisposed)
Expand Down Expand Up @@ -369,6 +376,56 @@ public void RunTestMethodShouldSetResultFilesIfPresentForDataDrivenTests()
Verify(results[1].ResultFiles.ToList().Contains("C:\\temp.txt"));
}

public void RunTestMethodWithEmptyDataSourceShouldFailBecauseConsiderEmptyDataSourceAsInconclusiveIsFalse()
=> RunTestMethodWithEmptyDataSourceShouldFailIfConsiderEmptyDataSourceAsInconclusiveIsNotTrueHelper(false);

public void RunTestMethodWithEmptyDataSourceShouldNotFailBecauseConsiderEmptyDataSourceAsInconclusiveIsTrue()
=> RunTestMethodWithEmptyDataSourceShouldFailIfConsiderEmptyDataSourceAsInconclusiveIsNotTrueHelper(true);

private void RunTestMethodWithEmptyDataSourceShouldFailIfConsiderEmptyDataSourceAsInconclusiveIsNotTrueHelper(bool considerEmptyAsInconclusive)
{
Mock<PlatformServices.Interface.IReflectionOperations2> existingMock = _testablePlatformServiceProvider.MockReflectionOperations;
try
{
// We want this test to go through the "real" reflection to hit the product code path relevant for the test.
_testablePlatformServiceProvider.MockReflectionOperations = null;

string xml =
$$"""
<RunSettings>
<MSTestV2>
<ConsiderEmptyDataSourceAsInconclusive>{{considerEmptyAsInconclusive}}</ConsiderEmptyDataSourceAsInconclusive>
</MSTestV2>
</RunSettings>
""";

var settings = MSTestSettings.GetSettings(xml, MSTestSettings.SettingsNameAlias, null);
MSTestSettings.PopulateSettings(settings);

var testMethodInfo = new TestableTestMethodInfo(
typeof(DummyTestClassEmptyDataSource).GetMethod(nameof(DummyTestClassEmptyDataSource.TestMethod)),
GetTestClassInfo<DummyTestClassEmptyDataSource>(),
_testMethodOptions,
() => throw ApplicationStateGuard.Unreachable());
var testMethodRunner = new TestMethodRunner(testMethodInfo, _testMethod, _testContextImplementation);

if (considerEmptyAsInconclusive)
{
UnitTestResult[] results = testMethodRunner.RunTestMethod();
Verify(results[0].Outcome == AdapterTestOutcome.Inconclusive);
}
else
{
ArgumentException thrownException = VerifyThrows<ArgumentException>(() => testMethodRunner.RunTestMethod());
Verify(thrownException.Message == string.Format(CultureInfo.InvariantCulture, FrameworkMessages.DynamicDataIEnumerableEmpty, nameof(DummyTestClassEmptyDataSource.EmptyProperty), typeof(DummyTestClassEmptyDataSource).FullName));
}
}
finally
{
_testablePlatformServiceProvider.MockReflectionOperations = existingMock;
}
}

#region Test data

[SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "Use through reflection")]
Expand Down Expand Up @@ -451,5 +508,15 @@ public class DummyTestClassWithTestContextWithoutSetter
public UTFExtension.TestContext TestContext { get; }
}

public class DummyTestClassEmptyDataSource
{
public static IEnumerable<object[]> EmptyProperty => Array.Empty<object[]>();

[DynamicData("EmptyProperty")]
public void TestMethod(int x)
{
}
}

#endregion
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ public TestablePlatformServiceProvider()
MockTraceListener = new Mock<ITraceListener>();
MockTraceListenerManager = new Mock<ITraceListenerManager>();
MockThreadOperations = new Mock<IThreadOperations>();
TestTools.UnitTesting.DynamicDataProvider.Instance = SourceGeneratorToggle.UseSourceGenerator
? new SourceGeneratedDynamicDataOperations()
: new DynamicDataOperations();
}

#region Mock Implementations
Expand Down