Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestContext.Progress shows up as Warning in TFS/Azure Dev Ops #1037

Closed
mharwig opened this issue Dec 2, 2022 · 13 comments
Closed

TestContext.Progress shows up as Warning in TFS/Azure Dev Ops #1037

mharwig opened this issue Dec 2, 2022 · 13 comments
Assignees
Milestone

Comments

@mharwig
Copy link
Contributor

mharwig commented Dec 2, 2022

Version information

Issue

TestContext.Progress written during a test shows up in TFS/Azure DevOps as warning when running a test via a VSTest task with NUnit Adapter 4.3.1.0. ConsoleOut is enabled via runsettings.

Code used to write progress in test:
TestContext.Progress.WriteLine("Message via TestContext.Progress");
Expected result white (informational) log line in TFS. Instead the message is shows as a warning (orange) warning in TFS:

ProgressMessageAsWarningInTFS

Possible cause

My theory regarding the cause of the issue based on analysis of the code (did not run/debug the code):
The text written via TestContext.Progress.WriteLine results in a TestOutput event in EventListenerTextWriter:

private bool TrySendToListener(string text) { context.Listener.TestOutput(new TestOutput(text, _streamName, context.CurrentTest?.Id, context.CurrentTest?.FullName));

The TestOutput event is in handled in nunit3-vs-testadapter in class NUnitEventListener OnTestEvent:

public void OnTestEvent(string report) { . . . case NUnitTestEventHeader.EventType.TestOutput: TestOutput(new NUnitTestEventTestOutput(node)); break;

The handled TestOutput is always forwarded as warning in NUnitEventListener TestOutput:

public void TestOutput(INUnitTestEventTestOutput outputNodeEvent) { . . . recorder.SendMessage(TestMessageLevel.Warning, text); }

Possible solution:
recorder.SendMessage(outputNodeEvent.IsProgressStream ? TestMessageLevel.Informational : TestMessageLevel.Warning, text);
or
recorder.SendMessage(outputNodeEvent.IsErrorStream ? TestMessageLevel.Warning : TestMessageLevel.Informational, text);
Further more one could argue that if settings.ConsoleOut == 1 the message level should also be TestMessageLevel.Informational, similar to the message sent in NUnitEventListener TestFinished.

Apologies if the issue turns out to be caused by something else entirely (e.g. configuration on my side).

@mharwig
Copy link
Contributor Author

mharwig commented Dec 2, 2022

I just noticed I only have this issue when running with batched tests, so vstest task settings
batchingBasedOnAgentsOption: customBatchSize
customBatchSizeValue: 100

@OsirisTerje
Copy link
Member

@mharwig Those settings don't do anything wrt the adapter. Can you provide a repro for this? Using a public azure pipeline and git repo would do.

@mharwig
Copy link
Contributor Author

mharwig commented Dec 5, 2022

@OsirisTerje I realize that those settings are not related to the adapter. However using customBatchSizes has the side effect that TFS runs the tests via the translation layer (see comment here: microsoft/azure-pipelines-tasks#10709 (comment)), as opposed to calling vstest.console.exe directly. Apparently the variant ran via the api handles the warning messages differently. I will see if I can find the time to create a reproducer.

@OsirisTerje
Copy link
Member

Your idea of using the ConsoleOut==1 could be a consistent solution. Alternatively, introduce explicit settings for how to handle Progress and Error.

@mharwig
Copy link
Contributor Author

mharwig commented Dec 5, 2022

Your idea of using the ConsoleOut==1 could be a consistent solution. Alternatively, introduce explicit settings for how to handle Progress and Error.

Explicit settings would also be fine with me. However the suggested solution using IsErrorStream to output warning and informational otherwise would also work and is consistent with output handled by TestConverter.FillResultFromOutputNodes. Explicit settings would of course be more flexible.

@OsirisTerje
Copy link
Member

OsirisTerje commented Dec 5, 2022

Thanks for the repro, I see your point there. You could just raise a Pull Request with your suggested change. I can't see any reason for not approving that.

@OsirisTerje OsirisTerje added this to the 4.4 milestone Dec 5, 2022
mharwig pushed a commit to mharwig/nunit3-vs-adapter-TestProgressDisplayedAsWarning that referenced this issue Dec 5, 2022
@mharwig
Copy link
Contributor Author

mharwig commented Dec 5, 2022

Raised a pull request, please note that Windows task of 'NUnit 3 VSTest Adapter CI ' currently fails for all branches. I'm guessing the 'Install .NET Core 2.1 runtime' step should be changed to 'Install .NET Core 3.1 runtime', which is something I cannot do.

mharwig pushed a commit to mharwig/nunit3-vs-adapter-TestProgressDisplayedAsWarning that referenced this issue Dec 5, 2022
@OsirisTerje
Copy link
Member

OsirisTerje commented Dec 5, 2022

Thanks!

PS: Yes, aware of those fails. Will be fixed. :-) But the Cake build is green, and that is the important one.

OsirisTerje pushed a commit that referenced this issue Dec 5, 2022
…ise (#1037) (#1038)

* Send message as warning in case of error stream, informational otherwise (#1037)

* Moved TestOutput unit test to separate region (#1037)

Co-authored-by: Martin Harwig <m.harwig@ellips.com>
@mharwig
Copy link
Contributor Author

mharwig commented Dec 5, 2022

Thanks for the quick responses, btw I tested the resulting changes on the reproducer and it had the desired effect

@OsirisTerje
Copy link
Member

Awesome!

@OsirisTerje
Copy link
Member

@mharwig Version 4.4.0-beta.1 is out now https://www.nuget.org/packages/NUnit3TestAdapter/4.4.0-beta.1 . It includes this issue and support for -net 8. Appreciate if you have the time to verify it works as it should.

@mharwig
Copy link
Contributor Author

mharwig commented Feb 24, 2023

@OsirisTerje Hi, Thanks for the update.

I just updated to Version 4.4.0-beta.1 in the reproducer, and it works as intended.

Run with version 4.3.1:
https://martinharwig.visualstudio.com/NUnitVsTestAdapterBug/_build/results?buildId=14&view=results
image

Run with Version 4.4.0-beta.1 (no more warning with progress message):
https://martinharwig.visualstudio.com/NUnitVsTestAdapterBug/_build/results?buildId=15&view=results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants