Eventlistener will no longer send single spaces when the test output … #523
Conversation
…is empty or just whitespace #516
| namespace NUnit.VisualStudio.TestAdapter.Internal | ||
| { | ||
| #if NET35 | ||
|
|
There was a problem hiding this comment.
Nitpick: empty line between the #if and the class, but no empty line between the #endif and either class.
| var site = resultNode.GetAttribute("site"); | ||
|
|
||
There was a problem hiding this comment.
Super silly nitpick: new whitespace added to this line.
There was a problem hiding this comment.
Happens when a new line is inserted and spaces are default :-(
| @@ -180,22 +178,21 @@ public void SuiteFinished(XmlNode resultNode) | |||
|
|
|||
| private static readonly string NL = Environment.NewLine; | |||
| private static readonly int NL_LENGTH = NL.Length; | |||
| private IDumpXml dumpXml; | |||
| private readonly IDumpXml dumpXml; | |||
There was a problem hiding this comment.
Nice to see the cleanup in this file.
| { | ||
| public static bool IsNullOrWhiteSpace(this string value) | ||
| { | ||
| return value == null || value.Trim().Length == 0; |
There was a problem hiding this comment.
It's good to stay simple since it's not in a hot path. 👍
For comparison: https://referencesource.microsoft.com/#mscorlib/system/string.cs,55e241b6143365ef
There was a problem hiding this comment.
Yes, just used the https://msdn.microsoft.com/en-us/library/system.string.isnullorwhitespace(v=vs.110).aspx as a reference, and under remarks this code is shown as equivalent, and it is easier than the code from the referencesource.
| [TestCase(" \t")] | ||
| [TestCase(" ")] | ||
| [TestCase("")] | ||
| public void ThatIsNullOrWhiteSpaceHandlesTabs(string value) |
There was a problem hiding this comment.
Not all of these cases are related to handling tabs. Also can we have [TestCase(null)] and [TestCase("\r\n")]?
|
|
||
| namespace NUnit.VisualStudio.TestAdapter.Tests | ||
| { | ||
| #if !NETCOREAPP1_0 |
There was a problem hiding this comment.
Shouldn't we be running these tests on netcoreapp1.0?
| @"<test-output stream='Progress' testid='0-1001' testname='Something.TestClass.Whatever'><![CDATA[Whatever | ||
| ]]></test-output>"; | ||
|
|
||
| private const string BlankTestoutput = |
There was a problem hiding this comment.
For both these fields, I think you'd capitalize the O since it's two words.
|
Seems there is some build issues with the test project running NSubstitute...... got to look at that before merging. |
| @@ -4,7 +4,8 @@ | |||
| <PropertyGroup> | |||
| <RootNamespace>NUnit.VisualStudio.TestAdapter.Tests</RootNamespace> | |||
| <AssemblyName>NUnit.VisualStudio.TestAdapter.Tests</AssemblyName> | |||
| <TargetFrameworks>net46;netcoreapp1.0</TargetFrameworks> | |||
| <TargetFrameworks>net35;netcoreapp1.0</TargetFrameworks> | |||
There was a problem hiding this comment.
@OsirisTerje Changing net46 to net35 is what is causing the NuGet restore failures. Any reason not to use net46?
There was a problem hiding this comment.
copy paste error when I reverted back from debugging this. It is very annoying that VS don't really work with multiple targets, so I always remove the .netcore one......., just my way.....
…is empty or just whitespace #516
The eventlistener will not send anything if the test output is null, empty or just consists of whitespaces