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

Add .NET 8 build for nunit-agent #1374

Closed
mlessmann opened this issue Dec 22, 2023 · 35 comments
Closed

Add .NET 8 build for nunit-agent #1374

mlessmann opened this issue Dec 22, 2023 · 35 comments
Milestone

Comments

@mlessmann
Copy link

mlessmann commented Dec 22, 2023

We are currently migrating to .NET 8 and are having trouble executing NUnit 3.13 tests with the NUnit Console 3.16.3. There are some errors at test runtime that occur because the test is run on the .NET 7 build of nunit-agent. Specifically, the test depends on a particular version of a DLL that is only present in .NET 8.

Would it be possible to create a .NET 8 build of the nunit-agent?

I have already created branches from main and from the 3.16.3 tag and changed code and project files. I haven't touched the pipelines.

@ilCosmico
Copy link

After updating my UnitTesting project from .NET6/.NET7 to .NET8 I get the following errors.

image

Is this the same issue as yours?

I'm using NUnit 3.13.3 and console runner 3.16.3 too.

@mlessmann
Copy link
Author

They were different assemblies (not WinForms), but otherwise I had the same errors.

@OsirisTerje
Copy link
Member

Can you please try to run version 3.15.5 instead.

@mlessmann
Copy link
Author

Using 3.15.5 leads to the same error messages:

Unable to load one or more of the requested types.
Could not load file or assembly 'System.Runtime, Version=8.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified.

@ilCosmico
Copy link

I confirm I get the same System.Runtime error when using 3.15.5.

@OsirisTerje
Copy link
Member

OsirisTerje commented Jan 10, 2024

Thanks @ilCosmico and @mlessmann .

I am on travel now, and have small opportunities to do changes myself. I can review, merge and get stuff out if you provide the code for this.

The engine should be good for .net 8, that was done some time ago, and works for the adapter. So there is then the console app that remains.

@mlessmann It seems you have found the reasons, and also, providing an agent for .net 8 should be trivial. But, need you to do the changes on the version315 branch.

The process for the console/engine release is not trivial however, but having the code in there in a PR would certainly make it easier for us to get it out.

I've created a milestone 3.17 for this issue. The branch name will remain version315 until I can get these sorted out.

PS: I assume you guys now that using dotnet test with the NUnit3Testadapter works for .net8? If you can use that instead of the console you should be unblocked. I know there might be reasons for using the console, so we want to have this fixed too.

@mlessmann
Copy link
Author

mlessmann commented Jan 10, 2024

I have created a PR that adds .NET 8 builds for the console and the agent. My .NET 8 tests run successfully with this version locally.
I have tried to adjust the cake scripts, but I'm unfamiliar with them and would appreciate feedback.

#1378

@OsirisTerje
Copy link
Member

The scripts look ok to me. I'll try to use them "as is", and we see how it goes.

@OsirisTerje
Copy link
Member

@mlessmann Did you get a series of errors like this one:

C:\repos\nunit\nunit-console\src\NUnitEngine\nunit.engine.tests\nunit.engine.tests.csproj : error NU1902: Warning As Er
ror: Package 'Microsoft.NETCore.App' 2.1.0 has a known moderate severity vulnerability, https://github.com/advisories/G
HSA-vgwq-hfqc-58wv [C:\repos\nunit\nunit-console\NUnitConsole.sln]

@mlessmann
Copy link
Author

@OsirisTerje Yes. That's why I added these entries to the affected csproj files:
<NuGetAudit Condition="'$(TargetFramework)'=='netcoreapp2.1'">false</NuGetAudit>
Unfortunately, the condition only seems to work in Visual Studio. I've created #1379 to fix this.

@ilCosmico
Copy link

@mlessmann thanks for the great job!
@OsirisTerje so do we need to wait for 3.17 to be able running .NET8 unit testing?

@OsirisTerje
Copy link
Member

@mlessmann Thanks!
@ilCosmico Yes, and it will be out with only this issue. So, as soon as we can make it work.

@OsirisTerje
Copy link
Member

@mlessmann @ilCosmico
I have created a pre-release at https://github.com/nunit/nunit-console/releases/tag/v3.17.0-dev.1

Please verify if these are working as they should. I am on travel tomorrow, so next week is probably best time for a prod release.
If you only need nuget packages I might be able to upload them, but anyone else is harder to get done to Choco. Getting them up on github however, is easier. Please advise what you need.

PS: Note that two of the components didnt come out with the -dev suffix. Be aware of these wrt later upgrades.

@ilCosmico
Copy link

Using the new console runner I get the following error. Am I missing something?

image

@OsirisTerje
Copy link
Member

@ilCosmico It doesn't look related, but there are multiple similar issues like this one. It happens on the exit, so should not be a problem. The current issue however seems to be solved, right?

@ilCosmico
Copy link

Yes, right!

@mlessmann
Copy link
Author

I'm also getting the error above. The reason for the agent process crashing is that it uses the BinaryFormatter, which by default throws an exception in .NET 8. There is a compatibility flag that re-enables the BinaryFormatter. I've opened #1382 for this.
Unfortunately I missed checking in the flag in my last PR.

Normally we are consuming nunit-console via an internal choco feed, on which we push the official package. However, we currently have a working environment with a self-compiled version, so getting a new release out is not time-critical for us.

@ilCosmico
Copy link

I'm using official Nuget packages instead, so I wait for 3.17 release 😅

@OsirisTerje
Copy link
Member

Published latest from @mlessmann , binaryformatter fix, see https://github.com/nunit/nunit-console/releases/tag/V3.17.0-dev.3

@ilCosmico
Copy link

@OsirisTerje with -dev.3 I get the original error again :/

image

@mlessmann
Copy link
Author

Works on my end using the nuget package. @ilCosmico can you please try adding the following lines to the file agents\net8.0\nunit-agent.runtimeconfig.json next to the Microsoft.NETCore.App section:

{
  "name": "Microsoft.WindowsDesktop.App",
  "version": "8.0.0"
}

If it works with those changes then you should open a separate issue. Loading the WindowsDesktop framework probably cannot be enabled out-of-the-box, as it only works on Windows.

@ilCosmico
Copy link

@mlessmann I confirm that replacing Microsoft.NETCore.App with Microsoft.WindowsDesktop.App in nunit-agent.runtimeconfig.json file seems to do the trick.

image

@OsirisTerje can this be fixed when the new official NuGet package 3.17 will be release, please?

@OsirisTerje
Copy link
Member

@ilCosmico @mlessmann I am kind of blind here. Can you two work out what needs to be done here now, issues and PRs please. And I'll get it into the 3.17 when I am back, in a weeks time. During next week it should be possible to push out a new prod release.

@ilCosmico
Copy link

I understand that we need to edit the nunit-agent.runtimeconfig.json file for every .NET target when running the same UnitTesting library built against net472/net6/net7/net8. However, it's not feasible to change the nunit-agent.runtimeconfig.json file for every case.
It would be great if version 3.17 could support all the .NET targets.
@OsirisTerje, I'm not sure what details we should include in the new issue.
@mlessmann, Could you kindly open the new issue, please?

@mlessmann
Copy link
Author

@ilCosmico I've opened #1383, but I have no idea how to fix the problem.

@ilCosmico
Copy link

Published latest from @mlessmann , binaryformatter fix, see https://github.com/nunit/nunit-console/releases/tag/V3.17.0-dev.3

@OsirisTerje this release works fine except for issue #1383 kindly opened by @mlessmann.

I hope you have enough info to create a v3.17 that can be used for all targets (net472/net6/net7/net8) 😊

@OsirisTerje
Copy link
Member

OsirisTerje commented Jan 24, 2024

I have released version 3.17.0 now. The nuget packages are up, and the Choco package has also been uploaded, and just waiting for the Choco verification to pass.

The release contains more than what is in the release notes. The extra is from backports where the milestones are set to other releases. This is the result of an automated release manager that doesn't handle cross version backports well. Sorry for that.
Details can be seen in the branch history. I'll try to add some more info on that later.

@ilCosmico
Copy link

ilCosmico commented Jan 26, 2024

@OsirisTerje I tried it and now I get the below error, even replacing Microsoft.NETCore.App with Microsoft.WindowsDesktop.App in nunit-agent.runtimeconfig.json as suggest by @mlessmann

Any idea? Do I need to wait for the fix of issue #1383?

System.MissingMethodException : Method not found: 'System.Drawing.Graphics System.Windows.Forms.Control.CreateGraphics()'.
   at UnitTests.UnitTestBase`1.CreateNewEyeshotControl[T](rendererType renderer)
   at UnitTests.UnitTestBase`1.MyTestInitialize() in C:\source\Workspaces\Product\Beta\Tests\UnitTesting\UnitTest.cs:line 496
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

I'm not sure it's related, but in the past few days, I updated some NuGet packages in the project, in particular Microsoft.Windows.Compatibility from 8.0.0 to 8.0.1... could it be a clue?

@OsirisTerje
Copy link
Member

No idea, without a repro it is near impossible to say anything. You can create a small repro and add that to the #1383 issue, where it seems to belong.

@ilCosmico
Copy link

@OsirisTerje Finally I found out the guilty: the adding of Microsoft.Windows.Compatibility package!

image

Here is a very simple project to reproduce the issue :)
NUnitTestProject1.zip

I hope this helps to fix the issue 🤞

@OsirisTerje
Copy link
Member

OsirisTerje commented Feb 13, 2024

Hi! I do get a similar issue, but not quite the same. This means the project doesn't properly contain all its dependencies, and is dependent upon the current PC installation.
But, now that you do have a repo, you can just add the console project and engine project to your solution and see if you can debug down into them to see better what is blocking this:

I got:
image

PS: THis problem is not the same issue as this one, and this one is closed. Suggest you raise another issue and copy the relevant information you have provided into that one. I doubt this works on earlier versions too, but nice if you check that out.

@ilCosmico
Copy link

ilCosmico commented Feb 13, 2024

@OsirisTerje the error you get is because of the issue explained in this same issue by @mlessmann, that's why I think that this issue should be reopened.

BTW it's worth noting that no issues occur with .NET6 and .NET7.

@OsirisTerje
Copy link
Member

OsirisTerje commented Feb 13, 2024

Ok, good, but since you know where the error occurs, then it would be awesome with a PR to fix it.

Also see this comment #1374 (comment) about raising a new issue.

The issue is not the important thing though, but since the fix in the PR related to this issue works for @mlessmann and others who do the same, the issue you point at seems related, but not the same.
The important thing however, is if you can produce a PR - if possible, or a PR to the docs repo if this can be fixed by following what @mlessmann wrote above and then just documenting that.

@ilCosmico
Copy link

ilCosmico commented Feb 13, 2024

Ok, I will add it to issue #1383, but no idea how to fix it :)

@manfred-brands
Copy link
Member

A MissingMethodException means that the binary was compiled against a version of a dependency that had the method, but that the version found and used at runtime doesn't have it.

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

5 participants
@OsirisTerje @manfred-brands @ilCosmico @mlessmann and others