TestOutputXml regressions

[mrvoorhe]

Our current setup is

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.2.0" />
<PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />

Using .NET Core 6.0.100.

We use a settings like that looks like

<?xml version="1.0" encoding="utf-8"?>
<RunSettings>
  <NUnit>
  <DiscoveryMethod>Legacy</DiscoveryMethod>
  <DisplayName>FullName</DisplayName>
    <Where>test == 'BuildPerformance.Tests.Windows.AveragedBuildPerformanceTests.DotsSample'</Where>
    <TestOutputXml>C:/UnitySrc/dev/il2cpp-7/results/TestResult_NUnitTest</TestOutputXml>
  </NUnit>
</RunSettings>

And the dotnet test command looks like

dotnet test C:\UnitySrc\dev\il2cpp-7\il2cpp.sln -p:SolutionDir="C:\UnitySrc\dev\il2cpp-7/" --no-build --no-restore --nologo --configuration Debug --settings "C:\UnitySrc\dev\il2cpp-7\results\RunSettings_NUnitTest.xml"

All Good.

I'm updating to net7 and have updated to the 7.0.100 sdk. Updated my packages

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.4.0" />
<PackageReference Include="NUnit3TestAdapter" Version="4.3.0" />

Everything else is the same.

Boom.

System.ArgumentNullException: Value cannot be null. (Parameter 'path1')
   at System.ArgumentNullException.Throw(String paramName)
   at System.IO.Path.Combine(String path1, String path2)
   at NUnit.VisualStudio.TestAdapter.AdapterSettings.SetTestOutputFolder() in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\AdapterSettings.cs:line 434
   at NUnit.VisualStudio.TestAdapter.AdapterSettings.Load(String settingsXml) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\AdapterSettings.cs:line 345
   at NUnit.VisualStudio.TestAdapter.AdapterSettings.Load(IDiscoveryContext context) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\AdapterSettings.cs:line 311
   at NUnit.VisualStudio.TestAdapter.NUnitTestAdapter.Initialize(IDiscoveryContext context, IMessageLogger messageLogger) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnitTestAdapter.cs:line 135
Exception System.ArgumentException,    Exception thrown executing tests in C:\UnitySrc\dev\il2cpp-s-1\il2cpp-s-1\Unity.IL2CPP.Debugger.Soft.Tests\bin\Debug\net7.0\Unity.IL2CPP.Debugger.Soft.Tests.dll
The value cannot be an empty string. (Parameter 'path')
   at System.ArgumentException.ThrowNullOrEmptyException(String argument, String paramName)
   at System.IO.Directory.CreateDirectory(String path)
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.CreateTestOutputFolder() in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 365
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.RunAssembly(String assemblyPath, IGrouping`2 testCases, TestFilter filter) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 275

I started poking around and found OutputXmlFolderMode. I added the following to our settings file

<OutputXmlFolderMode>AsSpecified</OutputXmlFolderMode>

Boom.

Exception System.ArgumentException,    Exception thrown executing tests in C:\UnitySrc\dev\il2cpp-s-1\il2cpp-s-1\Unity.IL2CPP.Debugger.Soft.Tests\bin\Debug\net7.0\Unity.IL2CPP.Debugger.Soft.Tests.dll
The value cannot be an empty string. (Parameter 'path')
   at System.ArgumentException.ThrowNullOrEmptyException(String argument, String paramName)
   at System.IO.Directory.CreateDirectory(String path)
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.CreateTestOutputFolder() in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 365
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.RunAssembly(String assemblyPath, IGrouping`2 testCases, TestFilter filter) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 275

The culprit appears to be holes in the logic added with #1001 cc @OsirisTerje

Depending on how you look at #1001 introduced a handful of quirks / bugs.

1. <WorkDirectory> is now required when using TestOutputXml and not using OutputXmlFolderMode. Otherwise NUnit will crash with the first set of exceptions.

