Nunit-console: Warnings not displayed

[CharliePoole]

The console displays a report of failures, errors and warnings only if the overall result is failure. If there are only warnings it is not displayed, but it should be.

[CharliePoole]

See the logic of NUnitLite for comparison.

[mikkelbu]

I was looking into the code (and comparing with NUnitLite), and I found that I probably have to change the following line to also call WriteErrorsFailuresAndWarningsReport when OverallResult == "Warning
https://github.com/nunit/nunit-console/blob/4c006f9618cbc4eb083a139dd75b5ccf4c1fa08c/src/NUnitConsole/nunit3-console/ResultReporter.cs#L69-L70

However, I have a question regarding OverallResult. If I run .\nunit3-console.exe nunit3-console.tests.dll after a build (on master) I get something like the following where the overall result is Warning even though there are no warnings among the tests.

Test Run Summary
Overall result: Warning
Test Count: 296, Passed: 292, Failed: 0, Warnings: 0, Inconclusive: 0, Skipped: 4
Skipped Tests - Ignored: 1, Explicit: 2, Other: 1
Start time: 2017-09-15 19:44:26Z
End time: 2017-09-15 19:44:37Z
Duration: 11.422 seconds

If I then add Assert.Warn("Warning Message"); to a test and again run the console then I get the following (where one test have changed from passed to warning). But now the overall result is Inconclusive

Test Run Summary
Overall result: Inconclusive
Test Count: 296, Passed: 291, Failed: 0, Warnings: 1, Inconclusive: 0, Skipped: 4
Skipped Tests - Ignored: 1, Explicit: 2, Other: 1
Start time: 2017-09-15 19:46:51Z
End time: 2017-09-15 19:47:03Z
Duration: 11.711 seconds

Is this behaviour expected, or do we have a problem here?

[CharliePoole]

Without looking at the code, it seems to me that...

1. Your first example is correct. Ignored tests are given a "warning" level historically. When I created the new Warning result, I proposed using it for ignored tests, but the team wanted to keep the traditional Skipped:Ignored designation for backwards compatibility. I think this is the only remnant of our older way of reporting, but I can't guarantee it. 😄

2. The second example seems wrong to me. We may not have allowed for warnings in the determination of the overall result. We should only get an inconclusive overall result if no tests are run or if all tests that are rune are inconclusive.

[mikkelbu]

@CharliePoole you are right about both issues. Wrt. 1) we have the following "translation"
https://github.com/nunit/nunit-console/blob/175ddc7fd59ef4852751e908dcc2428f57fb20b7/src/NUnitConsole/nunit3-console/ResultReporter.cs#L43-L44
and wrt. 2) we have the missing handling of Warning in this switch
https://github.com/nunit/nunit-console/blob/175ddc7fd59ef4852751e908dcc2428f57fb20b7/src/NUnitEngine/nunit.engine/Internal/ResultHelper.cs#L168-L187

So since OverallResult == "Warning" can also mean skipped tests I think that I will also test for Summary.WarningCount > 0 before calling WriteErrorsFailuresAndWarningsReport() and then add handling of Warning into the switch.

[ChrisMaddock]

@mikkelbu - I've added this to the 3.8 console milestone, which I've made up from high-impact/low-work bugs. I don't expect us to get through them at any point in the next month or two, but just wanted to check that this one was still on your radar, or if you'd rather unassign yourself. 😄

[mikkelbu]

@ChrisMaddock actually, I've a bundle containing the correction for weeks, but I've not have time to test it properly (and write some tests), so it is still on my radar, but prioritised a bit low :)