Fixes Issue #4705 - Wrong version numbers in dlls #4706

OsirisTerje · 2024-05-07T19:39:16Z

src/NUnitFramework/FrameworkVersion.cs

src/AssemblyInfo.g.cs

…ed csprojs and assemblyinfo.cs files accordingly.

…o fileversionissue

manfred-brands

@OsirisTerje Sorry for the delay, I was off sick and couldn't concentrate enough to do reviews.
I also like code generation, but I do think your second option is nicer.
I think we can remove a lot of conditionals in the other projects by utilizing the AssemblyConfiguration set in the Directroy.Build.props

build.cake

src/NUnitFramework/Directory.Build.props

src/NUnitFramework/framework/nunit.framework.csproj

src/NUnitFramework/nunit.framework.legacy/nunit.framework.legacy.csproj

src/NUnitFramework/nunitlite/nunitlite.csproj

Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

…cy.csproj Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

…o fileversionissue

…iguration .NET 7.0 is out of support as of May 14th 2024.

src/NUnitFramework/tests/CakeTests.cs

src/NUnitFramework/tests/Internal/Results/TestResultTests.cs

src/NUnitFramework/tests/WorkItemTests.cs

src/NUnitFramework/TestBuilder.cs

manfred-brands

You added various unnecessary null suppression operators (!).
All of them are unnecessary as NUnit.Analyzer does the suppressing for us as it knows those cannot be null.

…o fileversionissue

…ild.cake

manfred-brands

Still some issues.

CakeScripts.Tests/Usings.cs

src/NUnitFramework/tests/CakeTests.cs

src/NUnitFramework/tests/Internal/Results/TestResultTests.cs

src/NUnitFramework/tests/WorkItemTests.cs

src/NUnitFramework/TestBuilder.cs

CakeScripts.Tests/VersionParsersTests.cs

…rlier

manfred-brands

I raised one issue in the build.cake script.
It was mentioned originally in my first review but got lost in the further discussions.
I pushed up a commit for that.

One thing though is that if I build this locally it build version: 4.0.0-alpha.88
So maybe the code relies on annotated tags instead of normal ones.
git describe returns 4.0.0-alpha-244-ga604a39ca

I do see confusing tags in the repository some have a v prefix others don't

4.0.0-alpha
4.1.0
4.1.0-alpha.0
v4.0.0
v4.0.0-beta.1
v4.0.1

Note that the CI build doesn't fetch any tags and creates version 0.0.0.0:

I added a commit to fetch the tags to see what it does. You can revert this as it isn't needed for CI builds only for publications.
It seems to create 4.2.0 versions:

MinVer must pick up the 4.1.0 tag and create the next release.
There are various properties for MinVer we can use to tailor this.

CakeScripts.Tests/CakeScripts.Tests.csproj

CakeScripts/VersionParsers.cs

build.cake

…raised a PR there too, but the warning can be safely ignored for now

manfred-brands · 2024-06-02T04:33:31Z

@OsirisTerje for some reason dotnet test no longer finds any tests in master after this commit got merged.

If I go a commit before this dotnet test finds test, afterwards not anymore.
I looked at the changes and this doesn't change anything except versions.
Not sure how the adapter finds tests, but the adapter is the same in all cases, so it must be something in NUnit itself.
I tried dotnet test -- NUnit.DebugExecution=true but your blog seems to indicate it only works with a debug version of the adapter and my PC refused to execute any non-signed PS1 scripts.

@stevenaw Can you confirm this behaviour?

stevenaw · 2024-06-02T18:02:44Z

I think I can confirm. Based on the following to test:

dotnet clean
dotnet build
dotnet test

I tried checking out the previous merge commit (376b434) and could run all the tests that way. Presently, it seems like only the cake tests are running. This is a subset of the output I see presently:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
No test is available in .\nunit\src\NUnitFramework\tests\bin\Debug\net8.0\nunit.framework.tests.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.

Additionally, path to test adapters can be specified using /TestAdapterPath command. Example /TestAdapterPath:.

I converted the absolute path in the the message to relative for brevity

OsirisTerje · 2024-06-02T19:31:06Z

I do see the same using dotnet test, and build --target=test (cake tests) still works.

How can version number changes do this ???

In VS I only see the single test in CakeScripts.Tests

That project only has net 6.0

mikkelbu · 2024-06-05T12:27:53Z

How can version number changes do this ???

The only thing I can thing of - without debugging into the code and I don't know much about versioning in .NET - is that the IsSupportedTestFramework - https://github.com/nunit/nunit-console/blob/6ab7411d454ab2563426fdb60a9cdff7be655bb4/src/NUnitEngine/nunit.engine.core/Drivers/NUnit3DriverFactory.cs#L21-L24 - cannot determine that the major version is above or equal to 3.

Looking at the product versions it seems that we now add a lot of "prerelease" information that might confuse the validation (comparing NUnit.4.2.0-alpha.0.17.nupkg and NUnit.4.2.0-alpha.0.21.nupkg from myget)

OsirisTerje · 2024-06-05T14:34:29Z

Just checking: Product Version == Informational Version, and should into be used in the comparison.
FileVersion is the one being returned in the check in the engine, so it should work (I know, famous last words.....)

Am I right or wrong here?

manfred-brands · 2024-06-06T14:22:00Z

Ok, I figured it out. @mikkelbu pointed me in the right direction.
When building nunit from within visual studio no version number is created at all.

I added the following to Directory.Build.props:

    <Version Condition="'$(Version)'==''">4.0.0.0</Version>

Now when compiling with cake, I get version 4.2.0.0 and when building inside VS, it is version 4.0.0.0
I'll put a PR up shortly.

Fixes Issue #4705

c0e3970

OsirisTerje requested review from manfred-brands and stevenaw May 7, 2024 19:39

manfred-brands reviewed May 8, 2024

View reviewed changes

src/NUnitFramework/FrameworkVersion.cs Outdated Show resolved Hide resolved

Delete src/src.sln

55737bb

stevenaw reviewed May 9, 2024

View reviewed changes

src/AssemblyInfo.g.cs Outdated Show resolved Hide resolved

OsirisTerje added 2 commits May 10, 2024 19:18

Changed to using conditionals in the directory.build.props and adjust…

54a699d

…ed csprojs and assemblyinfo.cs files accordingly.

Merge branch 'fileversionissue' of https://github.com/nunit/nunit int…

67f2016

…o fileversionissue

manfred-brands requested changes May 13, 2024

View reviewed changes

Adding caketest

361e24d

OsirisTerje changed the title ~~Fixes Issue #4705~~ Fixes Issue #4705 - Wrong version numbers in dlls May 13, 2024

OsirisTerje and others added 11 commits May 13, 2024 16:25

Update src/NUnitFramework/framework/nunit.framework.csproj

9d66172

Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

Update src/NUnitFramework/nunit.framework.legacy/nunit.framework.lega…

a100204

…cy.csproj Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

Update src/NUnitFramework/nunitlite/nunitlite.csproj

8bcdfd9

Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>

updated files and directory.build.props

013a570

commented out Choose, didnt work, in directory.build.props

d9673d0

Merge branch 'fileversionissue' of https://github.com/nunit/nunit int…

f2066b4

…o fileversionissue

Updated some tests, fixing nullrefs

0840495

reverted formatting

d9758f2

fix compiler warnings/errors

6a357c4

updated cake

1b6232a

Raise Error when compiling for a TargetFramework with no AssemblyConf…

b644884

…iguration .NET 7.0 is out of support as of May 14th 2024.