Failing on test discovery if using IXunitSerializable in xUnit tests

Hi,

I am using xUnit and my tests are implemented as Theories with MemberData. I am using custom classes for test's input and I use IXunitSerializable interface to implement serialization for these classes. This way each test case gets discovered as an individual test instead of all cases being lumped together under one test per method.

During the test discovery nCrunch (3.15.0.6) under Visual Studio 2017 gives me this error:

An error occurred while analysing this project after it was built: System.Exception: Error while discovering test 'TestsClass.MyTests("TestsClass+MyTestData")':System.Runtime.Serialization.SerializationException: Type 'TestsClass+MyTestData' in Assembly 'Tests1, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' is not marked as serializable.
   at System.Runtime.Serialization.FormatterServices.InternalGetSerializableMembers(RuntimeType type)
   at System.Runtime.Serialization.FormatterServices.<>c__DisplayClass9_0.<GetSerializableMembers>b__0(MemberHolder _)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at System.Runtime.Serialization.FormatterServices.GetSerializableMembers(Type type, StreamingContext context)
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitMemberInfo()
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.InitSerialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.WriteObjectInfo.Serialize(Object obj, ISurrogateSelector surrogateSelector, StreamingContext context, SerObjectInfoInit serObjectInfoInit, IFormatterConverter converter, ObjectWriter objectWriter, SerializationBinder binder)
   at System.Runtime.Serialization.Formatters.Binary.ObjectWriter.Serialize(Object graph, Header[] inHeaders, __BinaryWriter serWriter, Boolean fCheck)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph, Header[] headers, Boolean fCheck)
   at nCrunch.TestExecution.Frameworks.XUnit2.TestCaseArgumentData.StoreInTest(FrameworkTest test)
   at nCrunch.Module.XUnit2.Integration.XUnitNCrunchDiscoveredTestContainer.StoreDiscoveredTest(ITestCase testCase, TestName testName)
   at nCrunch.Module.XUnit2.Integration.XUnitDiscoveryMessageSink.discoverTest(ITestCase testCase)

Here is the sample code:

public class MyTestData : IXunitSerializable
{
    public string Name { get; set; }

    public void Deserialize(IXunitSerializationInfo info)
    {
        Name = info.GetValue<string>("name");
    }

    public void Serialize(IXunitSerializationInfo info)
    {
        info.AddValue("name", Name, typeof(string));
    }
}

public static TheoryData<MyTestData> AllMyTests = new TheoryData<MyTestData>
{
    new MyTestData{ Name = "test1" },
    new MyTestData{ Name = "test2" },
};

[Theory]
[MemberData("AllMyTests")]
public void MyTests(MyTestData test)
{
}

If I remove IXunitSerializable interface, it all works, but all tests from the same method are shown as a single test in nCrunch and in the Visual Studio Test Explorer.

Adding [Serializable] attribute to all classes that are involved would not be a reasonable solution, but as an experiment I have tried that and it does not work as expected - only the first set of parameters values gets used, and the test method runs only once, all other tests age completely ignored.

Please note that Theories with parameters supplied via InlineData get discovered and executed as expected.

If possible, please provide an ETA for the fix.

Thank you,
Dmytro Zakharov

Hi Dmytro,

Thanks for sharing this issue.

Unfortunately, serialization of custom data using IXunitSerializable is not a use case we can support under NCrunch.

It may be possible for you to work around this problem by using [Serializable] instead, though as you've described, this will likely result in problems with test identification. See here for more information about this problem - http://www.ncrunch.net/documentation/considerations-and-constraints_unique-test-names.

Implementing .ToString() on your custom type to output a unique value may allow you to work around the test identification issue.

However, I strongly recommend avoiding the use of complex types for test case parameter passing, as this causes incredible complexity for test case runners and can make behaviour unpredictable. I suggest finding a different way of implementing this test.

Hi Remco,

I have looked into [Serializable] attribute route and that requires quite a lot of changes to the core data models simply because we do not do the binary serialization anywhere and supporting it was not necessary.

IXunitSerializable is basically what xUnit did to resolve this problem - they force you to implement this interface if you want to serialize complex types, and in our case this was easy to solve using the json serialization and just shove entire object into a single string value.

IXunitSerializable is a part of xUnit framework and it won't be too hard to inject support for it in nCrunch. Without it xUnit does not return you the individual test cases during the test discovery phase, which means that you don't have to serialize them. Instead it returns a test without parameters, and test cases will be discovered while running it during the next phase. I saw that you do support the test cases, so you already support the test case discovery and some serialization or their parameters. xUnit already implements full serialization and deserialization of the test cases either into a string or binary array, so there should be some easy way to piggyback on that.

We do override ToString, but since the test cases are not getting discovered individually, it only partially solves the issue.

Also, if there is not way to fully solve this issue, then it would be nice if you could make a change so that nCrunch would not fail on test discovery and just record the test without test cases.

Anyway, thanks for looking into this

Sorry, but we cannot support this use case with NCrunch. You will need to change your design.

I am struggling to understand if there is something I could do or not to have unique names with xUnit and ClassData. After trying different things for hours; I now have this very small setup:

The test:
[Theory]
[ClassData(typeof(MyTestCase))]
public void FigureItOut(MyTestCase testCase)
{
Assert.Equal(7, testCase.MyProperty);
}

The MyTestCase class:
public class MyTestCase : IEnumerable<object[]>
{
public override string ToString() {
return "Hello " + MyProperty.ToString();
}

public int MyProperty { get; set; }

public IEnumerator<object[]> GetEnumerator()
{
yield return new object[]
{
new MyTestCase{ MyProperty = 7}
};

yield return new object[]
{
new MyTestCase{ MyProperty = 8}
};
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

You mentioned something about overriding ToString() on this page http://www.ncrunch.net/documentation/considerations-and-constraints_unique-test-names but in this small example that don't make any difference. Is there a way to have the two tests show up as two tests in nCrunch or is that simply not possible?

Thank you very much for any kind of reply :-)

Thanks for the code sample. Using this I can observe the same behaviour as you've described - the test cases are being rolled up under the main test.

It looks like in this case, the limitation is in Xunit itself. Because the test case is reliant on a user defined type for its parameter data, it is considered to have unstable parameters. I understand this was implemented in this way for many of the same reasons described in the NCrunch unique test names documentation page. You'll likely notice that you see the same behaviour in the VS-based Xunit runner. Although NCrunch does form its own unique names using .ToString(), it never gets this far because Xunit reports the TestCases as being only a single test.

Again, I strongly recommend avoiding the use of user defined or complex types for test case parameters. By doing this, you are effectively injecting your own code into areas of the framework/runner where the error handling and reporting is not as robust as other areas, and there are a whole range of unexpected things that could happen. There is no guarantee that the environment being used to execute the test will be the same as that used for its discovery, and you greatly increase the risk of your code not being executable by other runners or being broken in future as the ecosystem changes. The risks of such an approach far outweigh its benefits. I suggest using simple types and constructing your user defined types at test runtime instead of during discovery.