Skip to content

Add non-serialization strategy for data unfolding #4467

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add non-serialization strategy for data unfolding #4467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 36 additions & 19 deletions src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,11 @@ internal ICollection<UnitTestElement> EnumerateAssembly(
DataRowAttribute.TestIdGenerationStrategy = testIdGenerationStrategy;
DynamicDataAttribute.TestIdGenerationStrategy = testIdGenerationStrategy;

TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy = ReflectHelper.GetTestDataSourceOptions(assembly)?.UnfoldingStrategy switch
TestDataSourceOptionsAttribute? testDataSourceOptionsAttribute = ReflectHelper.GetTestDataSourceOptions(assembly);
TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy = testDataSourceOptionsAttribute?.UnfoldingStrategy switch
{
// When strategy is auto we want to unfold
TestDataSourceUnfoldingStrategy.Auto => TestDataSourceUnfoldingStrategy.Unfold,
TestDataSourceUnfoldingStrategy.Auto => TestDataSourceUnfoldingStrategy.UnfoldUsingDataContractJsonSerializer,
// When strategy is set, let's use it
{ } value => value,
// When the attribute is not set, let's look at the legacy attribute
Expand All @@ -103,7 +104,7 @@ internal ICollection<UnitTestElement> EnumerateAssembly(
// as the ID generator will ignore it.
(null, TestIdGenerationStrategy.Legacy) => TestDataSourceUnfoldingStrategy.Fold,
#pragma warning restore CS0618 // Type or member is obsolete
_ => TestDataSourceUnfoldingStrategy.Unfold,
_ => TestDataSourceUnfoldingStrategy.UnfoldUsingDataContractJsonSerializer,
},
};

Expand All @@ -115,8 +116,7 @@ internal ICollection<UnitTestElement> EnumerateAssembly(
continue;
}

List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, testRunParametersFromRunSettings, type, warnings, discoverInternals,
dataSourcesUnfoldingStrategy, testIdGenerationStrategy, fixturesTests);
List<UnitTestElement> testsInType = DiscoverTestsInType(assemblyFileName, testRunParametersFromRunSettings, type, warnings, discoverInternals, dataSourcesUnfoldingStrategy, testIdGenerationStrategy, fixturesTests);
tests.AddRange(testsInType);
}

