Console output for minimal and quiet #2191

hvinett · 2019-09-24T07:32:26Z

Description

modified printing level in verbosity level is quiet

##issue
1836

src/vstest.console/Internal/ConsoleLogger.cs

src/vstest.console/Resources/Resources.resx

src/vstest.console/Internal/ConsoleLogger.cs

vagisha-nidhi · 2019-09-26T12:14:24Z

            if (testsFailed > 0)

Can we figure out testsFailed, totalTests etc from SummaryDictionary? Class level variables won't be needed then.

Refers to: src/vstest.console/Internal/ConsoleLogger.cs:698 in 5bda2ee. [](commit_id = 5bda2ee, deletion_comment = False)

src/vstest.console/Internal/ConsoleLogger.cs

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs

src/vstest.console/Internal/ConsoleLogger.cs

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs

src/vstest.console/Internal/ConsoleLogger.cs

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs

hvinett · 2019-10-04T10:33:48Z

@adamralph Can you please review the change? I have attached the screenshot too.

adamralph · 2019-10-04T11:09:54Z

@hvinett I assume this addresses #1836?

I don't have the bandwidth to review the diff, but the results look good. I just have two comments:

Is the blank line between the output lines deliberate? Can we remove it?
There seem to be two spaces betweeen some of the items in each line. Is that deliberate? Can we try to see how it looks with only space used consistently?

adamralph · 2019-10-04T11:13:31Z

Oh, two more things:

the target framework is not currently displayed
time unit (e.g. ms) should be preceded with a space

Taking all my comments into consideration, I would like to see this:

Failed! Total: 1. Pass: 1. Fail: 0. Skip: 0. (239 ms) UnitTestProject2.dll (netcoreapp3.0).
Failed! Total: 1. Pass: 1. Fail: 0. Skip: 0. (239 ms) UnitTestProject4.dll (netcoreapp3.0).

singhsarab · 2019-10-14T11:45:41Z

@hvinett Can you please take into consideration the feedback shared by @adamralph and push the changes.

vagisha-nidhi · 2019-10-22T05:46:55Z

src/vstest.console/Internal/ConsoleLogger.cs

+                    if (summary.FailedTests > 0)
+                    {
+                        // Failed! Pass {1} Failed {2} skipped {3} Time : 233 se ({4})
+                        Output.Information(false, ConsoleColor.Red, string.Format(CultureInfo.CurrentCulture, CommandLineResources.TestRunSummaryFailed, summary.TotalTests, summary.PassedTests, summary.FailedTests, summary.SkippedTests, GetFormattedDurationString(summary.TimeSpan), sd.Key.Split('\\').Last(), targetFramework));


What happens when target framework is not there? Do we always have a target framework?

Directly appending target framework will append the full name of target framework. eg: .NetCoreApp, Version 3.0.
Do we want to consider appending short folder name (netcoreapp3.0) as suggested by @adamralph ?
cc: @singhsarab

src/vstest.console/Internal/ConsoleLogger.cs

adamralph · 2019-10-22T07:09:34Z

This seems to be progressing nicely!

My current feedback:

I would prefer to have the short TFM rather than the long framework name. I.e. netcoreapp3.0 instead of NetCoreApp, Version 3.0. The short TFM matches what we specify in the project file, and is less verbose (i.e. better for "quiet" output).
There should be a space between the time value and the unit. E.g. 93 ms instead of 93ms.
I would prefer to have only a single space between the results, instead of two spaces. E.g.
```
Passed! Total: 1. Pass: 1. Fail: 0. Skip: 0.
```
instead of
```
Passed!  Total: 1.  Pass: 1.  Fail: 0.  Skip: 0.
```
I like the red colouring for when there is a failure. Should we also have yellow colouring for when there is no failure, but some tests are skipped?

Again, taking all my points into account, I'd like to see:

// in yellow, because Skip != 0
Passed! Total: 2. Pass: 1. Fail: 0. Skip: 1. (239 ms) UnitTestProject2.dll (netcoreapp3.0).

nohwnd · 2020-06-03T10:28:12Z

Removed the empty line that was removed previously in #2039

nohwnd · 2020-06-03T12:43:49Z

Hmm, since the minimal is the default, and Normal shows the passed tests, this should really affect just the Quiet output. But I would expect quiet to be, well, quiet. But since this was approved above I am just going to go with it, because I see no other option, that would not be a breaking change.

adamralph · 2020-06-03T21:41:05Z

It looks like some great progress has been made here!

Can we please update the issue description to show the final rendering and make it clear at what levels this output will be shown? Specifically, I would expect minimal to have this new, minimal output rather than the current multi-line output. I would also expect minimal to omit "Starting test execution, please wait..." and "A total of x test files matched the specified pattern.".

minimal is supposed to be minimal, i.e. just show the minimum required to be able to know what happened, so I would expect that to be the single line with the results.

I would expect quiet to be quiet, i.e. don't show anything unless there is a problem.

nohwnd · 2020-06-08T07:21:20Z

I would expect the same, but the problem is that Minimal is the default output, and changing the default is always problematic. :/

adamralph · 2020-06-08T08:04:37Z

But that's exactly what happened in #1836, when the default was changed from single line to multi-line. Now we have an opportunity to change it back again.

nohwnd · 2020-06-08T08:13:07Z

I am more concerned about not being able to see which tests actually failed. Because this output does not report the tests anymore, just the per-source summary.

adamralph · 2020-06-08T08:33:29Z

OK, then I misunderstood the current proposal. To summarise what I would like to see:

minimal — This is the level that I really care about. It should show the one-line summary that this PR is introducing, plus any test failures, and nothing else. This will effectively restore minimal to what it should be (prior to Improve readability of dotnet test #1836) and also improves it. minimal should never have been changed from a single line summary to a multi-line summary. And as I said above, it should not include "Starting test execution, please wait...", "A total of x test files matched the specified pattern.", etc. That is not minimal output.
quiet — I'm not too bothered what happens here, but I guess, in order to be "quiet", it shouldn't show anything, and the non-zero return code from dotnet test would indicate failure. I guess it could show the one-line summary if there are failures, but as I said, I don't really care.

nohwnd · 2020-07-13T10:45:04Z

minimal — This is the level that I really care about. It should show the one-line summary that this PR is introducing, plus any test failures, and nothing else. This will effectively restore minimal to what it should be (prior to #1836) and also improves it. minimal should never have been changed from a single line summary to a multi-line summary. And as I said above, it should not include "Starting test execution, please wait...", "A total of x test files matched the specified pattern.", etc. That is not minimal output.

Aaah okay, reverting back to where I was and just adding the failed results reporting. That is why it was writing the summary after the whole run. not after every assembly. Could have saved myself a lot of work if this would have become clearer before :)

adamralph · 2020-07-13T12:29:31Z

@nohwnd would it be possible to get an up-to-date screenshot which shows the current output from this PR? I'm finding it difficult to infer from the various changes that are being pushed.

nohwnd · 2020-07-13T12:51:46Z

Minimal output.

adamralph · 2020-07-13T13:30:20Z

Thanks @nohwnd.

☝️ I assume --nologo will still remove these lines (including the blank line)?

☝️ Can we please remove these lines from minimal output?

nohwnd · 2020-07-13T14:16:40Z

Yes, --nologo will remove those lines.

Removing those lines would be more difficult because the logger parses the verbosity, but in this case the runner is what writes those two lines, and they are also a nice indication that something happens. So I'd rather keep it to get his finally merged.

adamralph · 2020-07-13T14:38:17Z

OK. It's still a big improvement.

BTW, has the blank line which was removed here and here come back yet again?

nohwnd · 2020-07-13T14:43:24Z

You are right I reverted to the point where it worked basically as it should, added the change for removing the new line again. Now it should be gone.

nohwnd · 2020-07-14T09:11:35Z

Minimal
Minimal abort:
quiet:
normal:

adamralph · 2020-07-14T09:24:26Z

Thanks for the latest screenshots @nohwnd. It's really good to see how the output is progressing.

One observation I have is that the blank line usage seems to be a bit inconsistent:

In Minimal, there is a blank line after the skipped message but not in Minimal abort.
Similarly, that same blank line serves as a blank line before the summary in Minimal but not in Minimal abort
There is a blank line after "A total of 2 test files..." in quiet, but not in normal

Perhaps it's worth doing a full assessment of where blank lines are used and are not used, in order to align everything?

As I mentioned, my chief concern is minimal. My preference would be:

No blank line between "A total of 2 test files..." and the summary when there is no other content between them.
Blank lines after "A total of 2 test files..." and before the summary when there is some other content between those two lines, e.g. failures, skips, or abort messages

nohwnd · 2020-07-14T09:53:29Z

That output is coming from approximately 3 places in quite random order, especially in the abort case, there we have output from console logger, collector logger, and blame logger. Sadly that is how the architecture is done. I also don't like the output in the abort cases, it looks messy, but there is not much I can do about it.

adamralph · 2020-07-22T13:12:56Z

🎉 great to see this merged! Is there a rough timeline for 16.8?

added verbosity quiest

795d3a8

singhsarab reviewed Sep 24, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

singhsarab reviewed Sep 24, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

singhsarab reviewed Sep 24, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

singhsarab reviewed Sep 24, 2019

View reviewed changes

src/vstest.console/Resources/Resources.resx Outdated Show resolved Hide resolved

singhsarab reviewed Sep 24, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vineeth Hanumanthu added 2 commits September 24, 2019 19:04

removed source in printing

4da1046

added tests

5bda2ee

vagisha-nidhi reviewed Sep 26, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Sep 26, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Sep 26, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Sep 26, 2019

View reviewed changes

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs Outdated Show resolved Hide resolved

vineeth Hanumanthu added 2 commits October 3, 2019 14:42

added tests,changed private members of summary info to local members

103ee9e

naming corrected

afeda96

vagisha-nidhi reviewed Oct 3, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Oct 3, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Oct 3, 2019

View reviewed changes

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs Outdated Show resolved Hide resolved

combined two for loops to one for loop

dbd9b7e

vagisha-nidhi reviewed Oct 4, 2019

View reviewed changes

src/vstest.console/Internal/ConsoleLogger.cs Outdated Show resolved Hide resolved

vagisha-nidhi reviewed Oct 4, 2019

View reviewed changes

test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs Outdated Show resolved Hide resolved

removed redundant code and fomatted

bfb5804

modified resources

c952c42

added target framework in quiet case

3391a72

vagisha-nidhi reviewed Oct 22, 2019

View reviewed changes

nohwnd modified the milestones: 16.7.0, 16.8.0 Jun 16, 2020

pavelhorak added the pri: 1 label Jul 2, 2020

nohwnd added 2 commits July 13, 2020 13:44

Merge master

b0ab8d4

check

8644107

Minimal output

54aca6f

nohwnd force-pushed the ConsoleLoggerVerbosityQuietCase branch from 41927b6 to 54aca6f Compare July 13, 2020 12:49

Fix test

6ad0add

Write when aborting

c4a52e4

Remove empty line

ea8b54b

Sanan07 approved these changes Jul 22, 2020

View reviewed changes

nohwnd merged commit 6343278 into microsoft:master Jul 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Console output for minimal and quiet #2191

Console output for minimal and quiet #2191

hvinett commented Sep 24, 2019 •

edited

Loading

vagisha-nidhi commented Sep 26, 2019

hvinett commented Oct 4, 2019 •

edited

Loading

adamralph commented Oct 4, 2019

adamralph commented Oct 4, 2019

singhsarab commented Oct 14, 2019

vagisha-nidhi Oct 22, 2019

adamralph commented Oct 22, 2019

nohwnd commented Jun 3, 2020

nohwnd commented Jun 3, 2020

adamralph commented Jun 3, 2020

nohwnd commented Jun 8, 2020

adamralph commented Jun 8, 2020

nohwnd commented Jun 8, 2020

adamralph commented Jun 8, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

nohwnd commented Jul 14, 2020

adamralph commented Jul 14, 2020 •

edited

Loading

nohwnd commented Jul 14, 2020

adamralph commented Jul 22, 2020 •

edited

Loading

Console output for minimal and quiet #2191

Console output for minimal and quiet #2191

Conversation

hvinett commented Sep 24, 2019 • edited Loading

Description

vagisha-nidhi commented Sep 26, 2019

hvinett commented Oct 4, 2019 • edited Loading

adamralph commented Oct 4, 2019

adamralph commented Oct 4, 2019

singhsarab commented Oct 14, 2019

vagisha-nidhi Oct 22, 2019

Choose a reason for hiding this comment

adamralph commented Oct 22, 2019

nohwnd commented Jun 3, 2020

nohwnd commented Jun 3, 2020

adamralph commented Jun 3, 2020

nohwnd commented Jun 8, 2020

adamralph commented Jun 8, 2020

nohwnd commented Jun 8, 2020

adamralph commented Jun 8, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

adamralph commented Jul 13, 2020

nohwnd commented Jul 13, 2020

nohwnd commented Jul 14, 2020

adamralph commented Jul 14, 2020 • edited Loading

nohwnd commented Jul 14, 2020

adamralph commented Jul 22, 2020 • edited Loading

hvinett commented Sep 24, 2019 •

edited

Loading

hvinett commented Oct 4, 2019 •

edited

Loading

adamralph commented Jul 14, 2020 •

edited

Loading

adamralph commented Jul 22, 2020 •

edited

Loading