New issue
Advanced search Search tips

Issue 596640 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 596344



Sign in to add a comment

telemetry_unittests thinks windows 10 is windows 8

Project Member Reported by wfh@chromium.org, Mar 21 2016

Issue description

https://chromium-swarm-dev.appspot.com/user/task/2daf6577caef1010

telemetry.internal.browser.browser_unittest.ReferenceBrowserTest.testBasicBrowserActions
telemetry.web_perf.timeline_based_page_test_unittest.TimelineBasedPageTestTest.testTBM2ForSmoke

Skipping testCryptohome (<bound method CrOSCryptohomeTest.testCryptohome of <telemetry.internal.backends.chrome.cros_unittest.CrOSCryptohomeTest testMethod=testCryptohome>>) because it is only enabled for chromeos. You are running ['release_x64', 'win', 'win8', 'has tabs'].

It seems telemetry unittests thinks we are running win8?
 

Comment 1 by wfh@chromium.org, Mar 21 2016

Blocking: 596344
Cc: mar...@chromium.org
Running python on the bot:

C:\Users\chrome-bot>python
Python 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform
>>> platform.uname()
('Windows', 'bot', '8', '6.2.9200', 'AMD64', 'Intel64 Family 6 Model 62 Stepping 4, GenuineIntel')
>>>

it seems to be running without a windows 10 manifest.

Comment 2 by wfh@chromium.org, Mar 21 2016

seems this is fixed in latest python http://bugs.python.org/issue23018 perhaps just need to update to 2.7.11

Comment 4 by wfh@chromium.org, Mar 21 2016

I just pulled the latest python.manifest from the python source then patched my python.exe with command:

mt -manifest python.manifest -updateresource:"D:\src\depot_tools\python276_bin\python.exe;#1"

and now my python gives me:

C:\Users\wfh>python
Python 2.7.6 (default, Nov 10 2013, 19:24:18) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import platform
>>> platform.uname()
('Windows', 'DESKTOP-8M5VBA6', '', '10.0.10586', 'AMD64', 'Intel64 Family 6 Model 62 Stepping 4, GenuineIntel')
>>>

it seems the third field (release) is now empty rather than '8', but the version reported is correct.

is it easy for me to update the python.exe on all the bots with the new one, or perhaps having empty "release" value might cause other issues.
It's also quite easy to use a different Windows version check that isn't affected by the out-of-date manifest in the version of Python that we ship. I think that is probably a safer fix than patching python.exe.

Changing the manifest will change the shims that Windows applies to python.exe which can then lead to behavioral changes - I've dealt with crashes caused by this in the past (not with python.exe but with other binaries).

You can detect Windows versions up to 8.1 with this code (value will be 6.1, 6.2, or 6.3):

  key_name = r'Software\Microsoft\Windows NT\CurrentVersion'
  key = winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, key_name)
  value, keytype = winreg.QueryValueEx(key, "CurrentVersion")
  key.Close()

Detecting Windows 10 requires also reading CurrentMajorVersionNumber from the same key - CurrentVersion is still 6.3 for Windows 10.

Comment 6 by mar...@chromium.org, Mar 22 2016

Will, any idea why telemetry look at the current OS version?
Cc: achuith@chromium.org
+Achuith: why does cros test sniff the current OS version?

Comment 8 by wfh@chromium.org, Mar 22 2016

I think in theory it is used to filter tests, but in practice I don't think there are any filters that fine grained (I.e. have different settings between win8 and win10).

Comment 9 by aiolos@chromium.org, Mar 24 2016

The version is used in 2 cases that I know of:

1) To determine which binary dependency to use (not used on Windows afaik.)
2) To enable/disable tests that only work (or are currently broken) on specific versions of an os. Current example of a disable on win8: https://code.google.com/p/chromium/codesearch#chromium/src/tools/perf/benchmarks/blink_perf.py&l=228

Comment 10 by wfh@chromium.org, Mar 24 2016

is the code doing this here?

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapult/telemetry/telemetry/internal/platform/win_platform_backend.py&q=%22uname()%22%20file:.*%5C.py&sq=package:chromium&l=232&ct=rc&cd=32&dr=C

if so, I can try and land a patch to add support for win10 using one of the above methods, so we don't have to upgrade python everywhere.
SGTM. We can make such change to telemetry.
Owner: nedngu...@google.com
Status: Started (was: Untriaged)
Status: Fixed (was: Started)
Should be fixed with https://codereview.chromium.org/1830253003/ once telemetry roll. Thanks Bruce!

Sign in to add a comment