Filtering default traits/categories in the .runsettings file

[ViktorHofer]

VS Test Explorer displays all discovered tests by default. In dotnet/runtime some tests are disabled via traits (i.e. ActiveIssue, SkipOnTargetFramework, ConditionalFact, ConditionalTheory). These tests should be filtered out by default inside the VS Test Explorer by introducing a trait filter entry in the .runsettings file. Same applies to when running dotnet test on the CLI, default traits should be recognized in the .runsettings file.

1. The .runsettings file needs a trait filter option, i.e. <TraitFilter>-failing,-IgnoreForCI</TraitFilter>
2. That option needs to be honored by the VS Test Explorer UI.

cc @ManishJayaswal @singhsarab this is part of the the design document that I created a few months ago: https://github.com/ViktorHofer/designs-private/pull/1.

[ViktorHofer]

cc @AbhitejJohn

[ManishJayaswal]

tagging @vritant24 here as well as this would also require changes in VS TE window.

[ViktorHofer]

Tracking dotnet/runtime issue: dotnet/runtime#1161

[ViktorHofer]

To provide an example of why this is necessary, in https://github.com/dotnet/runtime we attributed tests which are failing either on all or some platforms with attributes that allow excluding them by filtering on that trait: https://github.com/dotnet/runtime/blob/04f22268efb4434b7cd81e8ae2c3354be99fd4a7/src/libraries/System.Composition/tests/CircularityTests.cs#L119-L127

In this example, [ActiveIssue] transforms into the trait failing which is excluded from our CLI test runs.

This feature most certainly makes sense to be implemented in .runsettings. Possibly with one of the following syntaxes:

1. As a single property: <TraitFilter>-failing,-IgnoreForCI,+abc</TraitFilter>.
2. As a child node enumeration:

<TraitFilters>
   <TraitFilter name="include" value="abc" />
   <TraitFilter name="exclude" value="failing" />
</TraitFilters>

3. As a child node enumeration with different nodes:

<TraitFilters>
   <TraitIncludeFilter value="abc" />
   <TraitExcludeFilter value="failing" />
</TraitFilters>

[vritant24]

I've created Bug 1069896 in azdo to track the changes needed on the VS TE Window.

[nohwnd]

I was briefly looking into this yesterday, and I am thinking if it we should take advantage of the filter syntax that already exists in the test platform because that allows for multiple ways of matching (equal, not equal and contains) and pointing the filter to different properties of the test (name / category etc.).

https://github.com/microsoft/vstest-docs/blob/master/docs/filter.md

[ViktorHofer]

So basically adding the ability to specify the -Filter switch in the .runsettings file, if I understand you right? That sounds like a good idea to me as that approach is more flexible. But wouldn't that leak specific test adapter implementations into the .runsettings file?

[nohwnd]

Yes that is what I meant. And yes it would leak the details to runsettings file, but the --filter / --TestCaseFilter is already leaking those details. What it gives us is that we don't need to come up with an alternative DSL to unify the possibilities already present in --filter, and we don't need to think about possible other options that are used in other custom adapters, or future adapters.

On the other hand the user will have to provide the filters for all the test adapters that they are using, but it's the same situation for the filter parameter as well, for example, including tests in both these files is done like this dotnet test --filter "TestCategory=include|Category=include":

# file UnitTestProject29\UnitTest1.cs
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace UnitTestProject29
{
    [TestClass]
    public class UnitTest1
    {
        [TestMethod]
        [TestCategory("include")]
        public void TestMethod1()
        {
        }
    }
}

# file XUnitTestProject1\UnitTest1.cs
using System;
using Xunit;

namespace XUnitTestProject1
{
    public class UnitTest1
    {
        [Fact]
        [Trait("Category", "include")]
        public void Test1()
        {

        }
    }
}

[ViktorHofer]

Sounds good, let me know if I can help somehow. Thanks.

[nohwnd]

@vritant24 there is a PR, I marked you as a reviewer:

# file runsettings.settings
<RunSettings>
    <RunConfiguration>
        <TestCaseFilter>TestCategory=include</TestCaseFilter>
    </RunConfiguration>
</RunSettings>

Here is my project that I used to figure this out, it uses the nugets from the locally built vstest and testfx, let me know if you need any help plugging this into VS:

vstest.console --settings:runsettings.settings "C:\Projects\temp\UnitTestProject29\UnitTestProject29\bin\Debug\netcoreapp3.1\UnitTestProject29.dll"

UnitTestProject29.zip

[nohwnd]

@ViktorHofer what test framework are your tests using, on xUnit it does not run the filtering during discovery, and would require change in their adapter.

"C:\Program Files (x86)\Microsoft Visual Studio\2019\IntPreview\Common7\IDE\Extensions\TestPlatform\vstest.console.exe" /Platform:x86 /ListTests /TestCaseFilter:TestCategory=include  C:\Users\jajares\source\repos\UnitTestProject29\XUnitTestProject1\bin\Debug\netcoreapp3.1\XUnitTestProject1.dll C:\Users\jajares\source\repos\UnitTestProject29\UnitTestProject29\bin\Debug\netcoreapp3.1\UnitTestProject29.dll
Microsoft (R) Test Execution Command Line Tool Version 16.6.0-dev
Copyright (c) Microsoft Corporation.  All rights reserved.

The following Tests are available:
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.0 (32-bit .NET Core 3.1.2)
[xUnit.net 00:00:00.44]   Discovering: XUnitTestProject1
[xUnit.net 00:00:00.50]   Discovered:  XUnitTestProject1
    XUnitTestProject1.UnitTest1.IncludedXunitTest
    XUnitTestProject1.UnitTest1.ExcludedXunitTest <-- this should have been filtered out
    IncludedMsTest <- this is mstest, and it correctly lists the test that passes the filter

[ViktorHofer]

Yes we use xunit. cc @onovotny

[clairernovotny]

The filtering happens in the run phase currently:
https://github.com/xunit/visualstudio.xunit/blob/master/src/xunit.runner.visualstudio/VsTestRunner.cs#L533-L536

I honestly don't remember why we don't/didn't do it in discover. @bradwilson?

[bradwilson]

If you filter during discovery, make sure it happens during discovery for the purposes of execution (which is how tests run in VSTest at least 99.9% of the time).

[bradwilson]

Looking at the blame, it looks like it's probably just a case of the filter being added in one place where it should've been added in two. And not surprisingly, it was added to the place where it made the 99.9% case work properly. 😉

[clairernovotny]

Thanks @bradwilson. That being the case, I should be able to put the logic in the discover phase as well and get a 2.4.2 build of the vs test adapter out. Then all kinds of templates would need to be updated.

[nohwnd]

@onovotny Great, I'll take care of updating our templates in https://github.com/dotnet/test-templates once this is done and tested.

[ViktorHofer]

@onovotny please let us know if we can help you in any way to speed up the process.

[clairernovotny]

A PR would help speed things along - my plate is pretty full and I'm not sure when I'll be able to get to it.

[nohwnd]

@clairernovotny I will consider PR to xUnit it once we get this work completely. MSTest works so we can use that as the pilot for the whole functionality, because the changes to the platform need to ship with VS, but xunit / nunit / mstest can be updated via nuget.

[ViktorHofer]

Jakub, just to understand this better, don't the changes necessary here also ship via nuget?

[nohwnd]

@ViktorHofer this will impact both source based test analysis which ships directly with VS and the vstest.console that is also shipping with VS. So the changes will have to be inserted into VS once we are done. You can still get vstest console separately, but if you are will be using this from VS Test Explorer you need to wait till it ships with VS.

If this was just impacting test host, or some of the test adapters (like xunit adapter) you could get it by updating your nuget package.

#2287 (comment) this and the subsequent comments will hopefully give you some more details about how this ships.

[ViktorHofer]

Thanks a lot for clarifying.

[nohwnd]

This still needs the xUnit part. Re-opening.

[nohwnd]

xUnit PR created, will continue discussion there. This is done for vstest, closing.