!DataSerializationHelper.SerializerSettings.SerializeReadOnlyTypes

[aliquid]

Describe the bug

Read-only data cannot be used as dynamic data, because the serializer ignores read-only types. Use-case for read-only data is when the application subject for testing has that kind of data.

Steps To Reproduce

Conceptually in C# Interactive window, showing example of read-only data not being serialized unless SerializeReadOnlyTypes=true;

#r "System.Runtime.Serialization.Json"
using System.Runtime.Serialization;
using System.Runtime.Serialization.Json;

readonly struct Foo1 { public Foo1(int a) { A = a; } public int A { get; } }
var ms1 = new MemoryStream();
new DataContractJsonSerializer(typeof(Foo1), new DataContractJsonSerializerSettings()).WriteObject(ms1, new Foo1(42));

[DataContract]
readonly struct Foo2 { public Foo2(int a) { A = a; } [DataMember]public int A { get; } }
var ms2 = new MemoryStream();
new DataContractJsonSerializer(typeof(Foo2), new DataContractJsonSerializerSettings { SerializeReadOnlyTypes = true }).WriteObject(ms2, new Foo2(42));

//Encoding.UTF8.GetString(ms1.ToArray()) => "{}" // So with default value SerializeReadOnlyTypes=false it will never work
//Encoding.UTF8.GetString(ms2.ToArray()) => "{\"A\":42}"

For DataSerializationHelperTests instead, though I have not managed to build under VS2022 yet;

    public void DataSerializerShouldRoundTripReadOnlyData()
    {
        var source = new ReadOnlyDataWrapper { ReadOnlyData = new ReadOnlyDataType(42) };

        var actual = DataSerializationHelper.Deserialize(
            DataSerializationHelper.Serialize(new object[] { source }));

        Verify(actual.Length == 1);
        Verify(actual[0].GetType() == typeof(ReadOnlyDataWrapper));
        Verify(((ReadOnlyDataWrapper)actual[0]).ReadOnlyData.Equals(source));
    }

    public class ReadOnlyDataWrapper
    {
        public ReadOnlyDataType ReadOnlyData { get; set; }
    }

    // Should serialize fine if DataSerializationHelper.SerializerSettings.SerializeReadOnlyTypes = true
    [DataContract]
    public readonly struct ReadOnlyDataType
    {
        public ReadOnlyDataType(int a)
        {
            A = a;
        }

        [DataMember]
        public int A { get; }
    }

Expected behavior

Dynamic test data with read-only types should not go lost.

Actual behavior

Read-only types in dynamic test data lose their values.

Additional context

Providing PR for your convenience.

[aliquid]

Untested (no pun intended, rly I have not managed to build using VS2022 yet) code change and test case below for your convenience, CONTRIBUTING.md said no PRs.

https://github.com/aliquid/testfx/tree/SerializeReadOnlyTypes

[aliquid]

By the way, similar to #1067 but different - allowing serializing of read-only data types seems like a trivial improvement with no side-effects.

[Evangelink]

Hi @aliquid,

Thanks for the bug report and thank you for the fix :) Can you please open the PR so we can move forward with the fix? I will sync with the team to see if we should cherry-pick that for a 3.0.3.

CONTRIBUTING.md said no PRs.

I will update the contributing doc to be a little more precise and suggest not to create PR before approval for new features. The goal is to avoid wasting people's time if that's something we don't plan to do or plan to do differently. I think bug fixes are different.

I have not managed to build using VS2022 yet

Would you mind describing the errors you are facing with trying to build locally on your machine? I'd like to understand if that's "just" some doc that needs to be improved to help other contributes or if we have some more "complex" build issue.

[aliquid]

Hi @Evangelink,

Opened PR, and unfortunately tests show this idea is a dud. https://github.com/microsoft/testfx/pull/1543/checks?check_run_id=10493375704

