Issafalcon / neotest-dotnet

Neotest adapter for dotnet
MIT License
71 stars 28 forks source link

Add support for test cases that uses a derived attribute #30

Closed X9VoiD closed 1 year ago

X9VoiD commented 1 year ago

Reproduction

dotnet new xunit -n DerivedFactXunit

UnitTest1.cs

namespace DerivedFactXunit;

public sealed class DerivedFactAttribute : FactAttribute
{
}

public class UnitTest1
{
    [DerivedFact]
    public void Test1()
    {

    }

    [DerivedFact]
    public void Test2()
    {

    }
}

Run just Test1().

Expected behavior

Runs only Test1().

Observed behavior

No test cases ran.

Changing Test2 to use [Fact] and then running Test1 made it so it ran all test cases of the class but no marker on Test1.

image

Why this should be fixed?

I override Fact.Timeout on a derived attribute to disable timeout when a debugger is attached. Really helpful when debugging asynchronous tests.

public sealed class AsyncFactAttribute : FactAttribute
{
    private int _timeout;

    public override int Timeout
    {
        get => Debugger.IsAttached ? 0 : _timeout;
        set => _timeout = value;
    }
}
Issafalcon commented 1 year ago

Thanks for raising this. I had this in the back of my mind when do the first bits of work on this adapter. I originally went for the approach of using the omnisharp LSP to discover the test positions in the tree, which would have caught the things with custom attribute names, but it caused far more issues that it solved. This is why I ended up sticking with the tree-sitter approach for discovering tests, which is a lot more reliable. However, this does mean it depends on quite specific names for things to pick out the tests.

What I can do is add a set of options to pass into the adapter, for users to provide their own names for the attributes, so the adapter knows to look for those custom names, and what they represent.

X9VoiD commented 1 year ago

May also get the test cases from dotnet test -t but that would be too slow to get I assume.

Issafalcon commented 1 year ago

Hi @X9VoiD , yes the dotnet test -t command is very slow unfortunately.

I've been thinking about how neotest has evolved now, and I'm going to try and bring back the original test discovery mechanism using the LSP. Given the performance enhancements in neotest core, using this approach might work better now. And it would also cover off all the less common scenarios you've highlighted her and in other tickets, without workarounds (in theory).

Bare with me for a bit and I'll see what I can do.

Issafalcon commented 1 year ago

Well, it wasn't long before I remembered a big reason why I moved away from using the LSP for test discovery. It isn't possible to locate positions of parameterized tests!

So I will continue with the approach of adding in adapter configuration for custom attributes.

Issafalcon commented 1 year ago

@X9VoiD - The changes should now provide support for these custom attributes. Give it a go and let me know if there are any issues 😄