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

Cannot run an individual test whose TestCase parameter contains characters from the range [U+0001..U+001F] #484

Closed
vchebotok opened this issue Mar 12, 2018 · 21 comments

Comments

@vchebotok
Copy link

Here is a sample code:

using NUnit.Framework;
using System;

namespace Test
{
    [TestFixture]
    public class TestClass
    {
        [TestCase('\x01', ExpectedResult = "%01")]
        [TestCase('\x1F', ExpectedResult = "%1F")]
        [TestCase('\x00', ExpectedResult = "%00")]
        [TestCase('\x20', ExpectedResult = "%20")]
        [TestCase('\x80', ExpectedResult = "%80")]
        [TestCase('\x90', ExpectedResult = "%90")]
        public string Test1(char c)
        {
            return Uri.HexEscape(c);
        }

        [TestCase("ABC\x01", ExpectedResult = "ABC%01")]
        public string Test2(string s)
        {
            return Uri.EscapeDataString(s);
        }
    }
}

My project targets .NET Framework v. 4.6.1 and I use:

  • Visual Studio Community 2017 v. 15.6.1;
  • NUnit v. 3.9.0;
  • NUnit 3 Test Adapter v. 3.10.0

If I press "Run all" in Test Explorer, all the tests from the sample above are executed without any errors. However, if I press "Run Selected Tests" in Test Explorer or "Run Tests" from the context menu in the code editor, Test2 and the first two test cases in Test1 are never executed.

@rprouse
Copy link
Member

rprouse commented Mar 12, 2018

Thanks for the repro. It sounds like a problem with the filters. We'll need to determine if it is the VS filters, the translation in the adapter or lower in NUnit.

@arulvenkatesh
Copy link

@vchebotok @rprouse I faced the same issue today. Its also difficult to debug the particular test from test explorer because of this issue.

@MatthewBeardmore
Copy link

MatthewBeardmore commented Oct 15, 2018

When running test2, the filter appears to be generated correctly by filter = filterBuilder.ConvertTfsFilterToNUnitFilter(TfsFilter, loadedTestCases); It is as below in master as of today:

<filter><test>Test.TestClass.Test2(&quot;ABC\u0001&quot;)</test></filter>

So I'm guessing this issue is with NUnit's filtering with unicode characters instead of the adapter.

@MatthewBeardmore
Copy link

After further investigation... it's actually a difference between what NUnit expects when filtering on tests and what VS is returning. In NUnitTestFilterBuilder.MakeTestFilter, it returns the filter from VS, which is the FQN as returned above with the unescaped XML character. When this filter makes it into NUnit, it fails to load the XML in TestFilter.FromXML, as the unescaped UTF8 character breaks XML deserialization. If we escape it, it will pass serialization, but then fail to match in FullNameFilter as the escaped UTF8 character will then not match the unescaped one that NUnit detected from the method argument list itself. So I think in NUnit, we would need to either:
1: Unescape the characters that we escaped in the XML to make it load, so that FullNameFilter would match correctly.
2. Escape all FullNameFilter options that come in to match what is able to be loaded from the XML.

I think 2 is more feasible, as unescaping is a more difficult task and we know that the test filters will be coming in as XML (which will be escaped if they load properly), so the escaping in the filter will make it so that they will match between the two in (I believe) all cases.

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2018

If the adapter is sending the framework invalid XML for any reason, the adapter should be fixed to stop deviating from the XML protocol and escape all values.

Likewise if the framework is sending the adapter invalid XML, the framework should be fixed to stop deviating from the XML protocol and escape all values.

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2018

I'm slowly catching up to where you all are. This issue might depend on nunit/nunit#3063 (comment).

@OsirisTerje
Copy link
Member

Yes, I think that is correct. Now, can we escape these for the xml and make the engine accept these ?

@jnm2
Copy link
Contributor

jnm2 commented Oct 24, 2018

Now, can we escape these for the xml and make the engine accept these ?

Not sure, I think so? Two things: I don't think we should send anything except valid XML, even if the current engine/framework combinations require invalid XML in order to filter these tests. Also, I don't want us to think of escaping the test names (implies round-trip) but rather as reformatting them for display purposes (implies one-way with collisions). Sort of in line with what I'm saying the framework should do: nunit/nunit#3063 (comment)

If the framework is behaving (by reformatting test names with control characters) and the engine is behaving (by rejecting invalid XML), the adapter only needs to worry about VSTest's source-based discovery if it misbehaves (by not reformatting test names that have control characters the way the framework would).
To deal with misbehaving source-based discovery, the adapter would also have to replace control characters in the discovered names the same way the framework would in order to get the filter to work.

@johnmwright
Copy link
Contributor

This issue has the same underlying cause as #622 and is resolved by PR #668

@OsirisTerje OsirisTerje added this to the 3.16 milestone Oct 31, 2019
@OsirisTerje
Copy link
Member

This is fixed by https://www.myget.org/feed/nunit/package/nuget/NUnit3TestAdapter/3.16.0-dev-01202

Before fix:
image

After fix:
image

@craigbroadman
Copy link

craigbroadman commented May 13, 2020

I am still getting this issue.

Microsoft Visual Studio Enterprise 2019
Version 16.5.4
VisualStudio.16.Release/16.5.4+30011.22
Microsoft .NET Framework
Version 4.8.03761

JetBrains ReSharper Ultimate 2019.2.2 Build 192.0.20190828.114715

image

<package id="NUnit" version="3.12.0" targetFramework="net472" />
<package id="NUnit.Engine" version="3.11.1" targetFramework="net472" />
<package id="NUnit3TestAdapter" version="3.16.1" targetFramework="net472" developmentDependency="true" />

Any ideas why?

@OsirisTerje
Copy link
Member

Did you use the options for enabling this? There are these two that has to be set in the runsettings:
https://github.com/nunit/docs/wiki/Tips-And-Tricks#UseParentFQNForParametrizedTests
https://github.com/nunit/docs/wiki/Tips-And-Tricks#usenunitidfortestcaseid

If you use version 3.16.0 they are default on.

@craigbroadman
Copy link

I'm not using a runsettings file at all - is that the problem?

@craigbroadman
Copy link

craigbroadman commented May 13, 2020

I've tried a runsettings file like below but it still isn't working...

image

@OsirisTerje
Copy link
Member

Grab the test project at https://github.com/nunit/nunit3-vs-adapter.issues/tree/master/Issue484
It should run as this:
image

You can then compare this with your own code.
Note that the Test.Sdk is 16.5. It doesnt work with e.g. 16.2, not sure exactly which version is the breaking point, but somewhere between them :-) Use 16.5, its the latest anyway.

The runsettings is a bit bigger than yours, but the only important thing is the two properties.

@craigbroadman
Copy link

I still get the same thing....

image

@OsirisTerje
Copy link
Member

You're sure the runsettings are enabled? The settings for the test explorer.... select the file simple.runsettings (auto doesnt always work).

Which version of VS 2019 is this ?

@craigbroadman
Copy link

It's slightly different with the runsettings file selected (the icon changes in test explorer) but it still isn't running the tests.

I am using Microsoft Visual Studio Enterprise 2019 - Version 16.5.4

image

The output displays this...

---------- Starting test run ----------
NUnit Adapter 3.16.1.0: Test execution started
Running selected tests in C:\Source\Temp\nunit3-vs-adapter.issues\Issue484\Issue484\bin\Debug\net472\Issue484.dll
Test Output folder checked/created : C:\Source\Temp\nunit3-vs-adapter.issues\Issue484\Issue484\bin\Debug\net472\false
NUnit3TestExecutor converted 2 of 2 NUnit test cases
Test results written to C:\Source\Temp\nunit3-vs-adapter.issues\Issue484\Issue484\bin\Debug\net472\false\Issue484.xml
NUnit Adapter 3.16.1.0: Test execution complete
Microsoft.VisualStudio.TestStorage.TestStoreIndexingException: Failed to add result for test 'Issue484.TestClass.Test1' with ID '94a1c9bc-6703-5715-60b9-f1ef941637e7'.
at Microsoft.VisualStudio.TestStorage.TestStoreIndexSet.AddToIndexSet(TestBaseRecord record)
at Microsoft.VisualStudio.TestStorage.TestStore.Add(TestBaseRecord record)
at Microsoft.VisualStudio.TestWindow.Host.TestRunSession.AddTestResult(TestResultRecord testResultRecord)
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at Microsoft.VisualStudio.Telemetry.WindowsErrorReporting.WatsonReport.GetClrWatsonExceptionInfo(Exception exceptionObject)
========== Test run finished: 2 Tests run in 1.1 sec (2 Passed, 0 Failed, 0 Skipped) ==========

@craigbroadman
Copy link

Ignore the previous comments - with a clean and rebuild and a restart of Visual Studio - they are all now running. Thank you for your help @OsirisTerje

@craigbroadman
Copy link

@OsirisTerje - FYI - I raised an issue with Microsoft to find out why Visual studio isn't picking up the runsettings file automatically...

the “Auto detect runsettings file” option picks up a file named “.runsettings” (not "simple.runsettings"). Details below...

https://developercommunity.visualstudio.com/content/problem/1033850/auto-detect-runsettings-file-is-not-working.html

@OsirisTerje
Copy link
Member

@craigbroadman Thanks!

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

Successfully merging a pull request may close this issue.

8 participants