[DataRow]: Original nested types are lost when nested deeper than one level

[mlsomers]

Describe the bug

Using a nested type like this in a DataRow:

object obj = new object[] 
{
  (byte)0,
  new object[] 
  {
    (short)597,
    (short)492
  },
  new object[0],
  (byte)0,
  (byte)0,
  "\u0001" 
};

All the Bytes and Shorts become Int32's

Steps To Reproduce

[TestClass]
public class DataRowTest
{
  [DataTestMethod]
  // These get corrupted
  [DataRow((byte)0, new object[] { (byte)0 })] // fails
  [DataRow((short)0, new object[] { (short)0 })] // fails
  [DataRow((long)0, new object[] { (long)0 })] // fails
  // [DataRow((int)0, new object[] { (byte)0 })] // Succeeds while it should fail
  public void CheckNestedInputTypes(object org, object nested)
  {
      Assert.IsTrue(org.GetType().Name.Equals(((object[])nested)[0].GetType().Name), // nested: fails, type changed to Int32
        string.Concat("Expected ", org.GetType().Name, " but got ", ((object[])nested)[0].GetType().Name)); 
  }

  [DataTestMethod]
  // These work correctly:
  [DataRow((byte)0, (byte)0)] // Succeeds (correct)
  [DataRow((short)0, (short)0)] // Succeeds (correct)
  // [DataRow((int)0, (byte)0)] // Fails (correct)
  public void CheckInputTypes(object org, object nested)
  {
      Assert.IsTrue(org.GetType().Name.Equals(nested.GetType().Name)); // not nested, works fine
  }
}

Expected behavior

I expect bytes, shorts and longs to stay bytes, shorts and longs and not upgrade/downgrade to Int32.

Actual behavior

Anything smaller than Int32 gets upgraded, for larger types it depends on the value if they get downgraded or not.

Additional context

Ran into this bug when migrating my MsgPack project to NetStandard2.1 and migrating this test to MSTest instead of NUnit (hoping to get a smoother integrated experience in Visual Studio). The NUnit version worked fine by the way.

[mlsomers]

I have now pushed my failing tests so you guys can just open the solution (LsMsgPack.sln) and run them.

Note that it is the LsMsgPackNetStandardUnitTests.csproj that has the MsTest version.
The other tests in LsMsgPackUnitTests.csproj are still in Nunit.

[Evangelink]

Hi @mlsomers,

This is yet another case of serialization issue (see #1462). There's sadly nothing I can do before v4.

[mlsomers]

Haha, no Json does not preserve types while in transit, it is "stringly" typed.
Not sure why the values need to be serialized in the first place, seems like quite allot of unneeded overhead but maybe my MsgPack serializer is worth a look, it has an option to preserve types.
https://github.com/mlsomers/LsMsgPack

[Evangelink]

Not sure why the values need to be serialized in the first place, seems like quite allot of unneeded overhead

Long story short, many weird decisions were made in MSTest 🤣

To give more context, original devs decided to serialize data to push it up to the UI so that when you run a single test from let's say VS you provide "all" the required info to run this test. This is a design as another but I personally don't like it and would have prefered to push only some "index" or identified of the data so that I can then resolve it locally. This is having a small local overhead of querying the data multiple times but outside access to big systems this should be neglictible.

Sadly because of all the open points in MSTest, we cannot easily update that decision.

MsgPack serializer is worth a look, it has an option to preserve types.

Yes this is a consideration. My ideal would be to avoid yet another deps as it could be causing issues to users. Let's say I want to test my main app with a given version of msg pack, if I force some version it could clash with the user one. We have seen that many times with VSTest forcing newtonsoft and nuget packages.

[mlsomers]

You could just copy the source and change the namespace. Currently only 17 files in netStandard2.1.
That way it would never give a conflict. I'll be glad to give permission if that's required :-)
I think all primitive types are covered. Complex types are put into nested dictionaries similar to Json, that part is not standardised in MsgPack, but it's a feature of my lib.
I'm planning to add support for base-types or interfaces that have multiple possible subtypes shortly. By optionally adding the parent types in the first dictionary entry. This will save the end-user allot of manual custom resolving code with minimal overhead.

[Evangelink]

I'll be glad to give permission if that's required :-)

Happy to hear that! I'll discuss with the team, this could be an easy upgrade/fix that would give us time to think how to change more fundamentaly the process. Thank you for the offer @mlsomers <3

[Evangelink]

Closing in favor of the overall ticket #1462