MauriceKayser commented 8 months ago

1. Inconsistent usage

dotnet test seems to use the MemberData function directly, instead of the IXunitSerializable implementation to get the test data, unlike the Visual Studio Test Explorer. Using an integer value as an example, the value of entry.Data in void Test(TestEntry entry) is 123 when executed via dotnet test, but 0 when executed via the Visual Studio Test Explorer, as nothing is (de-)serialized in the IXunitSerializable implementation.

1. Minimal example

In Visual Studio 2022 create a new xUnit Test Project with .NET 8.0 LTS and the name TestProject1.
Update all NuGet dependencies, which currently are:
- Microsoft.NET.Test.Sdk: 17.8.0
- xunit: 2.6.5
- xunit.runner.visualstudio: 2.5.6

Replace the content of the UnitTest1 class with:

[Theory]
[MemberData(nameof(TestData))]
public void Test(TestEntry entry)
{
   Assert.Equal(123, entry.Data);
}

public static IEnumerable<object[]> TestData()
{
   return new[] { new[] { new TestEntry<object> { Data = 123 } } };
}

public class TestEntry : IXunitSerializable
{
   public int Data { get; set; }
   public void Serialize(IXunitSerializationInfo info) {}
   public void Deserialize(IXunitSerializationInfo info) {}
}

Run the test in the Visual Studio Test Explorer, the assertion fails:

Assert.Equal() Failure: Values differ Expected: 123 Actual: 0
Run dotnet test TestProject1.sln, the assertion does not fail (freely translated from localized version):

Succeeded! : Errors: 0, successful: 1, skipped: 0, total: 1, Runtime: < 1 ms - TestProject1.dll (net8.0)

2. Inconsistent usage

The Visual Studio Test Explorer also inconsistently uses the (de-)serialized data. The test run and tree view use the deserialized data, while the details view uses the original data from the MemberData function.

2. Minimal example

Replace the content of the UnitTest1 class with:

[Theory]
[MemberData(nameof(TestData))]
public void Test(TestEntry _) {}

public static IEnumerable<object[]> TestData()
{
  return new[] { new [] { new TestEntry { Name = "123" } } };
}

public class TestEntry : IXunitSerializable
{
  public string Name { get; set; }
  public TestEntry() { Name = ""; }
  public void Serialize(IXunitSerializationInfo info) {}
  public void Deserialize(IXunitSerializationInfo info) { Name = "abc"; }
  public override string ToString() { return Name; }
}

Execute the test in the Visual Studio Test Explorer.
The tree view shows Test(_: abc) while the details view shows Test(_: 123).

Context

I stumbled upon this, because I use System.Text.Json.JsonSerializer to (de-)serialize data in class TestEntry<T> : IXunitSerializable, where T can contain types like tuples, which IXunitSerializationInfo.AddValue does not otherwise support. That causes entry.Data in void Test(TestEntry<object> entry) to be of type System.Text.Json.JsonElement in the Visual Studio Test Explorer execution, but their original, pre-JsonSerialization type in the dotnet test execution.

Reproduction steps:

Replace the content of UnitTest1.cs with:

using System.Text.Json;
using Xunit.Abstractions;

namespace TestProject1
{
   public class UnitTest1
   {
       [Theory]
       [MemberData(nameof(TestData))]
       public void Test(Property<object> property)
       {
           Assert.Equal(   typeof(JsonElement), property.Data!.GetType());
           Assert.NotEqual(typeof(JsonElement), property.Data!.GetType());
       }

       public static IEnumerable<object[]> TestData()
       {
           return new[] {
               new[] { new Property<object> { Name = "Age", Data = 18 } },
               new[] { new Property<object> { Name = "Name", Data = "John" } }
           };
       }

       public class Property<T> : IXunitSerializable
       {
           public string Name { get; set; }
           public T? Data { get; set; }

           public Property() { Name = ""; }

           public void Serialize(IXunitSerializationInfo info)
           {
               info.AddValue(nameof(Name), Name);
               info.AddValue(nameof(Data), JsonSerializer.Serialize(Data));
           }

           public void Deserialize(IXunitSerializationInfo info)
           {
               Name = info.GetValue<string>(nameof(Name));
               Data = JsonSerializer.Deserialize<T>(info.GetValue<string>(nameof(Data)));
           }

           public override string ToString() { return Name; }
       }
   }
}

Run the test in the Visual Studio Test Explorer, the 2nd assertion fails:

Assert.NotEqual() Failure: Values are equal Expected: not typeof(System.Text.Json.JsonElement) Actual: typeof(System.Text.Json.JsonElement)
Run dotnet test TestProject1.sln, the 1st assertion fails:

Assert.Equal() Failure: Values differ Expected: typeof(System.Text.Json.JsonElement) Actual: typeof(int)

I would prefer for the Visual Studio Test Explorer to behave like dotnet test and not expect any (de-)serialized data. Then I could get rid of the JsonSerializer and my clunky helper function to convert data types from JsonElement to their actual types.

bradwilson commented 8 months ago

You've given two examples, and they both show a fundamental problem: we (reasonably) expect that the data should be the same both pre- and post-serialization, and it's up to you to ensure that happens.

But first, let's clear up one part of the mystery: we don't always use serialization. In the case of dotnet test, we are running outside the IDE, so we don't serialize and de-serialize anything. The process of discovery and execution happens in the same process, so we use the original data. In the case of Test Explorer, VSTest runs discovery and execution in separate processes, and as such the data is serialized in the discovery process and deserialized in the execution process.

This is why it's critical that your data be identical pre- and post-serialization, because sometimes your data is serialized and sometimes it isn't.

So let's look at your two examples.

Minimal Example

This one is obvious: pre-serialization, the value is "123", and post-serialization the value is "abc".

This violates our expectations. Given the description of how dotnet test and Test Explorer differ w/rt serialization, you're getting exactly the results I would've expected.

Full Reproduction

When you call JsonSerializer.Deserialize<T> and T is object, the JSON deserializer gives you back JsonElement. This is its expected behavior. It does not convert the values back into int and string as you presumably expected. Prior to serialization, your Data values are int and string, but after deserialization they're both JsonElement.

Again, this violates our expectations, and again the results are exactly as I would've expected.

There is an overload of JsonSerializer.Deserialize<T> which accepts JsonSerializerOptions. You're calling the overload without it, which uses default options. One of those options is UnknownTypeHandling, which is an instance of JsonUnknownTypeHandling that indicates how deserializing of object is handled: return JsonElement or return JsonNode.

There is no option to "translate the value into the closest matching CLR type", which is what it sounds like you expected it would do. You could ask for an additional flag here to do that, which would then make JsonSerializer.Deserialize<object> do what you expected, though I presume they considered that and discarded it because the JSON doesn't carry type metadata information. In your example, you serialized an int, but there is no pure integral type in JavaScript/JSON, only Number. When deserializing a value like 18, should it become an int, float, double, or decimal? At the time you asked to deserialize it, it does not have any information about what the best type to map it would be. Guessing seems inappropriate, so it counts on you to pass the correctly desired type for T, and in the fact of object it just gives you back the raw JsonElement (or JsonNode, if you so choose) to let you decide what to do with it later.