2. Using TestOutputXml with an absolute path and <OutputXmlFolderMode>AsSpecified</OutputXmlFolderMode> is broken. The dependeny on WorkDirectory is avoided thanks to

   nunit3-vs-adapter/src/NUnitTestAdapter/AdapterSettings.cs

   Line 436 in d0f38dc

   case OutputXmlFolderMode.AsSpecified:

   however, that means TestOutputFolder is not uninitialized and the logic over in

   nunit3-vs-adapter/src/NUnitTestAdapter/NUnit3TestExecutor.cs

   Line 365 in d0f38dc

   Directory.CreateDirectory(path);

   cannot handle that. And you get the second exception.

3. There is a new invalid set of values that leads to an exception. If I make a mistake and have a relative TestOutputXml and use <OutputXmlFolderMode>AsSpecified</OutputXmlFolderMode> I get the following exception. Prior to Fix for #997 #1001 this mistake was not possible.

The value cannot be an empty string. (Parameter 'path')
   at System.ArgumentException.ThrowNullOrEmptyException(String argument, String paramName)
   at System.IO.Directory.CreateDirectory(String path)
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.CreateTestOutputFolder() in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 365
   at NUnit.VisualStudio.TestAdapter.NUnit3TestExecutor.RunAssembly(String assemblyPath, IGrouping`2 testCases, TestFilter filter) in D:\repos\NUnit\nunit3-vs-adapter\src\NUnitTestAdapter\NUnit3TestExecutor.cs:line 275

Is OutputXmlFolderMode.AsSpecified adding value? Would it be simpler to remove this value and add Path.IsRelative checks so that an absolute TestOutputXml value will always be respected regardless of the OutputXmlFolderMode value?

I was able to find a way around the problems introduced by #1001. I switched to using

<TestOutputXml>results/TestResult_NUnitTest</TestOutputXml>
    <WorkDirectory>C:/UnitySrc/dev/il2cpp-s-1/il2cpp-s-1</WorkDirectory>

And that got past the exceptions. The following also avoids the bugs

<TestOutputXml>C:/UnitySrc/dev/il2cpp-s-1/il2cpp-s-1/results/TestResult_NUnitTest</TestOutputXml>
    <WorkDirectory>C:/UnitySrc/dev/il2cpp-s-1/il2cpp-s-1</WorkDirectory>

Because Path.Combine(WorkDirectory, TestOutputXml) will return TestOutputXml if it is already an absolute path

[OsirisTerje]

@mrvoorhe Thanks! You're absolutely right, and the issues are also in #1027 and #1028. There is an alpha with the fixes here https://github.com/nunit/nunit3-vs-adapter/files/9965427/NUnit3TestAdapter.4.3.1-alpha.111.zip, please try this one and see if resolves it for you too. The plan is to release as soon as possible now, but we have failing tests as the fixes here hit a dormant NUnit bug that our tests triggered. See nunit/nunit#4255

[OsirisTerje]

Is OutputXmlFolderMode.AsSpecified adding value? Would it be simpler to remove this value and add Path.IsRelative checks so that an absolute TestOutputXml value will always be respected regardless of the OutputXmlFolderMode value?

Good point! There was a reason for it, I need to recheck my notes on that, but you might be right.

There is a new invalid set of values that leads to an exception. If I make a mistake and have a relative TestOutputXml and use AsSpecified I get the following exception. Prior to #1001 this mistake was not possible.

Dependent upon your previous point here, this needs to be fixed. Thanks!

[OsirisTerje]

@mrvoorhe The 4.3.1 alpha already has the fix you requested, so please try it and see if that solves the issues you see.

See this code:

nunit3-vs-adapter/src/NUnitTestAdapter/AdapterSettings.cs

Line 434 in fdf264e

if (Path.IsPathRooted(TestOutputXml))

[OsirisTerje]

Released here: https://www.nuget.org/packages/NUnit3TestAdapter/4.3.1

[mrvoorhe]

@OsirisTerje Thanks!