Expand Down Expand Up @@ -367,10 +367,12 @@ private static bool TryUnfoldITestDataSources(UnitTestElement test, TestMethodIn
try
{
bool isDataDriven = false;
int dataSourceIndex = -1;
foreach (ITestDataSource dataSource in testDataSources)
{
dataSourceIndex++;
isDataDriven = true;
if (!TryUnfoldITestDataSource(dataSource, dataSourcesUnfoldingStrategy, test, new(testMethodInfo.MethodInfo, test.DisplayName), tempListOfTests))
if (!TryUnfoldITestDataSource(dataSource, dataSourceIndex, dataSourcesUnfoldingStrategy, test, new(testMethodInfo.MethodInfo, test.DisplayName), tempListOfTests))
{
// TODO: Improve multi-source design!
// Ideally we would want to consider each data source separately but when one source cannot be expanded,
Expand Down Expand Up @@ -400,7 +402,7 @@ private static bool TryUnfoldITestDataSources(UnitTestElement test, TestMethodIn
}
}

private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy, UnitTestElement test, ReflectionTestMethodInfo methodInfo, List<UnitTestElement> tests)
private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, int dataSourceIndex, TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy, UnitTestElement test, ReflectionTestMethodInfo methodInfo, List<UnitTestElement> tests)
{
var unfoldingCapability = dataSource as ITestDataSourceUnfoldingCapability;

Expand Down Expand Up @@ -455,6 +457,7 @@ private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, TestDat
// If strategy is DisplayName and we have a duplicate test name don't expand the test, bail out.
#pragma warning disable CS0618 // Type or member is obsolete
if (test.TestMethod.TestIdGenerationStrategy == TestIdGenerationStrategy.DisplayName
#pragma warning restore CS0618 // Type or member is obsolete
&& testDisplayNameFirstSeen.TryGetValue(discoveredTest.DisplayName!, out int firstIndexSeen))
{
string warning = string.Format(CultureInfo.CurrentCulture, Resource.CannotExpandIDataSourceAttribute_DuplicateDisplayName, firstIndexSeen, index, discoveredTest.DisplayName);
Expand All @@ -464,24 +467,38 @@ private static bool TryUnfoldITestDataSource(ITestDataSource dataSource, TestDat
// Duplicated display name so bail out. Caller will handle adding the original test.
return false;
}
#pragma warning restore CS0618 // Type or member is obsolete

try
if (unfoldingCapability?.UnfoldingStrategy == TestDataSourceUnfoldingStrategy.UnfoldUsingDataIndex
|| (unfoldingCapability?.UnfoldingStrategy is null or TestDataSourceUnfoldingStrategy.Auto
&& dataSourcesUnfoldingStrategy == TestDataSourceUnfoldingStrategy.UnfoldUsingDataIndex))
{
discoveredTest.TestMethod.SerializedData = DataSerializationHelper.Serialize(d);
discoveredTest.TestMethod.TestDataSourceIgnoreMessage = testDataSourceIgnoreMessage;
discoveredTest.TestMethod.SerializedData = new string[3]
{
TestDataSourceUnfoldingStrategy.UnfoldUsingDataIndex.ToString(),
dataSourceIndex.ToString(CultureInfo.InvariantCulture),
index.ToString(CultureInfo.InvariantCulture),
};
discoveredTest.TestMethod.DataType = DynamicDataType.ITestDataSource;
}
catch (SerializationException ex)
else
{
string warning = string.Format(CultureInfo.CurrentCulture, Resource.CannotExpandIDataSourceAttribute_CannotSerialize, index, discoveredTest.DisplayName);
warning += Environment.NewLine;
warning += ex.ToString();
warning = string.Format(CultureInfo.CurrentCulture, Resource.CannotExpandIDataSourceAttribute, test.TestMethod.ManagedTypeName, test.TestMethod.ManagedMethodName, warning);
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning($"DynamicDataEnumerator: {warning}");
try
{
discoveredTest.TestMethod.SerializedData = DataSerializationHelper.Serialize(d);
discoveredTest.TestMethod.TestDataSourceIgnoreMessage = testDataSourceIgnoreMessage;
discoveredTest.TestMethod.DataType = DynamicDataType.ITestDataSource;
}
catch (SerializationException ex)
{
string warning = string.Format(CultureInfo.CurrentCulture, Resource.CannotExpandIDataSourceAttribute_CannotSerialize, index, discoveredTest.DisplayName);
warning += Environment.NewLine;
warning += ex.ToString();
warning = string.Format(CultureInfo.CurrentCulture, Resource.CannotExpandIDataSourceAttribute, test.TestMethod.ManagedTypeName, test.TestMethod.ManagedMethodName, warning);
PlatformServiceProvider.Instance.AdapterTraceLogger.LogWarning($"DynamicDataEnumerator: {warning}");

// Serialization failed for the type, bail out. Caller will handle adding the original test.
return false;
// Serialization failed for the type, bail out. Caller will handle adding the original test.
return false;
}
}

discoveredTests.Add(discoveredTest);
Expand Down
55 changes: 41 additions & 14 deletions src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,22 +169,11 @@ internal List<TestResult> RunTestMethod()

bool isDataDriven = false;
var parentStopwatch = Stopwatch.StartNew();
if (_test.DataType == DynamicDataType.ITestDataSource)
if (TryExecuteITestDataSource(results))
{
if (_test.TestDataSourceIgnoreMessage is not null)
{
return [new() { Outcome = UTF.UnitTestOutcome.Ignored, IgnoreReason = _test.TestDataSourceIgnoreMessage }];
}

object?[]? data = DataSerializationHelper.Deserialize(_test.SerializedData);
TestResult[] testResults = ExecuteTestWithDataSource(null, data);
results.AddRange(testResults);
}
else if (TryExecuteDataSourceBasedTests(results))
{
isDataDriven = true;
}
else if (TryExecuteFoldedDataDrivenTests(results))
else if (TryExecuteDataSourceBasedTests(results)
|| TryExecuteFoldedDataDrivenTests(results))
{
isDataDriven = true;
}
Expand Down Expand Up @@ -526,4 +515,42 @@ private static List<TestResult> UpdateResultsWithParentInfo(

return updatedResults;
}

private bool TryExecuteITestDataSource(List<TestResult> results)
{
if (_test.DataType != DynamicDataType.ITestDataSource)
{
return false;
}

UTF.ITestDataSource? dataSource;
object?[]? data;
if (_test.SerializedData?.Length == 3)
{
if (!Enum.TryParse(_test.SerializedData[0], out TestDataSourceUnfoldingStrategy _)
|| !int.TryParse(_test.SerializedData[1], out int dataSourceIndex)
|| !int.TryParse(_test.SerializedData[2], out int dataIndex))
{
throw ApplicationStateGuard.Unreachable();
}

dataSource = _testMethodInfo.GetAttributes<Attribute>(false)?.OfType<UTF.ITestDataSource>().Skip(dataSourceIndex).FirstOrDefault();
Copy link
Member Author

Choose a reason for hiding this comment

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

Runtime team just told me not to rely on guaranteed reflection order. I'll try to see what other option we have. Given that this is currently always working fine, I'd vote for adding a comment and moving on with the PR.

if (dataSource is null)
{
throw ApplicationStateGuard.Unreachable();
}

data = dataSource.GetData(_testMethodInfo.MethodInfo).Skip(dataIndex).FirstOrDefault();
}
else
{
dataSource = null;
data = DataSerializationHelper.Deserialize(_test.SerializedData);
}

TestResult[] testResults = ExecuteTestWithDataSource(dataSource, data);
results.AddRange(testResults);

return true;
}
}
26 changes: 22 additions & 4 deletions src/TestFramework/TestFramework/Interfaces/ITestDataSourceUnfoldingCapability.cs
View file
Open in desktop
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. See LICENSE file in the project root for full license information.

using System.ComponentModel;

namespace Microsoft.VisualStudio.TestTools.UnitTesting;

/// <summary>
Expand All @@ -22,17 +24,33 @@ public enum TestDataSourceUnfoldingStrategy : byte
/// <summary>
/// MSTest will decide whether to unfold the parameterized test based on value from the assembly level attribute
/// <see cref="TestDataSourceOptionsAttribute" />. If no assembly level attribute is specified, then the default
/// configuration is to unfold.
/// configuration is to unfold using <see cref="System.Runtime.Serialization.Json.DataContractJsonSerializer"/>.
/// </summary>
Auto,

/// <summary>
/// Each data row is treated as a separate test case.
/// </summary>
/// <inheritdoc cref="UnfoldUsingDataContractJsonSerializer"/>
[EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("Use 'UnfoldUsingDataContractJsonSerializer' instead")]
Unfold,

/// <summary>
/// The parameterized test is not unfolded; all data rows are treated as a single test case.
/// </summary>
Fold,

/// <summary>
/// Each data row is treated as a separate test case, and the data is unfolded using
/// <see cref="System.Runtime.Serialization.Json.DataContractJsonSerializer"/>.
/// </summary>
UnfoldUsingDataContractJsonSerializer,
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this the same numeric value as Unfold?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's probably not important, is it?


/// <summary>
/// Each data row is treated as a separate test case, and the data is unfolded using the data
/// source index and data index.
/// </summary>
/// <remarks>
/// Using this strategy will alter the test ID if the data source is reordered, as it depends
/// on the index of the data. This may affect the ability to track test cases over time.
/// </remarks>
UnfoldUsingDataIndex,
}
2 changes: 2 additions & 0 deletions src/TestFramework/TestFramework/PublicAPI/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ Microsoft.VisualStudio.TestTools.UnitTesting.ITestDataSourceIgnoreCapability.Ign
Microsoft.VisualStudio.TestTools.UnitTesting.ITestDataSourceIgnoreCapability.IgnoreMessage.set -> void
Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute.IgnoreMessage.get -> string?
Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute.IgnoreMessage.set -> void
Microsoft.VisualStudio.TestTools.UnitTesting.TestDataSourceUnfoldingStrategy.UnfoldUsingDataContractJsonSerializer = 3 -> Microsoft.VisualStudio.TestTools.UnitTesting.TestDataSourceUnfoldingStrategy
Microsoft.VisualStudio.TestTools.UnitTesting.TestDataSourceUnfoldingStrategy.UnfoldUsingDataIndex = 4 -> Microsoft.VisualStudio.TestTools.UnitTesting.TestDataSourceUnfoldingStrategy
Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute.IgnoreMessage.get -> string?
Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute.IgnoreMessage.set -> void
Microsoft.VisualStudio.TestTools.UnitTesting.UnitTestOutcome.Ignored = 10 -> Microsoft.VisualStudio.TestTools.UnitTesting.UnitTestOutcome
Expand Down
Loading
Loading