nunit3-vs-adapter icon indicating copy to clipboard operation
nunit3-vs-adapter copied to clipboard

NUnit3TestAdapter NuGet package hides included assets

Open Stefan75 opened this issue 2 years ago • 48 comments

Hello,

I have created a project with a dependency to following NuGet package: NUnit3TestAdapter 4.3.1

After restore and build I observed that the created project.assets.json file does not contain the assets of the package:

nunit.engine.api.dll
nunit.engine.core.dll
nunit.engine.dll
NUnit3.TestAdapter.dll
NUnit3.TestAdapter.pdb
Project.dll
Project.pdb
testcentric.engine.metadata.dll

This leads to missing assets during a SBOM creation, or we would miss the files we want to sign.

I have created a NuGet issue for it: https://github.com/NuGet/Home/issues/12406

This issue has been closed with following comment:

The only asset consumed by NuGet is build/net35/NUnit3TestAdapter.props. If the package author used contentFiles instead, they would be listed in the assets file. I suggest you file an issue against the package author to make this package more compatible with SBOM.

Please have a look to the issue, maybe the nuspec file could easily improved so that all assets are listed.

Thanks, Stefan

Stefan75 avatar Feb 03 '23 18:02 Stefan75

Not suite sure what @jeffkl is pointing to. The nuspec file contains all the files needed. The props file are just there to cover the different targets for MSBuild, so not sure why they would include that file, and not the Files listed in the nuspec. Content files are normally other non-executable files, (ref https://devblogs.microsoft.com/nuget/nuget-contentfiles-demystified/) so don't think it is wise to include the executables there. Suggest @jeffkl elaborates on this.

OsirisTerje avatar Feb 05 '23 20:02 OsirisTerje

Hello,

thanks for the fast response.

I am not an expert, my I understand @jeffkl in a way that the current nuspec is using a more "low-level" way via none to copy the required files to the output directory:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <ItemGroup>
    <None Include="$(MSBuildThisFileDirectory)NUnit3.TestAdapter.dll">
      <Link>NUnit3.TestAdapter.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)NUnit3.TestAdapter.pdb" Condition="Exists('$(MSBuildThisFileDirectory)NUnit3.TestAdapter.pdb')">
      <Link>NUnit3.TestAdapter.pdb</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)nunit.engine.dll">
      <Link>nunit.engine.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)nunit.engine.api.dll">
      <Link>nunit.engine.api.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)nunit.engine.core.dll">
      <Link>nunit.engine.core.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
    <None Include="$(MSBuildThisFileDirectory)testcentric.engine.metadata.dll">
      <Link>testcentric.engine.metadata.dll</Link>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Visible>False</Visible>
    </None>
  </ItemGroup>
</Project>

It seems for me that the asset creation does not "monitor" such low-level operations to collect all assets.

In my understanding by the usage of nuspec contentfiles those files becomes visible to the NuGet client and not just to msbuild itself.

I will ask jeffkl for it...

Stefan75 avatar Feb 06 '23 07:02 Stefan75

This section is already in the nuspec

<files>
    <file src="build\net35\NUnit3.TestAdapter.dll" target="build\net35\NUnit3.TestAdapter.dll" />
    <file src="build\net35\NUnit3.TestAdapter.pdb" target="build\net35\NUnit3.TestAdapter.pdb" />
    <file src="build\net35\nunit.engine.dll" target="build\net35\nunit.engine.dll" />
    <file src="build\net35\nunit.engine.api.dll" target="build\net35\nunit.engine.api.dll" />
    <file src="build\net35\nunit.engine.core.dll" target="build\net35\nunit.engine.core.dll" />
    <file src="build\net35\testcentric.engine.metadata.dll" target="build\net35\testcentric.engine.metadata.dll"/>
    <file src="build\net35\NUnit3TestAdapter.props" target="build\net35\NUnit3TestAdapter.props" />

    <file src="build\netcoreapp3.1\NUnit3.TestAdapter.dll" target="build\netcoreapp3.1\NUnit3.TestAdapter.dll" />
    <file src="build\netcoreapp3.1\NUnit3.TestAdapter.pdb" target="build\netcoreapp3.1\NUnit3.TestAdapter.pdb" />
    <file src="build\netcoreapp3.1\nunit.engine.dll" target="build\netcoreapp3.1\nunit.engine.dll" />
    <file src="build\netcoreapp3.1\nunit.engine.api.dll" target="build\netcoreapp3.1\nunit.engine.api.dll" />
    <file src="build\netcoreapp3.1\nunit.engine.core.dll" target="build\netcoreapp3.1\nunit.engine.core.dll" />
    <file src="build\netcoreapp3.1\testcentric.engine.metadata.dll" target="build\netcoreapp3.1\testcentric.engine.metadata.dll"/>
    <file src="build\netcoreapp3.1\NUnit3TestAdapter.props" target="build\netcoreapp3.1\NUnit3TestAdapter.props" />
  </files>

This should be used.

Adding executables in a contentFiles section doesn't seem correct.

OsirisTerje avatar Feb 06 '23 10:02 OsirisTerje

There are two .nuspecs in NuGet. The one checked into this repository, tells NuGet how to pack up a package. This one contains all of the files so that they end up in the .nupkg. The .nuspec is then added to the .nupkg but files that are already in the .nupkg. are stripped out because they don't take part in restore.

The .nupsec in the .nupkg looks like this:

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd">
  <metadata>
    <id>NUnit3TestAdapter</id>
    <version>4.3.1</version>
    <title>NUnit3 Test Adapter for Visual Studio and DotNet</title>
    <authors>Charlie Poole, Terje Sandstrom</authors>
    <requireLicenseAcceptance>false</requireLicenseAcceptance>
    <license type="expression">MIT</license>
    <licenseUrl>https://licenses.nuget.org/MIT</licenseUrl>
    <projectUrl>https://docs.nunit.org/articles/vs-test-adapter/Index.html</projectUrl>
    <iconUrl>https://cdn.rawgit.com/nunit/resources/master/images/icon/nunit_256.png</iconUrl>
    <description>The NUnit3 TestAdapter for Visual Studio, all versions from 2012 and onwards, and DotNet (incl. .Net core).

      Note that this package ONLY contains the adapter, not the NUnit framework.
      For VS 2017 and forward, you should add this package to every test project in your solution. (Earlier versions only require a single adapter package per solution.)</description>
    <summary>NUnit3 adapter for running tests in Visual Studio and DotNet. Works with NUnit 3.x, use the NUnit 2 adapter for 2.x tests.</summary>
    <releaseNotes>See https://docs.nunit.org/articles/vs-test-adapter/Adapter-Release-Notes.html</releaseNotes>
    <copyright>Copyright (c) 2011-2021 Charlie Poole, 2014-2022 Terje Sandstrom</copyright>
    <language>en-US</language>
    <tags>test visualstudio testadapter nunit nunit3 dotnet</tags>
    <repository type="git" url="https://github.com/nunit/nunit3-vs-adapter" />
  </metadata>
</package>

When the package is restored, NuGet looks at the folders in the package and sees a build folder, a folder for the target framework, and a file named NUnit3TestAdapter.props:

image

So in this case, the only "asset" that NuGet sees during restore, is the .props file so an import is added. This import, NUnit3TestAdapter.props, implements custom logic to copy the files to the output directory. Since these files are placed on disk using custom logic, NuGet does not report their existence in its own assets file because it does not know what they are for.

This package could instead place the files in contentFiles and indicate that they need to be copied to the output directory, and NuGet would generate the appropriate <None /> items with CopyToOutputDirectory=PreserveNewest. The contentFiles would also show up in the project.assets.json file because NuGet would know they are content files. This would help @Stefan75 detect which files take place in a build for SBOM purposes.

That said, the package still functions correctly, it just implements content files in a custom way so NuGet does not report them in the assets file after restore.

jeffkl avatar Feb 06 '23 20:02 jeffkl

@jeffkl Ok, but I assume you then mean the 4 "engine" files, not the adapter.dll ?

OsirisTerje avatar Feb 07 '23 12:02 OsirisTerje

@OsirisTerje

From my side, I talk about all files. Because I have following use-cases in mind:

  • Creation of a SBOM where we know the original of all files in our output dir
  • Further processing of those files (e.g. code signing)

Regards, stefan

Stefan75 avatar Feb 07 '23 16:02 Stefan75

@jeffkl Ok, but I assume you then mean the 4 "engine" files, not the adapter.dll ?

All of the files could be just content files that get copied to the output directory. The way the Visual Studio test adapter logic works, is it just looks in the output directory for files named *.TestAdapter.dll. So the goal of this package is really to just place the files in the output directory and contentFiles would work just fine.

jeffkl avatar Feb 07 '23 18:02 jeffkl

@jeffkl I looked at https://learn.microsoft.com/en-us/nuget/reference/nuspec#using-the-contentfiles-element-for-content-files but it seems to only work for non-framework related stuff. We have two different frameworks included. How would you suggest that to be specified?

OsirisTerje avatar Jul 03 '23 17:07 OsirisTerje

The Package Folder Structure section gives examples on how to specify target framework-specific assets.

/contentFiles/{codeLanguage}/{TxM}/{any?}

So if you place files under /contentFiles/any/net45, they would be copied to the output directory for any project that targets a framework compatible with net45.

jeffkl avatar Jul 03 '23 18:07 jeffkl

Ok, so to paraphrase, I need to copy the output from either release or debug, whatever I build, into a folder named according to the structure above. The same with any other files I need. I can try this, but it doesn't look right.

OsirisTerje avatar Jul 03 '23 18:07 OsirisTerje

In order to correctly do this, you'll need to do the following:

  1. Pack the appropriate assets into the correct location inside the .nupkg by telling NuGet which files go where
  2. Have the .nuspec that ends up in the .nupkg have the appropriate information to tell NuGet that files should be copied out of the .nupkg during restore.

Let me type up a quick sample...

jeffkl avatar Jul 03 '23 18:07 jeffkl

If the files are needed for building, they should be in the build folder, if they are only needed at runtime they should be in the runtimes folder. If both it should be in the lib folder. See https://learn.microsoft.com/en-us/nuget/create-packages/creating-a-package#from-a-convention-based-working-directory

manfred-brands avatar Jul 03 '23 18:07 manfred-brands

Here's an example:

Project.csproj:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net45</TargetFrameworks>
    
    <!-- Tells NuGet to not include the assemblies built by this project in the normal location of lib\TargetFramework since we want them to be in contentFiles -->
    <IncludeBuildOutput>false</IncludeBuildOutput>
    
    <!-- Tells NuGet to run a custom target when gathering files to be included in the package -->
    <TargetsForTfmSpecificContentInPackage>AddOutputAssembliesToPackage</TargetsForTfmSpecificContentInPackage>
    
    <!--
      NU5100 - The assembly 'contentFiles\any\net45\MyAssembly.dll' is not inside the 'lib' folder and hence it won't be added as a reference when the package is installed into a project. Move it into the 'lib' folder if it needs to be referenced.
        This is okay since this package only needs to copy its outputs to the consumers directory so that the Visual Studio test extensions can find it
      
      NU5128: Some target frameworks declared in the dependencies group of the nuspec and the lib/ref folder do not have exact matches in the other location. Consult the list of actions below: 
      - Add lib or ref assemblies for the net45 target framework
      - Add lib or ref assemblies for the netstandard2.0 target framework
        NuGet wants to keep the package quality high and doesn't understand that the package layout is correct, it is a utility package whose purpose is to place files in the consumer project's output directory.
     -->
    <NoWarn>NU5100;NU5128</NoWarn>
  </PropertyGroup>
  
  <Target Name="AddOutputAssembliesToPackage"
          DependsOnTargets="GetTargetPath">
    <ItemGroup>
        <!--
            Adds the @(TargetPathWithTargetPlatformMoniker) items which represent the assemblies built by this project
            to the TfmSpecificPackageFile item group which represents the files to be included in the package.
            The files are placed in the package under contentFiles/any/$(TargetFramework) and NuGet is told that they need
            to be copied to the output directory during restore.
        -->
        <TfmSpecificPackageFile Include="%(TargetPathWithTargetPlatformMoniker.Identity)"
                                PackagePath="contentFiles/any/$(TargetFramework)"
                                PackageCopyToOutput="true" />
    </ItemGroup>
  </Target>
</Project>

When you call Pack, the package has the following contents:

image

Also, the .nupsec inside of the .nupkg tells NuGet to copy the files to the output directory: image

The consumer of the package looks like this:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="My.TestAdapter" Version="1.0.0" />
  </ItemGroup>
</Project>

And its output folder now contains My.TestAdapter.dll:

D:\REPROS\NUGETCONTENTFILES\CONSUMER\BIN
└───Debug
    └───net7.0
            consumer.deps.json
            consumer.dll
            consumer.pdb
            My.TestAdapter.dll

More importantly, the project.assets.json of the consuming project now shows these assets as being consumed:

{
  "version": 3,
  "targets": {
    "net7.0": {
      "My.TestAdapter/1.0.0": {
        "type": "package",
        "contentFiles": {
          "contentFiles/any/netstandard2.0/My.TestAdapter.dll": {
            "buildAction": "None",
            "codeLanguage": "any",
            "copyToOutput": true,
            "outputPath": "My.TestAdapter.dll"
          }
        }
      }
    }
  },
  "libraries": {
    "My.TestAdapter/1.0.0": {
      "sha512": "U3s2sbNJXQYvARkHkwKzOGlnpgZ+92+JGJ3corKA9HqLMNfoAzC/0DBeeiW9ZyBdUYs52pul2ikJOA83KbxR1Q==",
      "type": "package",
      "path": "my.testadapter/1.0.0",
      "files": [
        ".nupkg.metadata",
        "contentFiles/any/net45/My.TestAdapter.dll",
        "contentFiles/any/netstandard2.0/My.TestAdapter.dll",
        "my.testadapter.1.0.0.nupkg.sha512",
        "my.testadapter.nuspec"
      ]
    }
  }
}

jeffkl avatar Jul 03 '23 19:07 jeffkl

Just FYI: The folder structure I have now is like this: image Note that each package version is given its own folder, that folder is called PACKAGE_IMAGE_DIR and is input to NugetPack as shown here (note, we're using Cake, but that's just a wrapper around this)

 NuGetPack("nuget/NUnit3TestAdapter.nuspec", new NuGetPackSettings()
        {
            Version = packageVersion,
            BasePath = PACKAGE_IMAGE_DIR,
            OutputDirectory = PACKAGE_DIR
        });

The PACKAGE_DIR is the \package folder itself.

When I try to do something like:

  <contentFiles>
          <files include="build\any\net462\NUnit3.TestAdapter.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\net462\NUnit3.TestAdapter.pdb" buildAction="Content" copyToOutput="true" />
          <files include="build\any\net462\nunit.engine.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\net462\nunit.engine.api.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\net462\nunit.engine.core.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\net462\testcentric.engine.metadata.dll" buildAction="Content" copyToOutput="true"/>
          
          <files include="build\any\netcoreapp3.1\NUnit3.TestAdapter.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\netcoreapp3.1\NUnit3.TestAdapter.pdb" buildAction="Content" copyToOutput="true" />
          <files include="build\any\netcoreapp3.1\nunit.engine.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\netcoreapp3.1\nunit.engine.api.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\netcoreapp3.1\nunit.engine.core.dll" buildAction="Content" copyToOutput="true" />
          <files include="build\any\netcoreapp3.1\testcentric.engine.metadata.dll" buildAction="Content" copyToOutput="true"/>
     </contentFiles>

Nothing comes out in the package

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

Ok, so this means we need to change the csproj for the adapter, and then skip the nuspec file, include the properties there into the csproj, then change to using dotnet pack, which means a change to the cake script too.

Summer fun it seems ::-) Thanks @jeffkl !

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

There are probably other ways to do it, this is just an example if you're relying soley on the built-in CSPROJ logic. In the end, you just want the layout of the package to like my example and the .nuspec that ends up in the package to have the same <files /> section. And then it should "just work".

jeffkl avatar Jul 03 '23 19:07 jeffkl

That could possibly be achieved by changing the existing "target" in the nuspec file we use now into "contentfiles/any/......" instead of as now adding it to the "build" target folder.

  <file src="build\net462\NUnit3.TestAdapter.dll" target="build\net462\NUnit3.TestAdapter.dll" />

=>

    <file src="build\net462\NUnit3.TestAdapter.dll" target="contentfiles\any\net462\NUnit3.TestAdapter.dll" />

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

Yeah that sounds right to me

jeffkl avatar Jul 03 '23 19:07 jeffkl

Well, it is close, but not quite there: image And the compiler complains with CS2015 image

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

Try the runtimes folder instead as mentioned above.

manfred-brands avatar Jul 03 '23 19:07 manfred-brands

Does the BuildContent attribute need to be None like in my sample?

jeffkl avatar Jul 03 '23 19:07 jeffkl

Yes, I think so, it now comes out as: image

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

So the buildAction adn copytoutput have to be set somehow

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

I don't think using the Files (assembly) section in nuspec will work, there is no way to set any more properties there, so it seems your mechanism with the custom target in csproj is the way to go

OsirisTerje avatar Jul 03 '23 19:07 OsirisTerje

@OsirisTerje actions are automatically set depending on target folder name: build vs lib vs runtimes vs content.

Note also that if you need to support nunit projects in old .csproj format using package.json you need to also use the content folder doubling the size of the .nupkg file.

manfred-brands avatar Jul 03 '23 20:07 manfred-brands

I don't need to support old csproj format, only the new SDK format.
But perhaps the hint here can do it: https://devblogs.microsoft.com/nuget/nuget-contentfiles-demystified/#second-example-adding-files-from-other-locations

OsirisTerje avatar Jul 03 '23 20:07 OsirisTerje

It is not what format you use but what your consumers use. Not everyone has upgraded their framework projects to the new format.

manfred-brands avatar Jul 03 '23 20:07 manfred-brands

This did it:

 <contentFiles>
         <files include="any/**/*.*" buildAction="None" copyToOutput="true" />
    </contentFiles>

OsirisTerje avatar Jul 03 '23 20:07 OsirisTerje

@manfred-brands You're right there, but if we are to support the old format going forward we either need to a) double the content, b) have two different packages created from same source c) maintain a 4.X version for old format and 5.X for the new, and backport changes.
We're also now only supporting framework 4.6.2 and upwards, so the old format and old framework is on its last leg.

OsirisTerje avatar Jul 03 '23 20:07 OsirisTerje

@OsirisTerje Hence my suggestion to use the runtimes folder instead which is supported by both.

manfred-brands avatar Jul 03 '23 20:07 manfred-brands