Issue #4686: Add ProgressTraceListener

[maettu-this]

As proposed in #4686, resolves #4686
Also fixes #4134.
Documentation update with separate docs PR.

[maettu-this]

I guess I need some support here. The Windows and MacOS tests indicate "1 tests failed" but searching the log for neither "fail" nor "Trace" nor "Progress" sheds any light into the failure.

On Linux, one of the newly added tests indeed fails:

1) Failed : NUnit.Framework.Tests.Diagnostics.ProgressTraceListenerTests.TestDebugIsDirectedToOutput 
Assert.That(TestResultListener.Outputs, Has.Count.EqualTo(1)) 
Expected: property Count equal to 1 
But was: 0 
at NUnit.Framework.Tests.Diagnostics.ProgressTraceListenerTests.TestDebugIsDirectedToOutput() in /_/src/NUnitFramework/tests/Diagnostics/ProgressTraceListenerTests.cs:line 108 

But the question is, why does this behave differently on Linux?

[maettu-this]

Review consideration regarding potential location of the listener:

• NUnit.Framework.Diagnostics.ProgressTraceListener (new namespace) pro: orthogonality with System.Diagnostics.ConsoleTraceListener
• NUnit.Framework.TestContext.ProgressTraceListener (class within TestContext) pro: in proximity with TestContext.Progress
• NUnit.Framework.ProgressTraceListener (top-level) pro: simplicity

[maettu-this]

@OsirisTerje thanks for re-triggering the workflows. I have found the culprit, of course the System.Diagnostics.Debug related tests must be DEBUG conditional. Please trigger the workflows once more.

[OsirisTerje]

@stevenaw @manfred-brands Is this ok with you guys? It's a new namespace but it may be fitting. I am good with this.

[stevenaw]

@OsirisTerje The new namespace is a good point. This appears to be the first time we're explicitly adding a new writer with external intent. Current writers are all in the Internal namespace.

I don't have strong opinions on it either way so I think the namespacing in this PR is fine until we decide on long term plans for some of the other Internal-namespaced items.

[manfred-brands]

A few small changes suggested to the XML documentation to better reflect what is needed and what is actually used in the ProgressTraceListenerTests

[manfred-brands]

Thanks @maettu-this, looks good to me now.

[OsirisTerje]

@maettu-this The builds failed, seems like some nuget stuff. And, you also have some warnings. Can you fix and push again?

[stevenaw]

@OsirisTerje The nuget warnings are odd. I've tried to rerun the job.

@maettu-this the other failures may just be related to unused using after having removed the attribute. Thank you for removing that! Seems like maybe just a bit more related cleanup

[stevenaw]

Thanks @manfred-brands
Been tough to push changes myself, most of these reviews have been from my phone. I see it building now, will approve and merge after it passes 🙂

[stevenaw]

The build is now passing!
Thank you for the contribution @maettu-this this looks good to me now too