microsoft / testfx

MSTest framework and adapter
MIT License
737 stars 254 forks source link

DynamicData test with parameter struct having implicit conversion to string leads to corrupted parameter #3962

Open abatishchev opened 4 days ago

abatishchev commented 4 days ago

Describe the bug

Given a struct with an implicit conversion to string (operator overload

public readonly struct AccountKind
{
    private readonly string _value;

    public AccountKind(string value) =>
        _value = value;

    public override string ToString() =>
        _value;

    public static implicit operator string(AccountKind obj) =>
        obj.ToString();

    public static implicit operator AccountKind(string str) =>
        new AccountKind(str);
}

And a test method which accepts it as a parameter via a [DynamicData] attribute:

[TestMethod]
[DynamicData(nameof(GetAccountKinds), DynamicDataSourceType.Method)]
public void TestMethod1(AccountKind kind)
{
  Assert.IsNotNull(kind.ToString());
}

private static IEnumerable<object[]> GetAccountKinds()
{
  yield return [new AccountKind("kind1")];
  yield return [new AccountKind("kind2")];
  yield return [new AccountKind("kind3")];
}

What happens then:

  1. Test discovery generates only 1 test:

Image

  1. Test execution passes a parameter with value null:

Image

Notes:

  1. This doesn't not happen when the assembly is decorated with [TestDataSourceDiscovery(TestDataSourceDiscoveryOption.DuringExecution)]

Image

  1. This does not happen when the parameter type is changed to string (see the attached repro for an example)

Image

  1. This does not happen when the parameter's type is class

Image

Steps To Reproduce

Attached a sample project: TestProject1.zip

Expected behavior

Test method is executed 3 times, each times receives a parameter with the correct value

Actual behavior

Test method is executed 1 time, it receives a parameter with null

Additional context

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
<PackageReference Include="MSTest.TestAdapter" Version="3.6.1" />
<PackageReference Include="MSTest.TestFramework" Version="3.6.1" />

VS 2022 version 17.11.5

Evangelink commented 3 days ago

Hi @abatishchev,

By default, MSTest tries to expand the parameterized tests to provide better experience in IDEs. To do so, the current solution is to serialize the data using JSON data contract serializer. Because your data isn't serializable, when reaching the method the data is null.

You could workaround that by adding the [Serializable] attribute on top of the AccountKind and if that's a production oriented type that you cannot/don't want to tweak for tests then I recommend to use an intermediate "test" type that would then be converted in the real type within the test method.

When using [TestDataSourceDiscovery(TestDataSourceDiscoveryOption.DuringExecution)] you are disabling the expansion of the data, removing the need to serialize the data. The result is that multiples results are attached to the same test (as you can see on screenshot 1).

I am not a big fan of the decision that was made to have the data serialized but we cannot easily change that without introducing breaking changes. I want to make some test to see if we could use a workaround to use an index based solution (with a manual optin) to avoid this issue. The only problem with index based solution is that we need the data source to be deterministic (e.g. a randomized data source index is not guaranteeing to produce the same data).

abatishchev commented 2 days ago

thanks for the detailed explanation, @Evangelink! do you know what's the (primary) reason for the (mis) behavior:

  1. The parameter is of type struct (given that class works fine, apparently)
  2. It has implicit conversion to string (or it doesn't play a role?)
Evangelink commented 1 day ago

Sadly, I don't know enough about this serializer to know why it behaves differently.