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

Drop agents for .NET 7.0 and .NET Core 3.1 #1603

Merged
merged 1 commit into from
Jan 18, 2025
Merged

Drop agents for .NET 7.0 and .NET Core 3.1 #1603

merged 1 commit into from
Jan 18, 2025

Conversation

CharliePoole
Copy link
Member

@CharliePoole CharliePoole commented Jan 18, 2025

Fixes #1601

@nunit/engine-team I have enabled automatic assignment of reviews for this team. If I have done it right, only those who have expressed a willingness to review will be included. Let's see what happens when I push the button!

UPDATE: Github picked @stevenaw ! So far so good. :-)

@CharliePoole CharliePoole requested review from a team and stevenaw and removed request for a team January 18, 2025 18:00
Copy link
Member

@veleek veleek left a comment

Choose a reason for hiding this comment

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

I know I wasn't assigned to this, but I had a few minutes. :D

- name: 🛠️ Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: |
3.1.x
6.0.x
7.0.x
Copy link
Member

Choose a reason for hiding this comment

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

Why are we adding the 7.0 runtime to remove support for it?

Copy link
Member

Choose a reason for hiding this comment

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

A bit of a similar question from me @CharliePoole . I assume these are needed for tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

An earlier build failed because I removed it by mistake, so I added it back. Yes, we need both 3.1 and 7.0 for the test data assemblies, like mock-assembly.dll.

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

Thanks @CharliePoole . LGTM, though I left a small question just for my own understanding.

- name: 🛠️ Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: |
3.1.x
6.0.x
7.0.x
Copy link
Member

Choose a reason for hiding this comment

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

A bit of a similar question from me @CharliePoole . I assume these are needed for tests?

@OsirisTerje
Copy link
Member

The adapter runs under .net core 3.1. Not sure yet when we can step up that one.

@CharliePoole
Copy link
Member Author

@OsirisTerje The adapter, however doesn't use agents. Even so, I'll take a look at the latest code to be sure before I merge this.

@CharliePoole
Copy link
Member Author

@OsirisTerje I reviewed the code for the adapter and I see it references the engine package without specifying a version. The NUnit.Engine package targeted .NET 4.6.2 and .NET Standard 2.0 through version 3.18.2. In 3.18.3, .NET 6.0 was added experimentally. In 3.19.0 both .NET Standard 2.0 and .NET 6.0 were dropped in favor of .NET 8.0, so we now only have builds for .NET 4.6.3 and .NET 8.0.

So... if the adapter has continued to build through all those changes, I would think we're OK. In this PR I have not changed the engine itself although the agents present in the package have changed.

OTOH, I'm surprised if you are able to call .NET 8.0 dependencies from .NET Core 3.1. In retrospect, I guess I ought to know more about what you need from the engine.

I'll go ahead and merge this because it seems that anything broken has been broken by the 3.19 release as well. Before we get to a release point, I think one of us should experiment by forcing the adapter to use the latest engine alpha build, so we can be know whether some further fix is needed.

@CharliePoole CharliePoole merged commit 04be095 into main Jan 18, 2025
4 checks passed
@CharliePoole CharliePoole deleted the issue-1601 branch January 23, 2025 05:09
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.

Drop .NET Core 3.1 and .NET 7.0 agents in preparation for 3.20 release
4 participants