While data indeed is serialized, it is not being deserialized by DataContractJsonSerializer because that neither uses the parameter ctor nor writes directly into the value type like it would if it was instead /*not readonly*/ struct with a private set; /*not used*/. Our business code uses Newtonsoft.Json.JsonConstructorAttribute on the ctor, which is how it gets deserialized for non-test purposes.

When upgrading from v2.1.1 to v3.0.2 we need to put DataContract/DataMember attributes on our dynamic test data classes if they contain readonly struct or similar, so that they fail to serialize thanks(!) to SerializeReadOnlyTypes defaulting to false.

• This way individual test cases will not show in explorer but the tests be running fine.
• Otherwise, now that individual test cases are showing with v3.0.2, the tests fail because of partially corrupt data not surviving the data contract serialize/deserialize.
• Alternatively, we could implement ISerializable including ctor(SerializationInfo info, StreamingContext context)

Suggest we reject the PR and close this bug without further action. Maybe if somebody reports a similar bug saying "Dynamic test data gets corrupt when it has readonly structs" a solution less cludgy than putting DataContract/DataMemer attributes on the data would be to more directly support TestDataSourceDiscoveryAttribute also on the class not only on the assembly.

I have not managed to build using VS2022 yet

Would you mind describing the errors you are facing with trying to build locally on your machine? I'd like to understand if that's "just" some doc that needs to be improved to help other contributes or if we have some more "complex" build issue.

On VS2022 v17.4.3 I installed .NET desktop development, Universal Windows Platform development and .NET Multi-platform App UI development (assuming that is the same as .Net Core cross-platform development in VS2017). When building getting below errors. Maybe I need to uninstall and reinstall some SDKs or something.

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS0234	The type or namespace name 'ApplicationModel' does not exist in the namespace 'Windows' (are you missing an assembly reference?)
TestFramework.Extensions (uap10.0.16299)
...\testfx\src\TestFramework\TestFramework.Extensions\Attributes\UWP_UITestMethodAttribute.cs	35	Active
Error	CS0234	The type or namespace name 'Core' does not exist in the namespace 'Windows.UI' (are you missing an assembly reference?)
TestFramework.Extensions (uap10.0.16299)
...\testfx\src\TestFramework\TestFramework.Extensions\Attributes\UWP_UITestMethodAttribute.cs	36	Active

[Evangelink]

I will have a deeper look after but we already have a couple of issues related to serialization of data during discovery (see #1462). So far the best option I see would be to use BinaryFormatter but I fear it will be flagged internally for the security issues it raises. I will sync with runtime team to see what they would suggest for this case.

[aliquid]

Keep up the good work! Let me know if I should do anything wrt this bug and the PR, feels abrupt closing from my end.

For any passersby, we came up with the following workaround simplifying what does not survive se-de-serialization. I guess this shows why we can't have good things. 😄

        public static IEnumerable<object[]> TESTDATA_COMPLEX()
        {
            yield return new object[]
            {
                "NAME OF TESTCASE",
                DATA1, DATA2, DATAn
            }, ...
        }
        public static IEnumerable<object[]> TESTDATA_SIMPLIFIED() => TESTDATA_COMPLEX().Select(x => new[] { x[0] });

        [DataTestMethod]
        [DynamicData(nameof(TESTDATA_SIMPLIFIED), DynamicDataSourceType.Method)]
        public async Task TestWithComplexTestDataAsync(string testName /*PULLING OTHER DATA TO VARIABLES BELOW*/)
        {
            var data = TESTDATA_COMPLEX().Single(x => (string)x[0] == testName);
            var DATA1 = (TYPEOFDATA1)data[1];
            var DATA2 = (TYPEOFDATA2)data[2];
            var DATAn = (TYPEOFDATAn)data[n];
        }

[Evangelink]

Thanks for the contribution and detailed explanations @aliquid. I will make sure to come back to you asap.

[Evangelink]

Closing this one, please follow #1462 to get notification about the fix.