Fix test name reporting when test is in a base class

[paulirwin]

Is there an existing issue for this?

• I have searched the existing issues

Task description

Context: #914 (comment)

From @NightOwl888:

we should have a new issue about fixing test reports so the correct superclass is reported for tests that are declared in a base class.

NUnit reports the name of the class that the [Test] attribute it finds is declared in. We need it to report the name of the top level superclass that inherited the class that the [Test] attribute is declared in, since we have tests in test fixture classes that are designed specifically to be inherited so standardized tests can be reused. Since several of these classes are in the test framework, this issue affects end users of the test framework as well as internally.

As a workaround, we currently override test methods, redefine the [Test] attribute on the superclass, and cascade the call to the base class. This gets NUnit to display the proper class name, but it obviously doesn't scale well.

Also see: #914 (comment)

[paulirwin]

I looked into this yesterday, and the only place where the test class name seems to be "reported" is in the stack trace. I don't believe we can easily manipulate the stack traces to include the child class name.

We could look into modifying the NUnit test naming pattern, although they strongly discourage modifying that. I will give it a try, though.

If that doesn't work or causes issues with the IDE, I think perhaps our best option, per our other discussions about having an internal analyzer project, could be to create an analyzer with quick fix that looks for tests defined in a base class that haven't been overridden in the derived class, and have the quick fix scaffold out the overrides. While this wouldn't solve the issue as originally defined, it would help solve the "doesn't scale well" concern by automating the detection of missing overrides in the IDE and build, and the fix in the IDE.

[NightOwl888]

Well, at least the analyzer will help prevent developers from backing out the overrides. At one point, these were overridden to fix all of the names, but it was commented and considered a temporary workaround. There have been at least 2 developers that decided they were "helping" by removing them, though. And now they are mostly gone.

Do note that this pertains to tests that inherit any class that contain tests. Some of them may be classes that are 2 or more levels of inheritance from the base class they need to override. The base class that contains the test does not necessarily need to be abstract.

It seems more like we should either change the name of the test or report this to NUnit so they can provide a fix or another workaround. It would be much simpler for us and our users to maintain if the inheritance-based test framework understood how to self report its own inherited tests.

It may also be possible to come up with a solution based on how xunit reports them. I don't ever recall having this issue with xunit even though J2N uses several tests that are inherited this way.

[NightOwl888]

Come to think of it, the only thing we really need is for the test message to have the name of the test subclass in it when there is a failure. A simple way to accomplish that would be to add the name of the class to the message in the LuceneTestCase.TearDown() method by doing a Reflection call (GetType().FullName) at that point (since it only prints if the test fails).

[paulirwin]

Yeah, I had that working locally before I found the solution. But here's the PR: #1086

That is running on ADO now, passes in GitHub. You can see in the GitHub test output that it uses the full name of the test including the derived class name (not the base class). Let me know if that causes any problems in VS for you locally if it picks up the runsettings file.

[paulirwin]

PR #1086 has been updated. Just FYI in case someone discovers this issue later looking for a solution, if you pass NUnit.DisplayName=FullName after the -- separator in dotnet test (i.e. dotnet test {any regular args here} -- NUnit.DisplayName=FullName), it will work without having a runsettings file. Alternatively, you can set it in a runsettings file if you'd prefer:

<?xml version="1.0" encoding="utf-8"?>
<RunSettings>
  <NUnit>
    <DisplayName>FullName</DisplayName>
  </NUnit>
</RunSettings>

[NightOwl888]

Just for reference, the documentation on how to supply runsettings on the command is at: https://github.com/Microsoft/vstest-docs/blob/main/docs/RunSettingsArguments.md. For CI purposes, it is important to note that the syntax varies by shell. We explicitly specify bash as the shell so the command will work on every OS.