FixtureLifeCycle not applied correctly to nested test classes

[mvdfugro]

Found during work on #3844.

Reproduction testcase:

    [TestFixture]
    [FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
    public class Nested
    {
        [TestFixture]
        public class NestedAgain
        {
            int x;

            [Test, Order(1)]
            public void Test1()
            {
                Assert.That(x++, Is.EqualTo(0));
            }

            [Test, Order(2)]
            public void Test2()
            {
                Assert.That(x++, Is.EqualTo(0));
            }
        }
    }

Expected result: Test passes
Actual result:

_localtests.Nested+NestedAgain.Test2

  Expected: 0
  But was:  1

Verified on 3.13.1 and 'latest master' (5093879)

[mvdfugro]

From #3844 (comment):

Dug into the wrapping class issue as well, but I'm not entirely sure how it's supposed to work:

• In NUnitTestFixtureBuilder.BuildFrom, fixture.ApplyAttributesToTest(new AttributeProviderWrapper<FixtureLifeCycleAttribute>(typeInfo.Type.GetTypeInfo().Assembly)); is called. This correctly applies attributes on inherited classes but doesn't so do for wrapped classes. The AttributeProviderWrapper could recurse into parent wrappers as well.

  • In the end, this is what sets the LifeCycle flag on the fixture, so this seems like a natural choice.

• TestExtensions.HasLifeCycle recurses through Test.Parent, but that is only set for tests, not for fixtures. I can't quite figure out how TestExtensions.HasLifeCycle is supposed to work -- it seems to search for the first use of 'InstancePerTestCase' rather than looking for the first defined lifecycle... but that would imply that you could never override from InstancePerTestCase back to SingleInstance.

[gleb-osokin]

Unfortunately, there is a more fundamental problem with applying the life cycle for nested fixtures due to current implementation:
The FixtureLifeCycle enum does not have any default Unset/Inherit value.
And then suppose you set FixtureLifeCycle.InstancePerTestCase at assembly level - how do we now, if we should overwrite current fixture value with the parent value?

Here's the example:

[FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
public class MainFixture
{
   [FixtureLifeCycle(LifeCycle.SingleInstance)]
   public class NestedFixture
   {
   }
}

Now, you have to check at a later time (the fixtures have already been built, so no more attributes) whether the NestedFixture.LifeCycle equals LifeCycle.InstancePerTestCase. You check the fixture - it has SingleInstance. But is it genuine SingleInstance or the one inherited from parent? We don't know and cannot make a reliable decision.

To fix this we should introduce a new value into LifeCycle enum (Unset / Default / Inherit), but that would be a breaking change, I guess.

[mvdfugro]

Hi @gleb-osokin,

I think this is fine in practice as the inheritance (of assembly level vs testcase level) is applied in TextFixtureAttribute.BuildFrom:

public IEnumerable<TestSuite> BuildFrom(ITypeInfo typeInfo, IPreFilter filter)
{
[...]
    fixture.ApplyAttributesToTest(new AttributeProviderWrapper<FixtureLifeCycleAttribute>(typeInfo.Type.GetTypeInfo().Assembly));
    fixture.ApplyAttributesToTest(typeInfo.Type.GetTypeInfo());

i.e. the assembly attributes are explicitly applied to the testcase, rather than being resolved at some later level.

I initially thought this was the role of TestExtensions.HasLifeCycle, but I that deals with traversing test.Parent which I think handles inheritance rather than nested tests (not entirely sure). The logic of that method could definitely be simplified if there had been an 'Unset' value in the enum (in which case you can just loop until you find the first one that's not 'unset').

[gleb-osokin]

@mvdfugro I guess I didn't express that in a right way: you don't even need the assembly-level attribute, just a simple nested TextFixture is enough to demonstrate the ambiguity (see my example above):
The NestedTestFixture has LifeCycle == SingleInstance, but HasLifeCycle will still traverse upwards and figure out the NestedFixture is InstancePerTestCase because that's MainFixture lifecycle.

[mvdfugro]

So.. here's where I'm confused. In the current code base, the following works as expected:

namespace Some.Namespace
{
    [FixtureLifeCycle(LifeCycle.InstancePerTestCase)]
    public class LifeCycleWithNestedOverridingFixture
    {
        [FixtureLifeCycle(LifeCycle.SingleInstance)]
        public class NestedFixture : BaseLifeCycle
        {
            private int _value;

            [Test]
            [Order(0)]
            public void Test1()
            {
                Assert.AreEqual(0, _value++);
            }

            [Test]
            [Order(1)]
            public void Test2()
            {
                Assert.AreEqual(1, _value++);
            }
        }
    }
}

In TestExtensions.HasLifeCycle, the following situation occurs:

• fixture.LifeCycle == SingleInstance (as you would expect)
• parent == TestSuite('Some.Namespace') (and not the wrapping class!) so the attribute on LifeCycleWithNestedOverridingFixture is never applied in this method
  • To make matters slightly more confusing: when running through TestBuilder.MakeFixture / TestBuilder.RunTest parent is null rather than a TestSuite.

The corresponding test case without attribute therefore doesn't work (same story: fixture.LifeCycle is SingleInstance as that's the default, and parent is null so there's no inheritance).

What I'm confused by is how ITest.Parent is supposed to work -- from what I can find in debugging I get the impression that:

• A Test (e.g. LifeCycleWithNestedOverridingFixture+NestedFixture.Test1) belongs to a Fixture (LifeCycleWithNestedOverridingFixture+NestedFixture)
• A Fixture belongs to a Suite (Some.Namespace)
• A Suite can belong to another suite (Some)
• The top-level Suite belongs to an assembly

and the parent class is not a 'parent' in the .Parent hierarchy.

So: There are currently two places where the Fixture + Assembly attributes are applied:

• TestFixtureAttribute.BuildFrom, which directly uses the attributes to first applies the Assembly attribute and then the Fixture attribute, correctly overwriting the Assembly attribute
• TestExtensions.HasLifeCycle, which uses the ITest.Parent hierarchy to iterate over the Test.LifeCycle ... except that doesn't work,. because the TestAssembly is not a TestFixture and thus has no lifecycle.

Long story short: I think

• HasLifeCycle should find the first (and only) parent Fixture, or check the immediate parent/grandparent (which is what SimpleWorkItem.MakeTestCommand does)
• Any inheritance should be handled while processing the attributes. That is already the case for the assembly, and this also seems like a natural place to handle attributes in nested fixtures (class inheritance is already handled here).

Alternatively, the internal logic that builds the .Parent hierarchy could be adapted such that there can be multiple fixture parents (i.e., we get Test -> InnerClass -> OuterClass -> Namespace).