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

NUnit3TestAdapter NuGet package hides included assets #1052

Open
Stefan75 opened this issue Feb 3, 2023 · 48 comments
Open

NUnit3TestAdapter NuGet package hides included assets #1052

Stefan75 opened this issue Feb 3, 2023 · 48 comments

Comments

@Stefan75
Copy link

Stefan75 commented Feb 3, 2023

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:
NuGet/Home#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 Stefan75 changed the title NUnit3TestAdapter NuGet package hides copied assets NUnit3TestAdapter NuGet package hides included assets Feb 3, 2023
@OsirisTerje
Copy link
Member

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.

@Stefan75
Copy link
Author

Stefan75 commented Feb 6, 2023

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...

@OsirisTerje
Copy link
Member

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.

@jeffkl
Copy link

jeffkl commented Feb 6, 2023

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.

@OsirisTerje
Copy link
Member

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

@Stefan75
Copy link
Author

Stefan75 commented Feb 7, 2023

@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

@jeffkl
Copy link

jeffkl commented Feb 7, 2023

@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.

@OsirisTerje
Copy link
Member

@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?

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

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.

@OsirisTerje
Copy link
Member

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.

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

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...

@manfred-brands
Copy link
Member

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

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

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"
      ]
    }
  }
}

@OsirisTerje
Copy link
Member

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
Copy link
Member

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 !

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

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".

@OsirisTerje
Copy link
Member

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" />

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

Yeah that sounds right to me

@OsirisTerje
Copy link
Member

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

@manfred-brands
Copy link
Member

Try the runtimes folder instead as mentioned above.

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

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

@OsirisTerje
Copy link
Member

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

@OsirisTerje
Copy link
Member

So the buildAction adn copytoutput have to be set somehow

@OsirisTerje
Copy link
Member

OsirisTerje commented Jul 3, 2023

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

@manfred-brands
Copy link
Member

@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.

@OsirisTerje
Copy link
Member

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

@manfred-brands
Copy link
Member

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

@OsirisTerje
Copy link
Member

This did it:

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

@OsirisTerje
Copy link
Member

OsirisTerje commented Jul 3, 2023

@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.

@manfred-brands
Copy link
Member

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

@OsirisTerje
Copy link
Member

OsirisTerje commented Jul 3, 2023

Ahh, enlighten me!
Will that still be detected by nuget as an asset ?
Is that a similar structure as the content?
(End of day here, no time to check)
Pushed up branch Issue1052b with the changes. Feel free to suggest

@manfred-brands
Copy link
Member

See my comment from 1 hour ago. #1052 (comment) which has a link to the nuget folder names.

You only need to rename the folder name.

@OsirisTerje
Copy link
Member

Thanks @manfred-brands ! Worked a bit on high speed here, though I was only chatting with Jeff :-) until 7 minutes ago

I'll fix that tomorrow and verify it works. It makes sense to me. @jeffkl : Can you confirm it will work from your side?

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

The runtimes folder is designed for runtime specific implementations of assemblies that are included in a package. In this case, we just want the test adapter DLL copied to the output directory so that the plug-in discovery mechanism of Visual Studio Test can find it. I think contentFiles is the better solution here since we aren't having the test adapter DLL passed to the compiler and the code in my project doesn't call into the test adapter. Its just a plug-in and nothing more.

@OsirisTerje
Copy link
Member

@jeffkl what about support for the older csproj format?

@manfred-brands
Copy link
Member

@jeffkl This is exactly what the runtimes folder is for. Code that is needed at runtime, in this case by the nunit adapter. As mentioned content files is not supported by old style .csproj using package.json files and would require 2 copies. The solution at https://stackoverflow.com/questions/47468941/prevent-duplicating-files-in-nuget-content-and-contentfiles-folders is a manual copy taking us back to square 1.

@OsirisTerje
Copy link
Member

@manfred-brands package.json? Do you mean the legacy project system or the shortlived package.json in early dotnet. I dont think we need to support that, but supportong legacy csproj would be good as long as it works.

@jeffkl
Copy link

jeffkl commented Jul 3, 2023

@jeffkl This is exactly what the runtimes folder is for

That is not correct. A "normal" NuGet package is used to pass around a class library that your code can consume. For example, I have a package that contains a class library for logging. I name my package Jeff.Loging and inside I have an Jeff.Logging.dll which contains the implementation of my logging framework.

lib\net45\Jeff.Logging.dll

Now consider another project is consuming the package. At restore time, NuGet pulls down the package and unzips it, then passes the path of Jeff.Logging.dll to the C# compiler for type checking to ensure that the consumer is only calling the available APIs in the logging framework. The .NET SDK also copies Jeff.Logging.dll to the output directory so that the .NET runtime will be able to locate it, load it, and allow my consumer EXE to call into it.

There are also "reference" assemblies, which are created and compilation time with dynamically generated code which represents only the public facing APIs. Reference assemblies can help reduce rebuilds for large repositories since the .NET SDK is only taking into account if a public API changed before rebuilding the closure of projects. These assemblies can be included in a package as well in the ref folder.

lib\net45\Jeff.Logging.dll
ref\net45\Jeff.Logging.dll

Now when a consumer uses my package, the ref\net45\Jeff.Logging.dll is passed to the compiler since it contains all of the public facing API and is enough for all of the type checking needed. But the .NET SDK then copies the lib\net45\Jeff.Logging.dll to the output directory because this assembly actually contains my implementation.

Now consider I want to have different implementations for runtime identifiers (RIDs), for example Linux vs Windows. I would compile my assembly once for each RID and include the RID-specific implementations in my package with these folders:

runtimes\win10-x64\net45\Jeff.Logging.dll
runtimes\linux-x64\net45\Jeff.Logging.dll
ref\net45\Jeff.Logging.dll

This means that during compilation, the ref\net45\Jeff.Logging.dll is passed to the compiler for type checking because it contains all of the public APIs of my library but the .NET SDK will determine what RID my application is targeting and copy the runtime specific implementation of the library. Assemblies under the lib folder can only specify a specific target framework and not a runtime. In this case the assembly in lib or ref is only used during compilation for type checking.

The NUnit test adapter is needed at runtime, but is not a runtime specific implementation of a test adapter. The code it contains will work for any runtime (operating system). You can include it in the runtimes folder and it will end up in the output directory, but I still don't think the NUnit test adapter really fits this model. Instead, it is a plug-in for the Visual Studio Test infrastructure and so we just want NuGet to copy it to the output directory. It is not a reference assembly or anything that my code is actually consuming.

As mentioned content files is not supported by old style .csproj using package.json files and would require 2 copies.

project.json was deprecated more than 8 years ago. The telemetry in Visual Studio shows it is not used very much at all. I'm not sure if its worth making the package support this technology if so few people are using it. If they're using such an older deprecated technology, I doubt they're using the latest and greated NUnit test adapter package.

I tried my example with a "legacy" csproj project and it worked fine with PackageReference but not with packages.config. Content files are not supported in packages.config and a custom build\MyPackage.props to copy stuff to the output directory is the only good solution. You could still include the build\MyPackage.props but disable the logic if the package is using PackageReference. I'm not sure how many people are using the latest and greated NUnit test adapter but are also using packages.config when packages.config is considered legacy.

@manfred-brands
Copy link
Member

As said before runtimes is for binaries needed at runtime. They will not be passed as reference for building your project.

Just like with content files, one can use the any for the runtimes folder to indicate that the files are runtime independent.

@OsirisTerje
Copy link
Member

@jeffkl Is there any problems related to using the runtimes/any as @manfred-brands suggests? To me it looks semantically more correct than using contentfiles. (I understand it is opiniated). If the two has the same end effect, and the runtimes works for legacy projects too, then that seems to be a more simple solution.

@OsirisTerje OsirisTerje added this to the 4.6 milestone Jul 4, 2023
@OsirisTerje
Copy link
Member

@Stefan75 Can you confirm the enclosed package works for you?
NUnit3TestAdapter.4.6.0-alpha.3.zip

@OsirisTerje
Copy link
Member

@manfred-brands Tried using runtimes, didn't work.

@OsirisTerje
Copy link
Member

@jeffkl You mentioned a condition for the props file for legacy projects, what would that be?

@jeffkl
Copy link

jeffkl commented Jul 5, 2023

NuGet adds imports to the project when you install a package that contains build extensions.

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <Import Project="packages\My.TestAdapter.1.0.0\build\My.TestAdapter.props" Condition="Exists('packages\My.TestAdapter.1.0.0\build\My.TestAdapter.props')" />
  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
  ...
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
  <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
    <PropertyGroup>
      <ErrorText>This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
    </PropertyGroup>
    <Error Condition="!Exists('packages\My.TestAdapter.1.0.0\build\My.TestAdapter.props')" Text="$([System.String]::Format('$(ErrorText)', 'packages\My.TestAdapter.1.0.0\build\My.TestAdapter.props'))" />
    <Error Condition="!Exists('packages\My.TestAdapter.1.0.0\build\My.TestAdapter.targets')" Text="$([System.String]::Format('$(ErrorText)', 'packages\My.TestAdapter.1.0.0\build\My.TestAdapter.targets'))" />
  </Target>
  <Import Project="packages\My.TestAdapter.1.0.0\build\My.TestAdapter.targets" Condition="Exists('packages\My.TestAdapter.1.0.0\build\My.TestAdapter.targets')" />
</Project>

I guess the best thing to condition on is the presence of a packages.config file. Legacy C# projects only really support targeting .NET Framework so you'll just want to add <Content /> items for the .NET Framework assembly.

I tried this and it worked:

<Project>
  <ItemGroup Condition="Exists('$(MSBuildProjectDirectory)\packages.config')">
    <Content Include="$(MSBuildThisFileDirectory)..\contentFiles\any\net45\*.dll">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
      <Link>%(RecursiveDir)%(Filename)%(Extension)</Link>
      <Visible>false</Visible>
    </Content>
  </ItemGroup>
</Project>

image

The build logic will always be imported regardless of the target framework of the project. You could go a step further and put the .props/.targets file in build\net45 but the condition is technically filtering out the <Content /> item if the project isn't using packages.config.

Let me know if you want me to create a sample you can interact with.

@OsirisTerje
Copy link
Member

Thanks @jeffkl ! I'll try that suggestion!

Another thing I noticed is that it seems VIsual Studio show these content files at a projects root level, although they are not present there as files.
image

@jeffkl
Copy link

jeffkl commented Jul 6, 2023

Sorry this looks like the current design by NuGet at the moment. It generates a <None /> item but doesn't set Visible="false".

You can work around this in your .targets with an Update gesture:

<ItemGroup>
  <None Update="@(None->WithMetadataValue('NuGetPackageId', 'My.TestAdapter'))" Visible="false" />
</ItemGroup>

image

@jeffkl
Copy link

jeffkl commented Jul 6, 2023

Forgot to link existing issue: NuGet/Home#4856

@OsirisTerje
Copy link
Member

Quite a long lived issue NuGet/Home#4856 . Is it even planned to be done?
What is the purpose of adding content files that way? It looks more like a bug than a feature.

However, if I think about a package that is intended to ADD content to a project, it may make sense. That is also why I feel that contentfiles is not quite right for a package containing non-content like this.

I'll try the updategesture too, it might be the solution to that issue. And, since the adapter rarely (if ever) is added into anything but the root test projects and not passed around in libraries, it should work.

@OsirisTerje OsirisTerje removed this from the 4.6 milestone Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants