[Ignore] on base classes does not work on derived classes.

[ghost]

In version 3.5.2 the [Ignore] attribute on base classes does no work on the derived classes.

Example:

    [TestClass]
#if !DEBUG
    [Ignore]
#endif
    public abstract class DebugTestBase
    {
       ...
    }

  [TestClass]
#if !RELEASE
    [Ignore]
#endif
    public abstract class ReleaseTestBase
    {
       ...
    }

[TestClass]
public class MyDebugTest1 : DebugTestBase
    {
       ...
    }

[TestClass]
public class MyReleaseTest1 : ReleaseTestBase
    {
       ...
    }

The expected behavior is that in RELEASE builds only unit tests deriving from ReleaseTestBase would be executed and in DEBUG builds only unit tests deriving from DebugTestBase.

In version 3.4.3 the behavior was exactly like expected but now we switched to version 3.5.2 and all unit tests are executed regardless which build configuration is used and regardless which is the base class of the unit test.

AB#2299814

[Evangelink]

Hi @Bruhst,

Would it be possible to get a full reproducer? I have tried with the following code:

TestProject1.csproj:

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

	<PropertyGroup>
		<TargetFramework>net8.0</TargetFramework>
		<LangVersion>latest</LangVersion>
		<ImplicitUsings>enable</ImplicitUsings>
		<Nullable>enable</Nullable>
	</PropertyGroup>

	<ItemGroup>
		<PackageReference Include="MSTest" Version="3.4.3" />
	</ItemGroup>

</Project>

Test1.cs:

namespace TestProject1
{
    [Ignore]
    [TestClass]
    public abstract class Test1
    {
        [TestMethod]
        public void TestMethod1()
        {
        }
    }

    [TestClass]
    public class Test2 : Test1
    {
        [TestMethod]
        public void Test1()
        {
        }
    }
}

and I do see the 2 tests executed in 3.4.3

Passed! - Failed: 0, Passed: 2, Skipped: 0, Total: 2, Duration: [521ms]

[pageyboy]

Hi @Evangelink, I ran into this issue today when upgrading from 3.2.2 to 3.6.1. I've traced it back to a change between 3.4.3 and 3.5.0. I've created a minimum working example here: https://github.com/pageyboy/minimum-example. As a bit of background we generally order tests of our classes by nesting test classes within classes e.g.:

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace Example.Tests
{
    [TestClass]
    [Ignore]
    public class SourceClassTests
    {
        [TestMethod]
        public void Constructor_Tests()
        {
            Assert.Fail(); // Skipped in all versions
        }

        [TestClass]
        public class MethodWithinSourceClass : SourceClassTests
        {
            [TestMethod]
            public void Should_DoSomething()
            {
                Assert.Fail(); // Fails from TestFramework 3.5.0
            }
        }
    }
}

I had previously thought that we might move away from this structure for other reasons but up until 3.5.0 the [Ignore] attribute on the SourceClassTests would prevent any of these tests executing. From 3.5.0 the nest test method now fails.

[Evangelink]

I can reproduce. I wasn't aware of this behavior so we will need to do the following:

1. Fix it
2. Document the behavior on doc + code

Additionally, we could add a property/ctor deciding if the behavior should be inherited or not as it doesn't feel natural to me.

[Evangelink]

Hi there,

We have done extended testing of various scenarios in the past versions to try to have an exact idea of the scope and we discovered that the feature is rather unintuitive and non deterministic in its behavior (depends if the class has test methods, depends if the class is an inner class, position of the inner class compared to outer class methods...).

I would like to understand the exact scope of your request. In your view:

1. Do you expect the [Ignore] to be ignoring the class it is applied to AND the inner classes:

[TestClass]
[Ignore]
public class MyTestClass
{
    // Some test methods or not

    [TestClass]
    // Inheritance doesn't matter,
    // this class is ignored because part of an outer class being ignored.
    public class InnerClass     
    {
    }
}

2. Do you expect the class to be ignored based on inheritance?

[TestClass]
[Ignore]
public class MyTestClass
{
    // Some test methods or not
}

[TestClass]
// this class is ignored because it inherits a class being ignored.
public class OtherClass : MyTestClass
{
}

3. or do you expect to have the combination of the 2, e.g.:

[TestClass]
[Ignore]
public class MyTestClass
{
    // Some test methods or not

    [TestClass]
    // this class is NOT ignored
    public class InnerClass     
    {
    }

    [TestClass]
    // this class is ignored because inner + inherit
    public class InnerClass2 : MyTestClass
    {
    }
}

Without trying to impact your call, here is my 2cts on the 3 cases above:

For case 1, I see some functional value because the outer test class can be seen as a container and so ignoring it, skips everything that belongs to this group.

For case 2, I don't like it because this seems like something unexpected and that would most of the time feels counter intuitive as I don't see the base class and cannot easily understand it's ignored which would lead me to some confusion as to why my tests are not running.

For the case 3, I don't see the value, going there I would prefer to have only case 1.

FYI the global consensus in the team was to say this is not a bug and we could consider a feature request for case 1 but not everyone was thinking it's an interesting feature.

[pageyboy]

Hi @Evangelink, I appreciate the time you've taken to look into this. Case 1 for me summarises the request most appropriately. I would expect the inner classes to be ignored rather than ignored based on inheritance, which might become a bit opaque as to why tests are not running. We only include the base class(es) due to legacy nomenclature and this could, and probably should, be dropped. Although possibly not a bug as the 'feature' was never intended, it has become a small blocker to upgrading to later versions for users in my position because of the work required to restructure. Again, appreciate the response. Cheers Chris

[Evangelink]

We will see how complex of a change it is, if low complexity I'll keep it for 3.7 that should happen early/mid December.

Hopefully some early Christmas gift for you 🎁

[nohwnd]

Can option 4. be also considered, please? Not inheriting it at all. And only fixing the bug with Ignore being applied based on whether parent class has test method.

From the investigations we did that was the closest to 2.x.x behavior, and seems the most transparent, and least error prone (I misplace my } and now my tests don't run.)

Or as a compromise adding Parameter to Ignore to allow scenario 1.

[Evangelink]

Can option 4. be also considered, please? Not inheriting it at all. And only fixing the bug with Ignore being applied based on whether parent class has test method.

It's really randomic and I would have hard time explaining why having tests declared or not would change the behavior which is IMO a good indicator of a bad design.

From the investigations we did that was the closest to 2.x.x behavior, and seems the most transparent, and least error prone (I misplace my } and now my tests don't run.)

Yes and no. It depends on ordering too.

Or as a compromise adding Parameter to Ignore to allow scenario 1.

I was considering it but I am not sure if the added complexity is worth it. I also think that option 1 is more aligned to how we are envisioning tests as trees.

Today TE shows:

But IMO the proper representation of this case should be

and in this case, I would definitely expect the [Ignore] attribute to apply to the group.

To be entirely safe, I think we can consider a global option in runsettings/json to have a global opt-out.

[nohwnd]

I think you answered my questions from some other perspective that I mean them.

My question was: Can you also list option 4.? Ignore applies directly on class where it is specified. It does not apply to inherited classes or inner classes. Ignore applies only to method on which it is specified, if ignored method is inherited from parent class it will be ignored.

This would be an easy to explain, and predictable behavior to me. This behavior runs as many tests as possible without "silently" ignoring inherited or nested classes.

What are all the bugs (order of methods / parent having some method etc.) that need to be fixed to get to this state is not relevant at the moment.

[nohwnd]

[TestClass]
[Ignore]
public class MyTestClass
{
    // Some test methods or not

    [TestClass]
    // this class is NOT ignored
    public class InnerClass     
    {
    }

    [TestClass]
    // this class is NOT ignored
    public class InnerClass2 : MyTestClass
    {
    }
}

[TestClass]
// this class is NOT ignored
public class ChildClass : MyTestClass
{
}

[pageyboy]

Hi @Evangelink, Is feedback still required from me or anyone else? Just making sure this doesn't get closed automatically. Thanks Chris

[Evangelink]

Hi @pageyboy,

Nope sorry I forgot to remove the label.

[Youssef1313]

Leaving a small note:

If we go with considering nested classes ignored if the outer class is ignored, we should also consider if we want a similar behavior for other attributes such as DeploymentItemAttribute, TestPropertyAttribute, and TestCategoryBaseAttribute.

[Youssef1313]

Closing as low priority.