Bug(?) when using a struct with DynamicDataAttribute

[JochemPalmsens]

trafficstars

Describe the bug

Today I was looking into this StackOverflow issue I couldn't understand why this failed and blamed it on the code being incomplete. However, I could quite simply reproduce it (code at bottom). Somehow both values in the struct are initialized to the same value (Unknown). This is the default value ((Assets)0), as I modified the SO example by adding an additional enum value before Assets.BTC

I also modified the test to run on xUnit and there it ran fine.

Steps To Reproduce

Run the tests

Expected behavior

All tests pass (as xUnit does)

Actual behavior

2 tests (with the not null values) fail:

Additional context

Code:

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestProject1;

[TestClass]
public class UnitTest1
{
    [TestMethod]
    [DynamicData(nameof(MatchingEngineExceptionConstructor4Data))]
    public void MatchingEngineException_Constructor4_Test(string? message, MatchingPair? matchingPair, string expectedMessage)
    {
        if (matchingPair.HasValue)
        {
            Assert.AreEqual(Assets.Btc, matchingPair.Value.Asset1); // Fails unexpectedly
            Assert.AreEqual(Assets.Etc, matchingPair.Value.Asset2); 
        }

        // ...actual test...
    }

    public static IEnumerable<object?[]> MatchingEngineExceptionConstructor4Data
    {
        get
        {
            foreach (var message in new[] { null, "Céẞßö." })
            {
                var expectedMessage = message ?? "Error in the matching engine.";
                foreach (MatchingPair? matchingPair in new MatchingPair?[] { null, MatchingPair.Create(Assets.Btc, Assets.Etc) })
                {
                    if (matchingPair != null) { expectedMessage += $" (MatchingPair '{matchingPair}')"; }

                    yield return [message, matchingPair, expectedMessage];
                }
            }
        }
    }

    public readonly struct MatchingPair : IEquatable<MatchingPair>
    {
        private MatchingPair(Assets asset1, Assets asset2, bool validate = true)
        {
            if (validate)
            {
                InvalidMatchingPairException.ThrowIfEqualOrUnknown(asset1, asset2);
            }

            if (asset1 < asset2)
            {
                Asset1 = asset1;
                Asset2 = asset2;
            }
            else
            {
                Asset1 = asset2;
                Asset2 = asset1;
            }
        }

        public static MatchingPair Create(Assets asset1, Assets asset2)
        {
            return new MatchingPair(asset1, asset2);
        }

        internal static MatchingPair InternalCreate(Assets asset1, Assets asset2)
        {
            return new MatchingPair(asset1, asset2, false);
        }

        public Assets Asset1 { get; }
        public Assets Asset2 { get; }

        public override string ToString()
        {
            return $"{Asset1}<->{Asset2}";
        }

        public bool Equals(MatchingPair other)
        {
            return Asset1 == other.Asset1 && Asset2 == other.Asset2;
        }

        public override bool Equals(object? obj)
        {
            return obj is MatchingPair other && Equals(other);
        }

        public override int GetHashCode()
        {
            return HashCode.Combine((int)Asset1, (int)Asset2);
        }

        public static bool operator ==(MatchingPair left, MatchingPair right)
        {
            return left.Equals(right);
        }

        public static bool operator !=(MatchingPair left, MatchingPair right)
        {
            return !left.Equals(right);
        }
    }

    public enum Assets
    {
        Unknown,
        Btc,
        Etc,
    }

    public sealed class InvalidMatchingPairException : ApplicationException
    {
        internal static void ThrowIfEqualOrUnknown(Assets asset1, Assets asset2)
        {
            if (asset1 == asset2) throw new InvalidMatchingPairException();
        }
    }
}

[JochemPalmsens]

I just found this is a dupe of #905

Come on guys, this should have been a opt-in feature. You can't simply enforce things on people like this. I appreciate you providing this framework. But these kinds of things make people look for alternatives (or maybe that's a goal?). It should at least be well documented.

[SetTrend]

Here's an even smaller MRE:

using System;

namespace DynamicDataAttributeTest;

[TestClass]
public class DynamicDataTests
{
  private static readonly DateOnly _today = DateOnly.FromDateTime(DateTime.Today);
  private static readonly DateOnly _tomorrow = _today.AddDays(1);

  private static Tuple<int, DateOnly, DateOnly>[] InvalidData =>
  [
    new Tuple<int, DateOnly, DateOnly>(1, _today, _tomorrow)
  ];



  [TestMethod]
  [DynamicData(nameof(InvalidData))]
  public void DynamicDataTest(int profileId, DateOnly from, DateOnly until)
  {
    Assert.AreEqual(1, profileId);
    Assert.AreEqual(_today, from);
    Assert.AreEqual(_tomorrow, until);
  }
}

[Youssef1313]

Duplicate of https://github.com/microsoft/testfx/issues/1462. This is unfortunately generally an issue with parameterized tests where we currently need to serialize/deserialize test data.