Fix Cabinet.Tests when run with xunit.console

[qmfrederik]

Environment.CurrentDirectory is not reliable when running within xunit.console - I've seen it being set to the output directory of other tests projects. This adds a manual workaround for that when detecting xunit.

[qmfrederik]

@AArnott I expect to get some pushbash on this change, but can't immediately come up with a better alternative.

Other than historical reasons, is there a specific motivation for this project not to use dotnet build and dotnet test (paired with global.json to pin to a specific SDK version)?

[AArnott]

is there a specific motivation for this project not to use dotnet build and dotnet test

I really like using dotnet build and dotnet test both locally and in CI for most of my repos. Sadly this one cannot, for reasons you can discover yourself if you try dotnet build. It doesn't support SDKs that are only available on full msbuild.exe. Building fails with a bunch of these errors:

C:\Users\andarno.nuget\packages\msbuild.sdk.extras\2.0.54\Build\Workarounds.targets(27,5): error : If you are building projects that require targets from full MSBuild or MSBuildFrameworkToolsPath, you need to use desktop msbuild ('msbuild.exe') instead of 'dotnet build' or 'dotnet msbuild' [D:\git\pinvoke\src\SetupApi\SetupApi.csproj]

As for testing, there's no reason I know of that we couldn't use dotnet test --no-build (after using msbuild.exe to build). I'm testing that locally now to see if the tests pass in the same way.

I expect to get some pushbash on this change

Quite right. :) Can you please use Assembly.GetExecutingAssembly().Location instead to find the directory where the test assembly actually is, and find the .cab file you need next to it that way?

[AArnott]

Using dotnet test --no-build produced 6 failed tests (as opposed to xunit's 1 that you're fixing here). dotnet test took 11 seconds compared to xunit's 3 seconds, and xunit's report was more compact and human readable. So not strong reasons, but at least the test failures would have to be resolved to switch.

[qmfrederik]

Unfortunately, Assembly.GetExecutionAssembly().Location (or typeof(CabinetTests).Assembly.Location) gets you this: %HOME%\AppData\Local\Temp\cce2d901-25de-494f-b0fc-de644ed773f5\cce2d901-25de-494f-b0fc-de644ed773f5\assembly\dl3\93016657\fabb8104_3d51d601\PInvoke.Cabinet.Tests.dll.

IIRC that's because the xunit runner uses shadow copies.

[AArnott]

OK, so we have two other options:

1. disable shadow copying (which can be done in app.config in the test project)
2. embed the .cab file and extract it to a file as part of the test.

I've used both approaches in various repos.

[AArnott]

I pushed option 1 to your branch. But the test still fails with:

 System.IO.FileNotFoundException : Could not find file 'D:\git\pinvoke\bin\debug\tests\Kernel32.Tests\net472\README.md'.
      Stack Trace:
           at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
           at System.IO.FileStream.Init(String path, FileMode mode, FileAccess access, Int32 rights, Boolean useRights, FileShare share, Int32 bufferSize, FileOptions options, SECURITY_ATTRIBUTES secAttrs, String msgPath, Boolean bFromProxy, Boolean useLongPath, Boolean checkHost)
           at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize)
           at System.IO.File.OpenFile(String path, FileAccess access, SafeFileHandle& handle)
           at System.IO.File.SetLastWriteTimeUtc(String path, DateTime lastWriteTimeUtc)
        src\Cabinet.Tests\CabinetFacts.cs(154,0): at CabinetFacts.ExtractNotify(NOTIFICATIONTYPE notificationType, NOTIFICATION* notification)
           at PInvoke.Cabinet.FDICopy(FdiHandle hfdi, String pszCabinet, String pszCabPath, Int32 flags, FNFDINOTIFY pfnfdin, Void* pfnfdid, Void* pvUser)
        obj\Cabinet\Debug\net45\Cabinet.4r7mIwcH.generated.cs(138,0): at PInvoke.Cabinet.FDICopy(FdiHandle hfdi, String pszCabinet, String pszCabPath, Int32 flags, FNFDINOTIFY pfnfdin, IntPtr pfnfdid, IntPtr pvUser)
        src\Cabinet.Tests\CabinetFacts.cs(80,0): at CabinetFacts.ExtractCabinetFileTest()

Is this because (I suggest in disbelief) the Cabinet APIs always extract the file to the process's current directory? There doesn't appear to be any parameter taken by FDICopy to specify where the extracted file should be placed.

[qmfrederik]

@AArnott Hah, I had this (based on the xunit documentation):

diff --git a/src/Directory.Build.props b/src/Directory.Build.props
index e2f9e38..dcf33aa 100644
--- a/src/Directory.Build.props
+++ b/src/Directory.Build.props
@@ -103,6 +103,7 @@
     <PackageReference Include="xunit.runner.console" Version="2.4.1" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.1" />
     <PackageReference Include="xunit" Version="2.4.1" />
+    <Content Include="$(MSBuildThisFileDirectory)xunit.runner.json" Link="xunit.runner.json" CopyToOutputDirectory="PreserveNewest"/>
   </ItemGroup>

   <Target Name="PrepareReleaseNotes" BeforeTargets="GenerateNuspec" DependsOnTargets="GetBuildVersion">
diff --git a/src/xunit.runner.json b/src/xunit.runner.json
new file mode 100644
index 0000000..533ef25
--- /dev/null
+++ b/src/xunit.runner.json
@@ -0,0 +1,4 @@
+{
+  "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json",
+  "shadowCopy": false
+}
\ No newline at end of file

Yours seems a bit more straightforward, so let's go with that.

[qmfrederik]

FDICopy only reasons in terms of relative paths, and passes these relative paths to FNOPEN. If you want to rewrite with the path, you can do so in CabinetFacts.Open, where path is passed as a parameter and then forwarded to Kernel32.CreateFile.

(Alternatively, you can also rewrite CabinetFacts to use memory streams or something entirely different to avoid hitting the disk at all).

[qmfrederik]

PS - Cabinet+CpuType is a testimony to how ancient this API is: it allows you to choose between a 286 and 386 instruction set.

[AArnott]

Alternatively, you can also rewrite CabinetFacts to use memory streams or something entirely different to avoid hitting the disk at all

Ooh! I like that idea. I'll give it a shot.

[AArnott]

My debugger isn't working well so I am not going to replace with memory stream right now. But I do think we have a stable test now.