-
Notifications
You must be signed in to change notification settings - Fork 106
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
Comments
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. |
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 <?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 I will ask jeffkl for it... |
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. |
There are two The <?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 So in this case, the only "asset" that NuGet sees during restore, is the This package could instead place the files in 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 Ok, but I assume you then mean the 4 "engine" files, not the adapter.dll ? |
From my side, I talk about all files. Because I have following use-cases in mind:
Regards, |
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 |
@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? |
The Package Folder Structure section gives examples on how to specify target framework-specific assets.
So if you place files under |
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. |
In order to correctly do this, you'll need to do the following:
Let me type up a quick sample... |
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 |
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 ! |
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 |
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.
=>
|
Yeah that sounds right to me |
Try the runtimes folder instead as mentioned above. |
Does the |
So the buildAction adn copytoutput have to be set somehow |
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 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. |
I don't need to support old csproj format, only the new SDK format. |
It is not what format you use but what your consumers use. Not everyone has upgraded their framework projects to the new format. |
This did it:
|
@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. |
@OsirisTerje Hence my suggestion to use the |
Ahh, enlighten me! |
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. |
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? |
The |
@jeffkl what about support for the older csproj format? |
@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. |
@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. |
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
Now consider another project is consuming the package. At restore time, NuGet pulls down the package and unzips it, then passes the path of 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
Now when a consumer uses my package, the 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:
This means that during compilation, the 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
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 |
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 |
@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. |
@Stefan75 Can you confirm the enclosed package works for you? |
@manfred-brands Tried using runtimes, didn't work. |
@jeffkl You mentioned a condition for the props file for legacy projects, what would that be? |
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 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> The build logic will always be imported regardless of the target framework of the project. You could go a step further and put the Let me know if you want me to create a sample you can interact with. |
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. |
Sorry this looks like the current design by NuGet at the moment. It generates a You can work around this in your <ItemGroup>
<None Update="@(None->WithMetadataValue('NuGetPackageId', 'My.TestAdapter'))" Visible="false" />
</ItemGroup> |
Forgot to link existing issue: NuGet/Home#4856 |
Quite a long lived issue NuGet/Home#4856 . Is it even planned to be done? 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. |
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: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:
Please have a look to the issue, maybe the nuspec file could easily improved so that all assets are listed.
Thanks,
Stefan
The text was updated successfully, but these errors were encountered: