Custom XUnit test bits aren't used/run by the test runner

[NTaylorMullen]

Description

Us on the ASP.NET team are attempting to use the new VSTest infrastructure and have found that it doesn’t respect custom test attributes. In particular we have a ConditionalFactAttribute that we look for to enable custom skip conditions when running tests cross platform.

What I've noticed is that the Test Explorer in VS 2017 picks up the existence of the Test but cannot run the test (even when trying to forcefully run it). Also, attempting to run tests at command line via dotnet test results in the tests not being run (don't exist in pass, fail or skip).

Steps to reproduce

• Download the following TestReproProject.zip.
• cd next to the csproj.
• dotnet restore
• dotnet test

Expected behavior

2 test failures.

Actual behavior

Microsoft (R) Test Execution Command Line Tool Version 15.0.0.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
Failed   XUnitTestProject1.UnitTest1.Test2
Error Message:
 Assert.True() Failure

Expected: True
Actual:   False
Stack Trace:

Total tests: 1. Passed: 0. Failed: 1. Skipped: 0.

Environment

.NET Command Line Tools (1.0.0-preview4-004233)

Product Information:
 Version:            1.0.0-preview4-004233
 Commit SHA-1 hash:  8cec61c6f7

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Users\nimullen\AppData\Local\Microsoft\dotnet\sdk\1.0.0-preview4-004233

<PackageReference Include="Microsoft.NETCore.App" Version="1.0.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.0.0-preview-20161123-03" />
<PackageReference Include="xunit" Version="2.2.0-beta4-build3444" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.2.0-beta4-build1194" />
<PackageReference Include="Microsoft.AspNetCore.Testing" Version="1.2.0-*" />

/cc @Eilon @muratg @DamianEdwards

[NTaylorMullen]

Debugged through our code and we're setting skip conditions appropriately. Just doesn't look like it's handled by the VStest infrastructure.

[codito]

This is likely an issue with xunit adapter (https://github.com/xunit/visualstudio.xunit). I don't see any exception in host logs. Xunit adapter discovers the tests correctly, execution is incorrect.

Will debug thru the xunit adapter and update.

[codito]

During execution, xunit is unable to deserialize SkipReasonTestCase. It fails with following stack trace:

{System.InvalidOperationException: Could not de-serialize type 'XUnitTestProject1.SkipReasonTestCase' because it lacks a parameterless constructor.
   at Xunit.Serialization.XunitSerializationInfo.DeserializeSerializable(Type type, String serializedValue) in C:\BuildAgent\work\cb37e9acf085d108\src\common\XunitSerializationInfo.cs:line 210
   at Xunit.Sdk.SerializationHelper.Deserialize[T](String serializedValue) in C:\BuildAgent\work\cb37e9acf085d108\src\common\SerializationHelper.cs:line 37
   at Xunit.Runner.VisualStudio.TestAdapter.VsTestRunner.Deserialize(LoggerHelper logger, ITestFrameworkExecutor executor, TestCase testCase)}

Because of this only Test2 is run and Test1 is ignored.

Adding a parameterless constructor goes further, but fails in SkipReasonTestCase.Deserialize since _wrappedTestCase is not available.

SkipReasonTestCase requires a fix. May be it could inherit from XunitTestCase, and override RunAsync to provide a choice over test case runner based on isTheory property. Serialize needs to ensure isTheory is serialized.

[NTaylorMullen]

@codito I'm not sure I follow. Are you suggesting that Xunit changes? This wasn't a problem in the old project.json world, what changed? I'm looking at the git histories of all the files in question and they've all been unchanged (the problem lines) for a very long time.

[NTaylorMullen]

Found the reason as to why it worked in the old world. We weren't required to serialize/deserialize stuff so we didn't see that it was broken. Digging a little deeper to find the fix.

[codito]

TestMethodTestCase may help, it implements serialization for data required during execution.

[NTaylorMullen]

Found a fix and have a PR out on our end. Closing this. Thanks for the investigation @codito!