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

Fix DataRow issues related to serialization/deserialization #4078

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
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 @@ -460,6 +460,7 @@ private static bool ProcessITestDataSourceTests(UnitTestElement test, Reflection
try
{
discoveredTest.TestMethod.SerializedData = DataSerializationHelper.Serialize(d);
discoveredTest.TestMethod.ActualData = d;
discoveredTest.TestMethod.DataType = DynamicDataType.ITestDataSource;
}
catch (SerializationException ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ internal UnitTestResult[] RunTestMethod()
{
if (_test.DataType == DynamicDataType.ITestDataSource)
{
object?[]? data = DataSerializationHelper.Deserialize(_test.SerializedData);
object?[]? data = _test.ActualData ?? DataSerializationHelper.Deserialize(_test.SerializedData);
TestResult[] testResults = ExecuteTestWithDataSource(null, data);
results.AddRange(testResults);
}
Expand Down
26 changes: 26 additions & 0 deletions src/Adapter/MSTest.TestAdapter/ObjectModel/TestMethod.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. See LICENSE file in the project root for full license information.

using System.Collections.Concurrent;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;

Expand Down Expand Up @@ -28,6 +29,8 @@ public sealed class TestMethod : ITestMethod
private string? _declaringClassFullName;
private string? _declaringAssemblyName;

private static readonly ConcurrentDictionary<string, object?[]> DataDictionary = new();
Copy link
Member Author

@Youssef1313 Youssef1313 Nov 18, 2024

Choose a reason for hiding this comment

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

Notes:

  • This is bad. The dictionary in theory can grow up indefinitely.
  • Currently, UniqueName isn't really unique and we need a better mechanism here. One possibility is to attach a test property resulting from Interlocked.Increment on a static counter and use that as the dictionary key. Then, instead of deserializing we look up that key.
  • This PR is mostly to open a discussion about the possibilities to improve the situation for some parameterized tests broken scenarios.

@Evangelink thoughts? I feel like some of the parameterized test issues could be fixable without the need for breaking changes?


public TestMethod(string name, string fullClassName, string assemblyName, bool isAsync)
: this(null, null, null, name, fullClassName, assemblyName, isAsync, null, TestIdGenerationStrategy.FullyQualified)
{
Expand Down Expand Up @@ -137,6 +140,29 @@ public string? DeclaringClassFullName
/// </summary>
internal string?[]? SerializedData { get; set; }

private object?[]? _actualData;

[DisallowNull]
internal object?[]? ActualData
{
get
{
if (_actualData is not null)
{
return _actualData;
}

DataDictionary.TryGetValue(UniqueName, out _actualData);
return _actualData;
}

set
{
_actualData = value;
DataDictionary.TryAdd(UniqueName, value);
}
}

/// <summary>
/// Gets or sets the test group set during discovery.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Testing.Platform.Acceptance.IntegrationTests;
using Microsoft.Testing.Platform.Acceptance.IntegrationTests.Helpers;
using Microsoft.Testing.Platform.Helpers;

namespace MSTest.Acceptance.IntegrationTests;

[TestGroup]
public class DataRowTests : AcceptanceTestBase
{
private const string SourceCode = """
#file DataSourceTests.csproj
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>$TargetFramework$</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<OutputType>Exe</OutputType>
<LangVersion>preview</LangVersion>
<EnableMSTestRunner>true</EnableMSTestRunner>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="$MicrosoftNETTestSdkVersion$" />
<PackageReference Include="MSTest.TestAdapter" Version="$MSTestVersion$" />
<PackageReference Include="MSTest.TestFramework" Version="$MSTestVersion$" />
</ItemGroup>
</Project>

#file MyTestClass.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[DataTestMethod]
[DataRow((byte)0, new object[] { (byte)0 })]
[DataRow((short)0, new object[] { (short)0 })]
[DataRow((long)0, new object[] { (long)0 })]
public void CheckNestedInputTypes(object org, object nested)
{
Assert.AreEqual(org.GetType(), (((object[])nested)[0].GetType()));
}
}
""";

private readonly AcceptanceFixture _acceptanceFixture;

public DataRowTests(ITestExecutionContext testExecutionContext, AcceptanceFixture acceptanceFixture)
: base(testExecutionContext) => _acceptanceFixture = acceptanceFixture;

public async Task TestDataRowNumericalInArrayDoesNotLoseOriginalType()
{
using TestAsset generator = await TestAsset.GenerateAssetAsync(
"DataRowTests",
SourceCode
.PatchCodeWithReplace("$MSTestVersion$", MSTestVersion)
.PatchCodeWithReplace("$MicrosoftNETTestSdkVersion$", MicrosoftNETTestSdkVersion)
.PatchCodeWithReplace("$TargetFramework$", TargetFrameworks.NetCurrent.Arguments),
addPublicFeeds: true);

await DotnetCli.RunAsync(
$"build {generator.TargetAssetPath} -c Release",
_acceptanceFixture.NuGetGlobalPackagesFolder.Path,
retryCount: 0);

var testHost = TestHost.LocateFrom(generator.TargetAssetPath, "DataSourceTests", TargetFrameworks.NetCurrent.Arguments);

TestHostResult result = await testHost.ExecuteAsync();
result.AssertExitCodeIs(ExitCodes.Success);
result.AssertOutputContainsSummary(failed: 0, passed: 3, skipped: 0);
}
}
Loading