Bogus check for Windows 11

nunit/src/NUnitFramework/framework/Internal/OSPlatform.cs

Line 596 in 2c9490b

    
           get { return Platform == PlatformID.Win32NT && Version.Major == 11 && Product == ProductType.WorkStation; }

This check was added recently but it is not correct, Windows 11 major version is still 10, need to also check the build number

see https://www.codeproject.com/Articles/5336372/Windows-11-Version-Detection

@SimonCropp

@SimonCropp

@pepone

@pepone give you have the context here, can you submit a PR that fixes it

I'm going to label this as confirm to see if anyone on the @nunit/framework-team can repro.
The check return Platform == PlatformID.Win32NT && Version.Major == 11 && Product == ProductType.WorkStation works as expected for me on Windows 11 Home 21H2.

On Windows 11 21H2 I see

[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion]
"SystemRoot"="C:\\Windows"
"BaseBuildRevisionNumber"=dword:00000001
"BuildBranch"="co_release"
"BuildGUID"="ffffffff-ffff-ffff-ffff-ffffffffffff"
"BuildLab"="22000.co_release.210604-1628"
"BuildLabEx"="22000.1.amd64fre.co_release.210604-1628"
"CompositionEditionID"="Enterprise"
"CurrentBuild"="22000"
"CurrentBuildNumber"="22000"
"CurrentMajorVersionNumber"=dword:0000000a
"CurrentMinorVersionNumber"=dword:00000000

Edition	Windows 11 Pro
Version	21H2
Installed on	‎12/‎9/‎2021
OS build	22000.778
Experience	Windows Feature Experience Pack 1000.22000.778.0

Looking at NUnit code it just read CurrentMajorVersion,

nunit/src/NUnitFramework/framework/Internal/OSPlatform.cs

Lines 112 to 113 in 2c9490b

    
           var major = key.GetValue("CurrentMajorVersionNumber") as int?; 
        
           var minor = key.GetValue("CurrentMinorVersionNumber") as int?;

@pepone

@pepone seems to be correct, see https://learn.microsoft.com/en-us/windows/win32/sysinfo/operating-system-version

@stevenaw I would assume the reason it works for you, is that it reports as Windows 10, and there are no real changes wrt NUnit.

@SimonCropp Can you correct this?

TL;DR;

The only place that information is used is in the IsPlatformSupported method:

and the code is just doing a check to be returned:

And the IsPlatformSupported method seems to only be used from tests. And the strings WIN11 and Windows11 also only appear in tests.

This test also probably has to go:

@nunit/framework-team : Why do we have this code at all? I've been trying to see where it actually test anything of the framework code . That doesn't mean it doesn't, only I haven't found it so far.

It seemed to internally detect it as Windows 11 for me on Windows 11 when stepping through the code, though I wasn't sure if it was because I'm on Home Edition. I think it could be helpful to confirm if this is a Pro edition issue or not.

Oh. I can't recall what runtime I tested on, but that could also be an issue here. Looks like the reliability of Environment.OSVersion improved on .NET5+

https://learn.microsoft.com/en-us/dotnet/core/compatibility/core-libraries/5.0/environment-osversion-returns-correct-version

I may have tested on NET6. I'm not at a computer to verify, but we should confirm on an older runtime like net462

I'm confused now. I tried this again on the same machine as before while targeting .NET7 on main and I can now repro. I am still on Windows 11 Home.

[Test, Explicit]
        public void DetermineOS()
        {
            var os = OSPlatform.CurrentPlatform;
            Assert.Pass();
        }

@stevenaw

@stevenaw What do you mean by "repro" ? Do you get 10?

I have tested this on a WIn 10 machine and a WIn 11 Pro, and I do get the following:

Win 10:

Win 11 Pro:

So, if you get the same with WIn 11 Home, then we don't need the Win11 definitions. I see it is used in InsertEnvironmentElement, and then in the WriteXmlResultOutput, but it seems only informative. And is just plucked out the mscorelib. The IsSupported seems to only be used by tests. SO imho, I think we can delete some code here.

@pepone

We have to change the IsWindows11 method to check for major version == 10 and minor=22000, that's it, as @pepone says. I don't think there is anything else to it.
@stevenaw @jnm2

As even MS sees Windows11 as Windows10 do we need a Windows11 check?

Or do we need a minor check, eg. Windows10.19000 and Windows10.22000?

If we think a Windows11 check should stay, then we should fix the condition and update the nunit tests.
Should minor be >= 22000 so later updates of Win11 are found?

@pepone

Yeah, that was my initial reaction too, just get rid of the code there, but @pepone also raised the original issue #4212 for this, and it seems someone is actually using that functionality. The method is public so it can be used externally, but should probably have been internal. Is has also been fixed in the documentation, so then I changed a bit to think it could stay. I actually don't see the purpose for these, some checks for parallelization is using some of this information, might be something more, but I am not quite sure. Agree that minor can be >= even if we're guessing a bit there on what happens in the future.

I don't think we need to fix any tests, but perhaps you see some I don't ? Found it ;-)

Fixes #4237

Merge pull request #4374 from nunit/Issue4237

@OsirisTerje

Lots happened here in the last 24 hrs 😅
Thanks for fixing this @OsirisTerje . I'd forgotten to explicitly call out the prop I was inspecting in the debugger, but you're right, the IsWindows11 check had been showing false for me. I'm unsure why it had been "working" earlier for me, but your PR seems to fix the underlying issue

Navigation Menu

Repository navigation

Bogus check for Windows 11 #4237

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	var major = key.GetValue("CurrentMajorVersionNumber") as int?;
	var minor = key.GetValue("CurrentMinorVersionNumber") as int?;

Bogus check for Windows 11 #4237

Description

Activity

jnm2 commented on Oct 13, 2022

SimonCropp commented on Oct 14, 2022

stevenaw commented on Oct 30, 2022

pepone commented on Oct 31, 2022

OsirisTerje commented on Apr 30, 2023

stevenaw commented on Apr 30, 2023

stevenaw commented on Apr 30, 2023

stevenaw commented on May 1, 2023

OsirisTerje commented on May 1, 2023

OsirisTerje commented on May 1, 2023

manfred-brands commented on May 1, 2023

OsirisTerje commented on May 1, 2023

stevenaw commented on May 1, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions