[Explicit] does not get overridden if there is another category exclude

[Kuno Meyer]

Steps to reproduce:
   - Test code below ([1])
   - nunit-console.exe <assemby.dll> /include=T /exclude=B"

Expected result:
   - TestA() is executed

Actual result:
   - No test is executed

Remarks
   - Everything works as expected, if you remove the [Expected] attribute

[1] Test case

    [TestFixture, Explicit, Category("T")]
    public class Test
    {
        [Test, Category("A")]
        public void TestA() { Console.WriteLine("-A-"); }
        [Test, Category("B")]
        public void TestB() { Console.WriteLine("-B-"); }
    }

[Kuno Meyer (kuno-meyer)]

(Happens on: NUnit 2.5.3)

[PA (peter-arius-deactivatedaccount)]

Using NUnit 2.5.5. I have a use case that is probably related:

[TestFixture, Category("cat1")]
public class F1
{
  [Test]
  public void NormalTest() {}

  [Test, Category("stress")]
  [Explicit("Stress tests shall only be run explicitly")]
  public void StressTest() {}
}

[TestFixture, Category("cat2")]
public class F2
{
  [Test]
  public void NormalTest() {}

  [Test, Category("stress")]
  [Explicit("Stress tests shall only be run explicitly")]
  public void StressTest() {}
}

With /include=stress one can run all stress tests, but if one tries to restrict the stress tests like /include=F1+stress, no test is performed.

Best regards,
Peter

[PA (peter-arius-deactivatedaccount)]

oops, that should have been /include=cat1+stress instead of /include=F1+stress

[Charlie Poole (charlie.poole)]

Hi Peter,

Your case is not the same snce no ExplicitAttribute is involved.

You're just misunderstanding what cat1+stress means, i.e....

   "Run only tests that have both cat1 and stress categories"

What you probably want is /include=cat1|stress

Admittedly, this is a bit unintuitive but it comes from our being unable to use
an unescaped & on the command line.

Charlie

[PA (peter-arius-deactivatedaccount)]

Hi Charlie,

concerning #4:

But there is an [Explicit] on both StressTest() methods in my example 8-).
And yes, I meant that "+" means "and". I hope can make my point more
understandable:

The use case is "I want to run [Explicit] stress tests, but to narrow down the
selection".

Currently, one can neither achieve this with /include=stress+cat1,
nor with /include=stress /run=MyNamespace.F1.

/include=cat1+stress should perform tests having both categories, which is
F1.StressTest() (as F1 has cat1, F1.StressTest() has cat1 from its parent F1 and
stress on its own).

Without [Explicit] on StressTest() methods, it works as expected:
F1.StressTest() is performed.

When selecting /include=stress, it works as expected, too: F1.StressTest() and
F2.StressTest() are performed (selection by category beats [Explicit]).

The point is that F1.StressTest does not have cat1 directly attached, but just
indirectly from its parent. With /include=cat1+stress, it appears that
"F1.StressTest has cat1 *from parent* and has stress on its own" is not enough
to run the [Explicit] method.

If one attaches cat1 and stress directly to F1.StressTest(), it's OK, too.

AFAICS, the reason lies in AndFilter, which produces following call tree:

AndFilter.Pass(test) // test == F1.Stress() TestMethod
    +> CategoryFilter.Pass(test) // "cat1"
        +> TestFilter.Pass(test)
            +> CategoryFilter.Match(test) // "cat1"
            <- true
            or ...
        <- true
    <- true
    and
    +> CategoryFilter.Pass(test) // "stress"
        +> TestFilter.Pass(test)
            +> CategoryFilter.Match(test) // "stress"
            <- false
            or
            +> TestFilter.MatchParent(test)
                +> test.RunState != RunState.Explicit
                <- false
            <- false
            or
            +> TestFilter.MatchDescendant(test)
            <- false
    <- false
<- false

So, the "and" operation comes on top of the "Match(test) || MatchParent(test)
|| MatchDescendant(test)".

My first guess is that one could evaluate the AndFilter.Pass like:

if (test.RunState == RunState.Explicit)
{
    + require at least one filter to Match exactly
    + require other filters to Match||MatchParent||MatchDescendant
}
else
{
    + normal and operation
}

but to me it's not obvious how to do this properly, and what all the side
effects would be.

Of course, this is not the same case Kuno brought up, but I think it's
related because both deal with:

+ What does "explicitly selected" mean exactly?
+ especially with complex filters (AndFilter, AndFilter+NotFilter)
+ especially when some categories are on the parent, some on the children.

Best regards,
    Peter

[Charlie Poole (charlie.poole)]

Hi Peter,

Of course, you're right about having Explicit - I must have blinked (twice).

