Skip to content

Replace net45 support with net462 #4050

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

Merged
merged 7 commits into from
May 3, 2022
Merged

Replace net45 support with net462 #4050

merged 7 commits into from
May 3, 2022

Conversation

stevenaw
Copy link
Member

@stevenaw stevenaw commented Mar 2, 2022

Fixes #4036
Fixes #3989

Updates the framework to target net462 where it had been targeting net45 prior. Updates some of the build documentation too.

@stevenaw stevenaw changed the title 4036 drop net45 Replace net45 support with net462 Mar 2, 2022
@@ -164,13 +164,13 @@ Task("CheckForError")
.Description("Checks for errors running the test suites")
.Does(() => CheckForError(ref ErrorDetail));

Task("Test45")
.Description("Tests the .NET 4.5 version of the framework")
Task("TestNetFramework")
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to update this build target name so that it would continue to make sense. I opted for this to make it version-agnostic (and complimentary to the pre-existing "TestNetStandard20"). I'm not totally happy with it and am hoping for feedback or suggestions here

Copy link
Member

Choose a reason for hiding this comment

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

I do like it as we want to test the .net framework version. This way you don't have to update if once we move the minimum target to 4.8

@@ -317,6 +317,7 @@ public override string ToString()
new FrameworkData(RuntimeType.NetFramework, new Version(3,5), new Version(2,0,50727), "net-3.5", "Net 3.5"),
new FrameworkData(RuntimeType.NetFramework, new Version(4,0), new Version(4,0,30319), "net-4.0", "Net 4.0"),
new FrameworkData(RuntimeType.NetFramework, new Version(4,5), new Version(4,0,30319), "net-4.5", "Net 4.5"),
new FrameworkData(RuntimeType.NetFramework, new Version(4,6), new Version(4,0,30319), "net-4.6", "Net 4.6"),
Copy link
Member Author

Choose a reason for hiding this comment

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

The current code doesn't support reading "build" numbers from the provided Version so I left it like this for now but I'd appreciate some feedback on this area please.

@stevenaw
Copy link
Member Author

stevenaw commented Mar 2, 2022

@nunit/framework-team I've taken a first-run at making this update. CI is currently a bit of an issue, but I'd appreciate some early feedback on some of the issues noted above please.

@stevenaw stevenaw marked this pull request as ready for review March 3, 2022 15:13

![image](https://user-images.githubusercontent.com/52075808/121511286-61775580-c9e0-11eb-8e1e-ff44d0d8873d.png)

Because NUnit solution targets multiple frameworks, JetBrains Rider knows this and offers the option to run the tests against a specific framework and/or all target frameworks.

Unfortunately, there is currently a known issue ([#3008](https://github.com/nunit/nunit/issues/3008)) preventing the tests from being run in Visual Studio and JetBrains Rider IDEs. Equally there is also a known issue ([#3867](https://github.com/nunit/nunit/issues/3867)) preventing the tests from being run from the command line using `dotnet test`
Copy link
Member Author

@stevenaw stevenaw Mar 8, 2022

Choose a reason for hiding this comment

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

I've moved this into a "Known Issues and Workarounds" section below to give it a "problem /solution" structure to hopefully clarify it a bit for readers

@stevenaw
Copy link
Member Author

Latest merge conflicts have been resolved. This should now be ready for review.

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

Nice work.

@stevenaw
Copy link
Member Author

Thanks @manfred-brands !

@@ -56,17 +55,15 @@ The tests that should be run in the solution are grouped by project name:

Other test projects contain tests designed to fail purposely for integration tests.

Normally you should be able to run the unit tests directly from within your development IDE of choice (as you would when using NUnit in any other development project). For example, this is what it looks like in JetBrains Rider (2021.1.2) when right clicking on the `AssertEqualsTests` TextFixture:
You should then be able to run the unit tests directly from within your development IDE of choice against one or all target frameworks (as you would when using NUnit in any other development project). For example, this is what it looks like in JetBrains Rider (2021.1.2) when right clicking on the `AssertEqualsTests` TextFixture:
Copy link
Member

Choose a reason for hiding this comment

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

I initially, thought that it was odd to write "You should then be able to run the unit tests directly from within...." and then later have "Known Issues and Workarounds", but looking at the resulting page I think this is fine

Copy link
Member

@mikkelbu mikkelbu left a comment

Choose a reason for hiding this comment

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

LGTM

@stevenaw
Copy link
Member Author

stevenaw commented May 3, 2022

Thanks @mikkelbu !
I had tried to retain the original wording as much as possible on the documentation updates, but I'd also be happy to make any further changes. Since we're a few days past net45 EOL, I'd like to go ahead and merge this today, but I can happily do a follow-up PR to adjust any wording in the README.

@stevenaw stevenaw merged commit ccbbd11 into master May 3, 2022
@stevenaw stevenaw deleted the 4036-drop-net45 branch May 3, 2022 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop net45 build target in nunit4 Revisit build documentation relating to IDEs
3 participants