xUnit1024 fired even if one of the overloads is a private helper method

[jasonmalinowski]

If I install xunit.analyzers 0.5.0 and write:

    public class X
    {
        [Fact]
        public void Test()
        {
            Test(42, 42);
        }

        private void Test(int x, int y)
        {
            Assert.Equal(x, y);
        }
    }

xUnit1024 ('test method has same name as another method') is fired on the method, despite that the second method isn't a test. If I disable analyzers, build this library, and run it under the console runner, I do see the test is ran:

xUnit.net Console Runner (64-bit .NET 4.0.30319.42000)
  Discovering: TestXunit
  Discovered:  TestXunit
  Starting:    TestXunit
  Finished:    TestXunit
=== TEST EXECUTION SUMMARY ===
   TestXunit  Total: 1, Errors: 0, Failed: 0, Skipped: 0, Time: 0.138s

It seems here the analyzer is being overly aggressive and I suspect shouldn't be flagging in this case.

Happy to contribute the fix if somebody who knows how this Should Work™ can confirm my gut.

[marcind]

That analyzer was added to catch this issue. Looks like TestMethodCannotHaveOverloads should filter out private (and maybe protected?) methods.

[jasonmalinowski]

That's my guess. Do we know if there's a formal statement of what is unsupported other than @bradwilson's statement on that issue? It's a bit imprecise for me to fix (although I can make a guess...)

[marcind]

Looking at the descriptor comment I put in here it seems like at least for some runners even non-public overloads might be a problem (likely because some runners can execute non-public tests).

However, I do not remember which runners might be problematic.

Or the comment might just be plain wrong.

[marcind]

I don't have the hardware right now to figure out what conventions xunit enforces, but maybe @bradwilson can comment or point out the place in the source code where they are encoded.

[bradwilson]

The rule as it stands is correct. Test methods can be private. If you were to attempt to run this code in the Visual Studio test runner, you would find that it fails. The fact that it only sometimes fails in runtime (depending on the runner) is precisely why this rule is important.

[jasonmalinowski]

OK, then this is by-design, and another Roslyn test problem found!

[jeroenlandheer]

If the rule is correct, the documentation here is not: https://xunit.net/xunit.analyzers/rules/xUnit1024.html

This is listed as "Does not violate".

using Xunit;

public class xUnit1024
{
    [Fact]
    public void TestMethod() { }

    private void TestMethod(int age) { }
}

@bradwilson @jasonmalinowski

[bradwilson]

Thanks, you're correct that the documentation was wrong. I've updated the page.

Commit: e8ba183
Page: https://xunit.net/xunit.analyzers/rules/xUnit1024