Regarding the category selection part of the problem, the crux of the issue
is this...

> /include=cat1+stress should perform tests having both categories, which is
> F1.StressTest() (as F1 has cat1, F1.StressTest() has cat1 from its parent F1 and
> stress on its own).

As implemented, categories filtering doesn't work that way by design. The
and filter was written to avoid matching multiple categories unless they
actually appeared on the same test. The change you suggest wouldn't
be a correction, but a new implementation.

Of course, it might be a better implementation, but the key question will be
whether it introduces breaking behavior for existing tests - at least
for NUnit 2.x.
I'll think about this as I work on the bug.

Charlie

On Tue, Jul 13, 2010 at 2:02 AM, Peter Arius <email address hidden> wrote:
> Hi Charlie,
>
> concerning #4:
>
> But there is an [Explicit] on both StressTest() methods in my example 8-).
> And yes, I meant that "+" means "and". I hope can make my point more
> understandable:
>
> The use case is "I want to run [Explicit] stress tests, but to narrow down the
> selection".
>
> Currently, one can neither achieve this with /include=stress+cat1,
> nor with /include=stress /run=MyNamespace.F1.
>
> /include=cat1+stress should perform tests having both categories, which is
> F1.StressTest() (as F1 has cat1, F1.StressTest() has cat1 from its parent F1 and
> stress on its own).
>
> Without [Explicit] on StressTest() methods, it works as expected:
> F1.StressTest() is performed.
>
> When selecting /include=stress, it works as expected, too: F1.StressTest() and
> F2.StressTest() are performed (selection by category beats [Explicit]).
>
> The point is that F1.StressTest does not have cat1 directly attached, but just
> indirectly from its parent. With /include=cat1+stress, it appears that
> "F1.StressTest has cat1 *from parent* and has stress on its own" is not enough
> to run the [Explicit] method.
>
> If one attaches cat1 and stress directly to F1.StressTest(), it's OK,
> too.
>
> AFAICS, the reason lies in AndFilter, which produces following call
> tree:
>
> AndFilter.Pass(test) // test == F1.Stress() TestMethod
>    +> CategoryFilter.Pass(test) // "cat1"
>        +> TestFilter.Pass(test)
>            +> CategoryFilter.Match(test) // "cat1"
>            <- true
>            or ...
>        <- true
>    <- true
>    and
>    +> CategoryFilter.Pass(test) // "stress"
>        +> TestFilter.Pass(test)
>            +> CategoryFilter.Match(test) // "stress"
>            <- false
>            or
>            +> TestFilter.MatchParent(test)
>                +> test.RunState != RunState.Explicit
>                <- false
>            <- false
>            or
>            +> TestFilter.MatchDescendant(test)
>            <- false
>    <- false
> <- false
>
> So, the "and" operation comes on top of the "Match(test) || MatchParent(test)
> || MatchDescendant(test)".
>
> My first guess is that one could evaluate the AndFilter.Pass like:
>
> if (test.RunState == RunState.Explicit)
> {
>    + require at least one filter to Match exactly
>    + req...

[Charlie Poole (charlie.poole)]

Postponing this bug to another release because it needs discussion.

As implemented, test cases act as if they "inherited" categories from the fixture _unless_ they have some other category specified. We may want to change this design, but it will take a bit of carefful thinking.

Let's do that on the nunit-discuss list.

[Kuno Meyer (kuno-meyer)]

Then there is still a bug left, since running my test case above (leaving away the [Explicit] attribute) with "/include=T /exclude=B" still executes TestA(), which should be (according your statement) only be marked with category name "A", but not with category name "T".

Personally, I think that this overriding-instead-of-inheriting strategy of category names is bad behavior and very unintuitive. With assigning category names you make statements about individual aspects of a test/test fixture. These statements are normally not exclusive. (For example: "This fixture needs external hardware", "This test is very time consuming", "This is an entry-level test", ...). On the other hand, you can of course always argue with splitting up the fixture into several smaller ones and into different assemblies.

[Charlie Poole (charlie.poole)]

OK, I see it now. The problem is that we want NotFilter to work differently in different situations. /exclude:B all by itself should not cause Explicit tests to be run. Nor should /include:-B. But /include:T /exclude:B or /include:T-B needs to examine the explicit cases under the Not.

As a fix for now I'm putting in a property that tells NotFilter whether it's at the top level. I don't much like this, but it's expedient.

This entire area needs to be re-examined, since the feature set "just grew" without a lot of prior planning.

Charlie

[Charlie Poole (charlie.poole)]

The supplied fix only deals with the anomalous behavior where Explicit is involved. The overall pattern of how tests are selected using categories is not changed and may need to be discussed further on the nunit-discuss list in order to come up with a direction for NUnit 3.0.