InlineData and implicit conversions #1742
Comments
|
Hey, guys! I've found similar behaviour in some tests of mine, so I decided to investigate this. DescriptionIt looks like on xUnit 2.4.0.4010 this works well, at least Resharper isn't hanging. The error is the same as reported by @FantasticFiasco. But if I take 2.3.1 — everything will freeze. And I actually couldn't be able to start any other tests before shutting down I've also found that there are other cases when ReSharper hangs, for example: [Theory]
[InlineData(double.MaxValue)]
public void Test(decimal a) { }Which in, as I mentioned above, breaks in 2.3.1 but works in 2.4.0.4010 as: So it looks like any exception during some prewarm phase can cause freeze no matter of concrete semantics. Good news are that it is probably already fixed in 2.4.0. Versions
|
|
@Joshualight Thanks for the information! I don't think it's the same problem as the original (in your case, the values "look" convertible to one another once they've been serialized, but don't fit in the destination data type). |
|
Maybe I'm wrong, but I think in both cases we have exceptional situtations:
They should be reported as they are (and they're reported in some cases), but it looks like sometimes both exceptions kinda stuck in the throat of ReSharper runner. )) It can be worth considering to check whether any other exception can hang an environment. |
|
The original bug here (1) is an actual bug. Your new issue (2) is by design (that is supposed to throw an exception).
If you find anything which does, please open a new issue. This one is not about exceptions hanging environments, but rather about a feature that should work, that isn't working. |
|
Ah now I see, thanks! |
Hi, |
|
Hi All, I've had a quick look and I think the problem lies in the CanSerializeObject method of the XUnitSerializationInfo class. I wonder if adding code similar to below would help solve the problem. What do you think? using System;
using System.Linq;
using System.Reflection;
using Shouldly;
using Xunit;
namespace XunitIssue1742
{
public class DetectImplicitConversion
{
public class Id
{
public Id(string value)
{
Value = value;
}
public string Value { get; }
public static implicit operator Id(string id)
{
return id != null ? new Id(id) : null;
}
public static implicit operator string(Id id)
{
return id?.Value;
}
}
public class NoImplicitConversions
{
}
[Fact]
public void DetectsImplicitConversionFromString()
{
var otherType =
typeof(string);
HasImplicitConversionFromOtherType<Id>(otherType)
.ShouldBeTrue();
HasImplicitConversionFromOtherType<NoImplicitConversions>(otherType)
.ShouldBeFalse();
}
[Fact]
public void DetectsImplicitConversionToString()
{
var otherType =
typeof(string);
HasImplicitConversionToOtherType<Id>(otherType)
.ShouldBeTrue();
HasImplicitConversionToOtherType<NoImplicitConversions>(otherType)
.ShouldBeFalse();
}
/// <summary>
/// Based on https://stackoverflow.com/questions/32025201/how-can-i-determine-if-an-implicit-cast-exists-in-c/32025393#32025393
/// </summary>
private static bool HasImplicitConversionFromOtherType<T>(Type otherType) where T : class
{
var typeToConvertTo = typeof(T);
return typeToConvertTo.GetMethods(BindingFlags.Public | BindingFlags.Static)
.Where(mi => mi.Name == "op_Implicit" && mi.ReturnType == typeToConvertTo)
.Any(mi =>
{
ParameterInfo pi = mi.GetParameters().FirstOrDefault();
return pi != null && pi.ParameterType == otherType;
});
}
private static bool HasImplicitConversionToOtherType<T>(Type otherType) where T : class
{
var typeToConvertFrom = typeof(T);
return typeToConvertFrom.GetMethods(BindingFlags.Public | BindingFlags.Static)
.Where(mi => mi.Name == "op_Implicit" && mi.ReturnType == otherType)
.Any(mi =>
{
ParameterInfo pi = mi.GetParameters().FirstOrDefault();
return pi != null && pi.ParameterType == typeToConvertFrom;
});
}
}
} |
|
I believe the custom serialization system is being removed in v3 (and there are no more planned releases for v2), so this would become unnecessary. |
|
OK, cool. I couldn't find a release roadmap. Do you know when v3 is scheduled for release? |
|
As a workaround for unit test. I have added support of IXunitSerializable to help XUnit to build the correct test case data. public sealed record CountryCode(string Value) : IXunitSerializable
{
// It's required by IXunitSerializable
public CountryCode(): this(string.Empty) { }
public override string ToString()
{
return Value;
}
public static implicit operator CountryCode(string self)
{
return new CountryCode(self);
}
public static implicit operator string(CountryCode self)
{
return self.Value;
}
void IXunitSerializable.Serialize(IXunitSerializationInfo info)
{
info.AddValue(nameof(Value), Value, typeof(string));
}
void IXunitSerializable.Deserialize(IXunitSerializationInfo info)
{
throw new NotSupportedException("This should not be used.");
}
} |
|
@kolbasik You're throwing in Deserialize. That would never work in Test Explorer (where serialization comes into play). |
|
Hi, @bradwilson. You might be right that in some cases it might not work. <Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net7.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.1" />
<PackageReference Include="xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
</ItemGroup>
</Project>
using Xunit;
using Xunit.Abstractions;
namespace XUnitRecordsDemo
{
public sealed class XUnitTest
{
[Theory]
[InlineData("BE")]
[InlineData("NL")]
[InlineData("LU")]
public void ShouldAcceptRecordsAsArguments(CountryCode countryCode)
{
Assert.NotNull(countryCode);
}
}
public sealed record CountryCode(string Value) : IXunitSerializable
{
// It's required by IXunitSerializable
public CountryCode(): this(string.Empty) { }
public override string ToString()
{
return Value;
}
public static implicit operator CountryCode(string self)
{
return new CountryCode(self);
}
public static implicit operator string(CountryCode self)
{
return self.Value;
}
void IXunitSerializable.Serialize(IXunitSerializationInfo info)
{
info.AddValue(nameof(Value), Value, typeof(string));
}
void IXunitSerializable.Deserialize(IXunitSerializationInfo info)
{
throw new NotSupportedException("It should not be used.");
}
}
}Using cli
Using ReSharper Unit Test Session
Using Visual Studio 2022 Test Explorer
I would be glad if it would help anyone. |
|
Hi all, I want to resurrect this just because it was a huge pain to try and debug. I accidentally relied on an implicit cast without knowing the side effects. I used a
The only way I managed to track this down is by disabling every theory and then reenabling them one-by-one. I'm mainly writing this because searches for the problem led me to this issue and I'm hoping documenting this will help someone else. |
|
I did some debugging and found out that the cause of this bug is as follows:
The fix is somewhat complex but I would suggest two things:
@bradwilson Worth a fix? I would try to implement it. |
…rProvider fails.
…iptorProvider fails.
|
I'd be curious to know whether this still reproduces on v3, and if not, then I would just leave this as-is for v2, as we're winding down work on that. |
|
I did some digging. The exception in the computation of the I already made a PR (#3067) for a mitigation of the error for v2 (slightly over-eager 😅). |
Expected Behavior
dotnet testshould pass ifdotnet xunitpassActual Behavior
dotnet xunitwill successfully run the test whiledotnet testwill report the following error, which actually doesn't describe the real problem, i.e. that the implicit conversion for some reason cannot be handled.Steps to Reproduce the Problem
dotnet new xunitdotnet test. You will see the cryptic error messagedotnet xunit. You will see that the test passVersions
The text was updated successfully, but these errors were encountered: