Skip to content

NET9 Support#4851

Merged
stevenaw merged 13 commits into
nunit:mainfrom
Lachstec:main
Nov 15, 2024
Merged

NET9 Support#4851
stevenaw merged 13 commits into
nunit:mainfrom
Lachstec:main

Conversation

@Lachstec
Copy link
Copy Markdown
Contributor

@Lachstec Lachstec commented Oct 1, 2024

Changes that are needed in order to support .NET 9.0
Fixes #4846

Work in progress PR in order to prepare for the release of version 9.0.

@Lachstec
Copy link
Copy Markdown
Contributor Author

Lachstec commented Oct 1, 2024

@dotnet-policy-service agree

@Lachstec Lachstec closed this Oct 1, 2024
@Lachstec Lachstec reopened this Oct 1, 2024
Copy link
Copy Markdown
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Lachstec for your contribution

We need to keep net8.0

Comment thread src/NUnitFramework/Directory.Build.props Outdated
Comment thread nuget/nunitlite/nunitlite.nuspec
Comment thread src/NUnitFramework/Directory.Build.props Outdated
@manfred-brands
Copy link
Copy Markdown
Member

It looks like the linux build failed due to: terminate called after throwing an instance of 'PAL_SEHException'
This is normally a "illegal pointed" which in managed code we should not have.
We will revisit once .NET 9.0 is released.

@Lachstec
Copy link
Copy Markdown
Contributor Author

Lachstec commented Oct 3, 2024

I think so to. I spent some time trying to figure out what the issue could be, but it is likely related to some internal things of .NET. Build is failing with the same exception on MacOS.

@stevenaw
Copy link
Copy Markdown
Member

I've tried rerunning the builds as RC2 was released this past Tuesday. The present state is all three builds (Windows, Linux, Mac) fail with the following error:

Error: /home/runner/work/nunit/nunit/src/NUnitFramework/windows-tests/obj/Release/net8.0-windows10.0.19041.0/WinRT.SourceGenerator/Generator.WinRTAotSourceGenerator/WinRTGlobalVtableLookup.g.cs(5,25): error CS0116: A namespace cannot directly contain members such as fields, methods or statements [/home/runner/work/nunit/nunit/src/NUnitFramework/windows-tests/windows-tests.csproj::TargetFramework=net8.0-windows10.0.19041.0]

We'll have to look into this

@Lachstec
Copy link
Copy Markdown
Contributor Author

I will look into it This Weekend. Thank you for trying with the new RC.

@stevenaw
Copy link
Copy Markdown
Member

stevenaw commented Oct 12, 2024

This may actually be unrelated to .NET 9, and instead related related to a change in the latest version of the SDK.

There's a workaround listed here: dotnet/sdk#44026

@Lachstec This fix may actually be better as a separate PR so that main can continue to build. We'd welcome a second PR here if you're interested, or I could put one up myself sometime this weekend if you can't.

@Lachstec
Copy link
Copy Markdown
Contributor Author

@stevenaw Thought so, as I already stumbled on this Issue while trying to fix the build. I will open a PR with the fix mentioned there in a minute.

@Lachstec Lachstec closed this Oct 12, 2024
@Lachstec Lachstec reopened this Oct 12, 2024
@Lachstec
Copy link
Copy Markdown
Contributor Author

Tests are passing, so we should be good to go when version 9 drops.

Copy link
Copy Markdown
Member

@stevenaw stevenaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, thanks for keeping an eye on this one @Lachstec
This looks good to me as well. Thanks for the contribution!

@stevenaw
Copy link
Copy Markdown
Member

.NET 9 has launched today 🎉

@Lachstec The PR is listed as a Draft. Is it ready for review on your end?
@manfred-brands you'd previously requested some changes here. Does it look alright to you now?

Copy link
Copy Markdown
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two changes requested

Comment thread global.json Outdated
Comment thread src/NUnitFramework/testdata/TestFixtureSourceData.cs Outdated
@Lachstec Lachstec marked this pull request as ready for review November 13, 2024 10:15
@Lachstec
Copy link
Copy Markdown
Contributor Author

CI fails because of formatting issues. Should I address them in a separate pr?

@manfred-brands
Copy link
Copy Markdown
Member

CI fails because of formatting issues. Should I address them in a separate pr?

Ok, if the changes to the StaticProperty is because of the new net9.0 target, then leave it in (revert the revert)

Copy link
Copy Markdown
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lachstec Thanks for your patience.

@Lachstec
Copy link
Copy Markdown
Contributor Author

No problem, hope that it works out now ;)

@stevenaw stevenaw merged commit 596f761 into nunit:main Nov 15, 2024
@stevenaw
Copy link
Copy Markdown
Member

LGTM. Thanks for your contribution @Lachstec !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add NET9 target for the framework tests

4 participants