Use full test name in NUnit tests, #923

[paulirwin]

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Uses the full test name in NUnit tests, so that it's clear where inherited tests come from.

Fixes #923

Description

This outputs the full test name including namespace and class name in the GitHub, Azure DevOps, and local build scripts for the log output as well as the trx files.

[NightOwl888]

A couple of things:

1. The .runsettings file is unnecessary because we can supply the settings on the command line (the same way the other runsettings are applied, i.e. NUnit.DisplayName=FullName.
2. This PR assumes the problem only applies to us, but it will also apply to all users of Lucene.Net.TestFramework. Arguably, the most useful functionality in Lucene.Net.TestFramework is the BaseTokenStreamTestCase class, which will have this problem for anyone who uses it. It would not be practical to tell all of our users that they must configure FullName in their CI environment to get useful debug info. See my comment about how we can report the class name for all users of the test framework: Fix test name reporting when test is in a base class #923 (comment)

[paulirwin]

I'll add in the failure message to print the class name.

This fails currently on ADO, wrong path to the runsettings file. I'll replace this with command-line args which will solve the problem. I think it's worthwhile to have it in the test log output and trx file too, not just in the failure message.

[NightOwl888]

I'll add in the failure message to print the class name.

This fails currently on ADO, wrong path to the runsettings file. I'll replace this with command-line args which will solve the problem. I think it's worthwhile to have it in the test log output and trx file too, not just in the failure message.

Fair enough. I would like to see what the UX looks like without the extra info in the log file, though. Could we make this a parameter that defaults to enabled if not supplied in ADO that allows it to be disabled in a specific ADO pipeline? For example, the properties that are documented at the top of the azure-pipelines.yml file can all be defined in the pipeline. Perhaps name it DisplayFullName and put it under the "Testing variables".

[paulirwin]

Updated to remove the runsettings file; pass NUnit.DisplayName=FullName after the -- in dotnet test in GitHub, ADO, and local build scripts; add ability to disable this in ADO; and also add to the GitHub workflows to trigger the workflow on workflow changes (so we don't have to do fake commits). I do like this approach better. It results in nice full test names in the GitHub and ADO logs/trx files by default, and should not interfere with the IDE or anyone's existing runsettings files.

[NightOwl888]

The changes look good and I can confirm they work as expected.

There is a bit of extra time used to both log and upload the test results, but it is only around 5%. That being said, I will be configuring the environments I use to disable this feature. The logs are only the secondary way to view the test results and it isn't worth the extra time for me. There are still many things we can do to improve performance on ADO, though (like request more parallel agents, upload the test results in the background in parallel, and start testing NuGet packages so we can skip dotnet publish).

The effect on GitHub Actions is far less noticeable because it only uploads the files as artifacts without also pushing them to an API. Not to mention, there are far more build agents working in parallel to minimize the time it takes to run.