Fix few issues with labels #56

somdoron · 2016-07-18T10:43:16Z

Outcome of a test is never printed
Test is being printed when the test is finished, problematic when I want to find an hanging thread. To fix printing the name of the test when the test starts

ghost · 2016-07-18T12:26:39Z

src/dotnet-test-nunit/TestRunner.cs

@@ -190,7 +190,7 @@ int Execute()
                else
                {
                    TestExecutionListener listener = new TestExecutionListener(_testExecutionSink, _options, assemblyPath);
-                    SetupLabelOutput(listener);
+                    SetupLabelOutput(listener);                    


nit: extra spaces

fixed and rebased

somdoron · 2016-07-18T12:41:22Z

@jasonwilliams200OK fixed and rebased.

ghost · 2016-07-18T12:50:35Z

Thanks @somdoron. Would be nice to have a test to verify the changes. :)

somdoron · 2016-07-18T13:01:18Z

@jasonwilliams200OK will add, might take me a day.

rprouse · 2016-07-18T13:10:55Z

src/dotnet-test-nunit/TestListeners/TestExecutionListener.cs

+
+            string output = null;
+
+            if (testResult.Messages.Count > 0)


This was a miss, good catch. 👍

CharliePoole · 2016-07-18T13:42:20Z

@rprouse I there a stated standard C# level to be used for dotnet-test-nunit?

rprouse · 2016-07-18T14:27:30Z

@CharliePoole there is no stated C# level, but because .NET Core 1.0 requires Visual Studio 2015 Update 3, I have been using C# 6 features.

somdoron · 2016-07-18T19:55:33Z

@jasonwilliams200OK I added tests.

@rprouse the current implementation doesn't print the label twice as the WriteLabelLine protects against that.

@CharliePoole @rprouse Regarding printing the label before the tests, right now it is only if you set the labels to All, if set to ON it will print the test when test is finished.
I know the behavior is different than nunit-console but does helps find hanging thread, try to guess the alphabetic order is pretty annoying and this behavior already helps find hanging tests.
If not for "All" we might create a new option called "Debug" that prints when a test begin.

ghost · 2016-07-18T20:02:42Z

test/dotnet-test-nunit.test/TestListeners/TestExecutionListenerTests.cs

@@ -215,7 +250,7 @@ public void FiresSuiteFinished()

            Assert.That(test, Is.Not.Null);
            Assert.That(test, Is.EqualTo("NUnit.Framework.Internal.CultureSettingAndDetectionTests.LoadWithFrenchCanadianCulture"));
-        }
+        }        


nit: extra space :)

fixed and rebased.

CharliePoole · 2016-07-18T20:06:14Z

@somdoron Actually, your implementation is correct, based on the intent for All versus On. The console implementation originally worked that way but it somehow got messed up when we made some changes to it.

somdoron · 2016-07-19T16:12:03Z

Can the PR be merged?

rprouse · 2016-07-19T16:18:45Z

src/dotnet-test-nunit/TestRunner.cs

+                if (labels == "ALL")
+                    WriteLabelLine(args.TestName);
+            };
+
            listener.TestFinished += (sender, args) =>
            {
                if (labels == "ALL")


If we are going to add the labels ALL to the start, let's remove it from here. When we go multi-threaded, the currentLabel code you pointed to could fail to prevent the second print.

rprouse · 2016-07-19T16:20:17Z

@somdoron one small change, then we can merge.

somdoron · 2016-07-19T16:38:19Z

@rprouse fixed and rebased

rprouse · 2016-07-19T16:39:45Z

👍 waiting for CI

Thanks for your help.

somdoron · 2016-07-20T07:00:44Z

thanks

Fix issue with results of a test missing

70135b1

ghost reviewed Jul 18, 2016
View reviewed changes

somdoron force-pushed the master branch from 86a9488 to 389eabf Compare July 18, 2016 12:41

somdoron force-pushed the master branch from 389eabf to 461b0ac Compare July 18, 2016 12:44

rprouse reviewed Jul 18, 2016
View reviewed changes

ghost reviewed Jul 18, 2016
View reviewed changes

somdoron force-pushed the master branch from 09e249d to 076111a Compare July 19, 2016 05:29

rprouse reviewed Jul 19, 2016
View reviewed changes

somdoron added 3 commits July 19, 2016 19:36

on labels=All print test name on test start instead on test outcome

9172436

add tests to TestListener.TestFinished

15b2c8b

add tests to TestListener.TestStarted

03c1cdb

somdoron force-pushed the master branch from 076111a to 03c1cdb Compare July 19, 2016 16:36

rprouse merged commit 1781116 into nunit:master Jul 19, 2016

rprouse mentioned this pull request Jul 19, 2016

when labels is set to All no outcome is printed, only test name #55

Closed

This was referenced Oct 4, 2016

NUnitLite on .NET Core - impossible to know which test is crashed. nunit/nunit#1820

Closed

Can't suppress headers and other output when executing tests #78

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix few issues with labels #56

Fix few issues with labels #56

somdoron commented Jul 18, 2016

ghost Jul 18, 2016

somdoron Jul 18, 2016

somdoron commented Jul 18, 2016

ghost commented Jul 18, 2016

somdoron commented Jul 18, 2016

rprouse Jul 18, 2016

CharliePoole commented Jul 18, 2016

rprouse commented Jul 18, 2016

somdoron commented Jul 18, 2016

ghost Jul 18, 2016

somdoron Jul 19, 2016

CharliePoole commented Jul 18, 2016

somdoron commented Jul 19, 2016

rprouse Jul 19, 2016

rprouse commented Jul 19, 2016

somdoron commented Jul 19, 2016

rprouse commented Jul 19, 2016

somdoron commented Jul 20, 2016

Fix few issues with labels #56

Fix few issues with labels #56

Conversation

somdoron commented Jul 18, 2016

ghost Jul 18, 2016

Choose a reason for hiding this comment

somdoron Jul 18, 2016

Choose a reason for hiding this comment

somdoron commented Jul 18, 2016

ghost commented Jul 18, 2016

somdoron commented Jul 18, 2016

rprouse Jul 18, 2016

Choose a reason for hiding this comment

CharliePoole commented Jul 18, 2016

rprouse commented Jul 18, 2016

somdoron commented Jul 18, 2016

ghost Jul 18, 2016

Choose a reason for hiding this comment

somdoron Jul 19, 2016

Choose a reason for hiding this comment

CharliePoole commented Jul 18, 2016

somdoron commented Jul 19, 2016

rprouse Jul 19, 2016

Choose a reason for hiding this comment

rprouse commented Jul 19, 2016

somdoron commented Jul 19, 2016

rprouse commented Jul 19, 2016

somdoron commented Jul 20, 2016