Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip attribute on [InlineData] is not respected in Visual Studio #266

Closed
martincostello opened this issue Aug 10, 2018 · 31 comments
Closed

Comments

@martincostello
Copy link

If the Skip property of an [InlineData] attribute (or derived types of DataAttribute) is populated with a message that it should be skipped, either statically in code or at runtime, Visual Studio does not respect that value and runs the test when the Visual Studio Test Explorer is used.

A repo containing a solution that demonstrates this issue can be found here: https://github.com/martincostello/broken-inlinedata-skip-repro

This appears to be a regression since xunit 2.3.1.

Running dotnet test using the CLI is not affected:

PS C:\Coding\broken-inlinedata-skip-repro> dotnet test
Build started, please wait...
Build completed.

Test run for C:\Coding\broken-inlinedata-skip-repro\bin\Debug\netcoreapp2.1\broken-inlinedata-skip-repro.dll(.NETCoreApp,Version=v2.1)
Microsoft (R) Test Execution Command Line Tool Version 15.7.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
[xUnit.net 00:00:00.98]     BrokenInlineDataSkip.Tests.Can_Add_Integers(x: 2, y: 2, expected: 4) [SKIP]
Skipped  BrokenInlineDataSkip.Tests.Can_Add_Integers(x: 2, y: 2, expected: 4)

Total tests: 2. Passed: 1. Failed: 0. Skipped: 1.
Test Run Successful.
Test execution time: 2.2485 Seconds

xunit 2.3.1 Behaviour

xunit 2.4.0 Behaviour

@martincostello
Copy link
Author

Forgot to include version information in case it's helpful:

dotnet --info

PS C:\Coding\broken-inlinedata-skip-repro> dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   2.1.302
 Commit:    9048955601

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17134
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\2.1.302\

Host (useful for support):
  Version: 2.1.2
  Commit:  811c3ce6c0

.NET Core SDKs installed:
  2.0.3 [C:\Program Files\dotnet\sdk]
  2.1.3 [C:\Program Files\dotnet\sdk]
  2.1.100 [C:\Program Files\dotnet\sdk]
  2.1.104 [C:\Program Files\dotnet\sdk]
  2.1.200 [C:\Program Files\dotnet\sdk]
  2.1.201 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.300-rc1-008673 [C:\Program Files\dotnet\sdk]
  2.1.300 [C:\Program Files\dotnet\sdk]
  2.1.301 [C:\Program Files\dotnet\sdk]
  2.1.302 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.0-rc1-final [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.0-rc1-final [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.4 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.0-rc1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

Visual Studio

image

@bradwilson
Copy link
Member

Does this only reproduce in Visual Studio? Can you reproduce it with any other runner?

@bradwilson
Copy link
Member

Actually, I see that dotnet test is unaffected. That means this is probably a bug with source-based discovery. Can you validate that by turning source-based discovery off? If that's the case, then this bug belongs to the VSTest team, not us.

image

@martincostello
Copy link
Author

Sure I’ll try that later. Is that a feature that needs 2.4.0 to light up? Otherwise it’s a bit odd that the same version of VS would shoe different behaviours with different NuGet packages in use.

@bradwilson
Copy link
Member

I'm not sure why it's happening. I'm just looking for a way to narrow down where the issue might be.

@martincostello
Copy link
Author

OK, so I'm trying this on my home laptop (above my was my work), and first time I opened and ran it was fine. The I turned discovery off and then it started to reproduce. Turned it back on and still reproduced it. Cleaned the solution and re-ran and it was behaving correctly again. Bizarre.

It seems like it starts to happen once you rebuild the solution with the setting either way, but I'll need to play around more and restart VS in different combinations to see which scenario(s) the issue happens with.

@martincostello
Copy link
Author

So, I did the following and it reproduces immediately and consistently:

  1. Disable source-based discovery
  2. Closed Visual Studio
  3. Deleted the bin and obj folders.
  4. Open solution in Visual Studio.
  5. Build
  6. Run all tests -> 2 run
  7. Rebuild
  8. Run all tests -> 2 run

@bradwilson
Copy link
Member

This doesn't reproduce for me at all with VS 2017 15.7.6.

Source-based discovery on:

image

Source-based discovery off:

image

Also as expected, it also works from dotnet test:

image

@wouterdirks
Copy link

I have the same issue; in Visual Studio the "Skip" property is ignored but in dotnet test it works fine.

@ReneGoos
Copy link

I'm having the same issue and after some testing found the following:
If I do a Run All the Skip attribute is respected. If however I run a selected set of tests it is ignored.
We set Skip inside the customized attribute, when a test fails. If I debug the specific test case the skip value is not queried (breakpoint is not hit), when I do a Run All and afterwards Run... Debug Last Run the skip value is queried (breakpoint is hit).

@estrizhok
Copy link

@ReneGoos is correct. xUnit test adapter ignores Skip property on test-cases of a Theory (but still respects them on Facts) when running specific tests. Running all tests from assembly respects Skip property in both cases.

@rynowak
Copy link

rynowak commented Sep 11, 2019

I did a little debugging on this, and I think I understand what's going on.

Deserialization of SkippedDataRowTestCase is just broken (and possibly other derived types). Here's the call stack that makes it clear:
image

When GetSkipReason is called, the skipReason is null because of the order of these calls.
image

SkippedDataRowTestCase.Deserialize is on the stack, but we haven't yet read the skipReason because the call to base.Deserialize() happens first.

This is happening because the property setter of XUnitTestCase.Timeout is calling the Initialize() method https://github.com/xunit/xunit/blob/master/src/xunit.execution/Sdk/Frameworks/XunitTestCase.cs#L177 - which results in the call stack above. It seems like the expectation of Deserialize is that Initialize will be called after, but it's not anymore.

This change happened in the following commit, which I think is only in 2.4.1 and not earlier versions xunit/xunit@dabc047

rynowak referenced this issue in dotnet/extensions Sep 16, 2019
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
rynowak referenced this issue in dotnet/extensions Sep 20, 2019
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
rynowak referenced this issue in dotnet/extensions Sep 22, 2019
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
rynowak referenced this issue in dotnet/extensions Sep 23, 2019
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
JunTaoLuo referenced this issue in dotnet/aspnetcore Feb 12, 2020
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
\n\nCommit migrated from dotnet/extensions@cbe90b8
JunTaoLuo referenced this issue in dotnet/aspnetcore Feb 15, 2020
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782
\n\nCommit migrated from dotnet/extensions@cbe90b8
maryamariyan referenced this issue in maryamariyan/runtime Mar 11, 2020
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782


Commit migrated from dotnet/extensions@cbe90b8
maryamariyan referenced this issue in maryamariyan/runtime Mar 11, 2020
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782


Commit migrated from dotnet/extensions@cbe90b8
maryamariyan referenced this issue in maryamariyan/runtime Mar 27, 2020
We're currently experiencing a bug where conditional skips aren't working in VS.
This is caused by https://github.com/xunit/xunit/issues/1782


Commit migrated from dotnet/extensions@cbe90b8
@jzabroski
Copy link

Workaround: Don't use Visual Studio Test Explorer. JetBrains Resharper Ultimate is your friend.

@jzabroski
Copy link

@martincostello Why did you thumbs down the workaround. It actually works, and is something I didn't see explained. I found this issue because I temporarily disabled Resharper and used VS Test Explorer and ran into this problem, and it confused the heck out of me.

@martincostello
Copy link
Author

It just came across as a bit of an "X is better than Y because it doesn't have this specific problem" sort of thing, like when people argue about iOS vs. Android, Windows vs. Linux etc. 😄

@jzabroski
Copy link

jzabroski commented Jul 1, 2020

Well, I might believe that 😄, but I also believe that it's helpful to point out this may just be a screwy Microsoft bug, not a Brad Wilson/xunit issue. It might also be that Resharper is doing some hacky stuff to get this to work.

@jzabroski
Copy link

@martincostello A new workaround: With the introduction of Assert.Skip and Assert.SkipWhen, you can encode skipping via adding a parameter to your InlineData. See: xunit/xunit#1913 (comment) CC @bradwilson

https://github.com/martincostello/broken-inlinedata-skip-repro/blob/main/Tests.cs becomes:

using Xunit;

namespace BrokenInlineDataSkip
{
    public static class Tests
    {
        [Theory]
        [InlineData(1, 1, 2, false)]
        [InlineData(2, 2, 4, true)]
        public static void Can_Add_Integers(int x, int y, int expected, bool skip) => { Assert.SkipWhen(skip, "Skipped"); Assert.Equal(expected, x + y); }
    }
}

@Happypig375
Copy link

Happypig375 commented Aug 20, 2020

@bradwilson Can this hopefully be included in xunit/xunit#2133? This bug has been present for 2 years now.

@jzabroski
Copy link

@Happypig375 How does making the test self-hosting solve the problem that the IDE has bugs? Why does Resharper work but VS does not?

@Happypig375
Copy link

@jzabroski Go read @rynowak's comment and come back to me.

@jzabroski
Copy link

@Happypig375 I see. So technically, xunit moving to a tests-as-program model instead of tests-as-library model may actually exacerbate this problem if its just a data deserialization issue.

@Happypig375
Copy link

@bradwilson Can this hopefully be included in xunit/xunit#2133? I hit this bug again today.

Happypig375 referenced this issue in asc-community/AngouriMath Sep 6, 2020
@Bidthedog
Copy link

Bidthedog commented Mar 2, 2021

One of my clients is seeing this exact bug - Theory with InlineData skip is not being respected:

        [Theory]
        [InlineData("filename.jpg", "image/jpeg")]
        [InlineData("filename.jpeg", "image/jpeg")]
        [InlineData("filename.jpe", "image/jpeg")]
        [InlineData("filename.avi", "video/x-msvideo")]
        [InlineData("filename.mpg", "video/mpeg")]
        [InlineData("filename.mp2", "audio/mpeg")]
        [InlineData("filename.mp3", "audio/mpeg")]
        [InlineData("filename.mp4", "video/mp4")]
        [InlineData("filename.pptx", "application/vnd.openxmlformats-officedocument.presentationml.presentation")]
        [InlineData("filename.ppt", "application/vnd.ms-powerpoint")]
        [InlineData("filename.xlsx", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")]
        [InlineData("filename.xls", "application/vnd.ms-excel")]
        [InlineData("filename.doc", "application/msword")]
        [InlineData("filename.docx", "application/vnd.openxmlformats-officedocument.wordprocessingml.document")]
        [InlineData("filename.acc", "application/vnd.americandynamics.acc")]
        [InlineData("filename.xml", "application/xml")]
        [InlineData("filename.aiff", "audio/x-aiff")]
        [InlineData("filename.bwf", "audio/x-wav")] //
        [InlineData("filename.caf", "audio/x-caf")]
        [InlineData("filename.wav", "audio/x-wav")]
        [InlineData("filename.flv", "video/x-flv")]
        [InlineData("filename.wmv", "video/x-ms-wmv")]
        [InlineData("filename.bmp", "image/bmp")]
        [InlineData("filename.gif", "image/gif")]
        [InlineData("filename.png", "image/png")]
        [InlineData("filename.psd", "image/vnd.adobe.photoshop")]
        [InlineData("filename.tga", "image/x-tga")]
        [InlineData("filename.tiff", "image/tiff")]
        [InlineData("CANON & NIKON", "", Skip = "Need to support Canon and Nikon raw image formats")]
        public void GetMimeType_ReturnsExpectedMIMETypes_ForSupportedTypes(string filePath, string expectedMIMEType) {}

This only happens in Visual Studio Test Runner. Has never happened in dotnet test and does not happen in Resharper's test runner.

Shutting down VS, clearing the Visual Studio test cache for the solution doesn't seem to make a difference.

For completeness, the following packages are installed in the test project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="AutoFixture.AutoMoq" Version="4.15.0" />
    <PackageReference Include="AutoFixture.Xunit2" Version="4.15.0" />
    <PackageReference Include="FluentAssertions" Version="5.10.3" />
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
    <PackageReference Include="Moq" Version="4.16.0" />
    <PackageReference Include="xunit" Version="2.4.1" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="1.3.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>
...

@IT-CASADO
Copy link

This is really annoying and should be fixed not only in next major version, but also in version 2.x.

@bradwilson bradwilson transferred this issue from xunit/xunit Apr 1, 2021
bradwilson added a commit to xunit/xunit that referenced this issue Oct 6, 2021
@bradwilson
Copy link
Member

Fixed in 2.4.2-pre.14 (currently on MyGet). The serialization of this class has changed in v3 with the move to traditional .NET Serialization, so the issue is not present in v3.

@asasine
Copy link

asasine commented Oct 11, 2021

When will this be available as on nuget.org as a non-prerelease?

@bradwilson
Copy link
Member

When will this be available as on nuget.org as a non-prerelease?

Soon, but no fixed date yet.

@Issung
Copy link

Issung commented Dec 5, 2022

I am still encountering this issue with 2.4.5 has it worked its way into that release or is it still pending release?

Test
image
Test Results
image
DLL Version proof
image

@bradwilson
Copy link
Member

@Issung wrote:

I am still encountering this issue with 2.4.5 has it worked its way into that release or is it still pending release?

This should be fixed w/ xUnit.net 2.4.2. Make sure you've upgraded the core framework package(s).

@jzabroski
Copy link

Works great for me. You need to use both Xunit and Xunit.runner to be successful, though.

@Issung
Copy link

Issung commented Dec 5, 2022

Ah that is where I fell short, must also update the base xunit package, I have done that now and it works perfect, thanks for your effort on this bug!

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests