Skip to content

Conversation

@ashishdawale20
Copy link
Contributor

@ashishdawale20 ashishdawale20 commented Oct 17, 2023

Changes required to support .NET 8.0 #4503
Work in progress, .NET8.0 RC should be changed when a stable version is released and few tests are are failing will take a look at that as well.
image

@manfred-brands
Copy link
Member

BinarySerialization is no longer supported in .NET 8. due to security concerns

Do we make it conditional, or shall we remove it from NUnit4 completely?

@manfred-brands
Copy link
Member

@ashishdawale20 you need to update .git/workflows/continuous_integration.yml to install .net 8.0:
You can combine all into one step:

    - name: Setup .NET
      uses: actions/setup-dotnet@v3
      with:
        dotnet-version: |
          6.0.x
          7.0.x
          8.0.100-rc.2.23502.2

@stevenaw
Copy link
Member

stevenaw commented Oct 17, 2023

Do we make it conditional, or shall we remove it from NUnit4 completely?

Last I checked (also on mobile), this is only used for one constraint so it might mean dropping support for that constraint on non-netfx builds. I'm ok with this myself.

EDIT: Though perhaps within this PR we just wrap the test within an #if !NET8_ORGREATER block?

@OsirisTerje OsirisTerje changed the title .NET 8 support WIP .NET 8 support Oct 17, 2023
@ashishdawale20
Copy link
Contributor Author

ashishdawale20 commented Oct 18, 2023

@ashishdawale20 you need to update .git/workflows/continuous_integration.yml to install .net 8.0: You can combine all into one step:

    - name: Setup .NET
      uses: actions/setup-dotnet@v3
      with:
        dotnet-version: |
          6.0.x
          7.0.x
          8.0.100-rc.2.23502.2

Thank you @manfred-brands - this is done, i will check why github action is failing to setup .net

@manfred-brands
Copy link
Member

dotnet cake doesn't want to run, not even locally.

@manfred-brands
Copy link
Member

@ashishdawale20 Have you compiled the source locally?

There are two compile errors due to obsolete code.

@ashishdawale20
Copy link
Contributor Author

ashishdawale20 commented Oct 18, 2023

@ashishdawale20 Have you compiled the source locally?

There are two compile errors due to obsolete code.

Yes @manfred-brands , it works for me locally

@manfred-brands
Copy link
Member

I think you have the same 'warnings':

image

When building inside Visual Studio they are allowed, when building with dotnet build they become errors.

@OsirisTerje
Copy link
Member

@stevenaw Which constraint is it impacting?

@manfred-brands
Copy link
Member

@stevenaw Which constraint is it impacting?

BinarySerializableConstraint

image

It was already obsolete in .NET6.0, but it got physically removed in .NET8.0, breaking the tests.

image

@OsirisTerje
Copy link
Member

Do we make it conditional, or shall we remove it from NUnit4 completely?

Imho, remove it completely. The 4.0 is the good one for breaking changes.
We also must update the docs on it.

@ashishdawale20
Copy link
Contributor Author

ashishdawale20 commented Oct 19, 2023

BinarySerializableConstraint

@OsirisTerje - i have created this PR for doc update : nunit/docs#814

@manfred-brands
Copy link
Member

The build seems to suffer from the .NET 8.0 RC2 dotnet tool bug](dotnet/sdk#35989)

ashishdawale20 and others added 4 commits November 15, 2023 03:40
'dotnet cake' becomes 'dotnet tool run dotnet-cake'

Drop net7.0 runtime testing
We only create a .NET6.0 binary.
@manfred-brands manfred-brands changed the title WIP .NET 8 support .NET 8 support Nov 14, 2023
@manfred-brands
Copy link
Member

@OsirisTerje Updated to final release of .NET8.0 SDK

Copy link
Member

@OsirisTerje OsirisTerje left a comment

Choose a reason for hiding this comment

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

Directory.Build.Props: The runtimeframeworks is defined there, and missing net8, but includes net7. Since testing for net7 is replaced with net8 we could remove net7.

@manfred-brands
Copy link
Member

Directory.Build.Props: The runtimeframeworks is defined there, and missing net8, but includes net7. Since testing for net7 is replaced with net8 we could remove net7.

It contains: <NUnitRuntimeFrameworks>net462;net6.0;net8.0</NUnitRuntimeFrameworks>

@OsirisTerje
Copy link
Member

@manfred-brands Nice. But, should we then remove .net 7 from wherever that is used?

Copy link
Member

@OsirisTerje OsirisTerje left a comment

Choose a reason for hiding this comment

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

Awesome!

@manfred-brands
Copy link
Member

@manfred-brands Nice. But, should we then remove .net 7 from wherever that is used?

I just removed it from the places you found.
There is some conditional code on NET7_0_OR_GREATER which I would like to stay as that is the SDK where that feature was introduced.

@OsirisTerje
Copy link
Member

We'll then push out a beta.2 , but I was planning for doing a non-beta release of 4 by the end of the week.

@manfred-brands manfred-brands merged commit 097a1be into nunit:master Nov 14, 2023
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.

4 participants