Closed
Bug 1264325
Opened 9 years ago
Closed 9 years ago
Talos on windows seems broken with python 2.7.11
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(firefox49 fixed)
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: avih, Unassigned)
Details
(Whiteboard: [talos_wishlist])
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
avih
:
review+
|
Details | Diff | Splinter Review |
Using Mozilla-build 2.2 (has python 2.7.11):
- cd mozilla-central/testing/talos
- python INSTALL.py
- source Scripts/activate
- talos --help
Gives the following error:
Traceback (most recent call last):
File "t:\dev\moz\src\m-c\testing\talos\Scripts\talos-script.py", line 9, in <module>
load_entry_point('talos', 'console_scripts', 'talos')()
File "d:\mozilla-build\python\lib\site-packages\pkg_resources\__init__.py", line 549, in load_entry_point
return get_distribution(dist).load_entry_point(group, name)
File "d:\mozilla-build\python\lib\site-packages\pkg_resources\__init__.py", line 2547, in load_entry_point
return ep.load()
File "d:\mozilla-build\python\lib\site-packages\pkg_resources\__init__.py", line 2207, in load
return self.resolve()
File "d:\mozilla-build\python\lib\site-packages\pkg_resources\__init__.py", line 2213, in resolve
module = __import__(self.module_name, fromlist=['__name__'], level=0)
File "t:\dev\moz\src\m-c\testing\talos\talos\run_tests.py", line 13, in <module>
import utils
File "t:\dev\moz\src\m-c\testing\talos\talos\utils.py", line 42, in <module>
PLATFORM_TYPE = _get_platform()
File "t:\dev\moz\src\m-c\testing\talos\talos\utils.py", line 37, in _get_platform
raise TalosError('unsupported windows version')
NameError: global name 'TalosError' is not defined
If I do the same from Mozilla-Build 2.1 (has 2.7.10), then it does run correctly, and 'talos --help' now also works from MB2.2 (since the python binaries at the virtualenv are from MB2.1).
Ryan created a test package based on MB2.2 - but with python 2.17.10, and it does seems to work correctly.
I also tried Ubuntu 16.04 beta2 (which has python "2.7.11+") and I _think_ there's no issue there.
So it seems the problem is between talos and python 2.7.11 (as provided by Mozilla-Build) on Windows.
Updated•9 years ago
|
Whiteboard: [talos_wishlist]
Comment 1•9 years ago
|
||
Joel, we'll be upgrading "soon" to py2.7.11 on test slaves (we're doing builders first)...
Any insight into if this is an actual problem or how difficult/long it would be to fix (with backports of the fix to all trees if necessary)
Blocks: 1266240
Flags: needinfo?(jmaher)
Reporter | ||
Comment 2•9 years ago
|
||
I think platform.system() returns 6.3.9600 (Windows 8.1 64), but talos/talos/utils.py only searches 5.1, 6.1, 6.2, but doesn't find any of those so throws an "unsupported windows version" exception.
Not sure if win8.1 should be treated differently than win8, but locally, the following patch seems to make it work (both talos --help and also actually running a test):
diff --git a/testing/talos/talos/utils.py b/testing/talos/talos/utils.py
index ac5cad7..af76c01 100755
--- a/testing/talos/talos/utils.py
+++ b/testing/talos/talos/utils.py
@@ -33,6 +33,8 @@ def _get_platform():
return 'w7_'
elif '6.2' in platform.version(): # w8
return 'w8_'
+ elif '6.3' in platform.version(): # w8
+ return 'w8_'
else:
raise TalosError('unsupported windows version')
elif platform.system() == "Darwin":
Reporter | ||
Comment 3•9 years ago
|
||
(also, no windows 10 support?)
Reporter | ||
Comment 4•9 years ago
|
||
(sorry for the spam).
With python 2.7.10, platform.version() returns:
- Windows 8.1 (64): 6.2.9200
- Windows 10 (32): 6.2.9200 (yes, same as win8.1)
So it's obvious why the current code works.
With Python 2.7.11, platform.version() returns:
- Windows 8.1 (64): 6.3.9600
- Windows 10 (32): 10.0.10586
And so it fails.
Comment 5•9 years ago
|
||
(In reply to Avi Halachmi (:avih) from comment #4)
> (sorry for the spam).
>
> With python 2.7.10, platform.version() returns:
> - Windows 8.1 (64): 6.2.9200
> - Windows 10 (32): 6.2.9200 (yes, same as win8.1)
>
> So it's obvious why the current code works.
>
>
> With Python 2.7.11, platform.version() returns:
> - Windows 8.1 (64): 6.3.9600
> - Windows 10 (32): 10.0.10586
>
> And so it fails.
Apparantly thats from the py2.7.11 rel notes:
- Issue #19143: platform module now reads Windows version from kernel32.dll to
avoid compatibility shims.
Which I found in some similar bugs when I was doing win10 work, like https://hg.mozilla.org/mozilla-central/rev/1989889145ce
Comment 6•9 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #5)
> Apparantly thats from the py2.7.11 rel notes:
>
> - Issue #19143: platform module now reads Windows version from kernel32.dll
> to
> avoid compatibility shims.
To be clear, upstream this is:
https://bugs.python.org/issue19143
with the 2.7 branch upstream cset of https://hg.python.org/cpython/rev/d8453733cc0c/ which used a different approach than my rev for w10 work.
Comment 7•9 years ago
|
||
thanks avih for chiming in here. To fix this we would need to patch talos here:
* https://dxr.mozilla.org/mozilla-central/source/testing/talos/talos/utils.py#27
** and verify other testing code: https://dxr.mozilla.org/mozilla-central/search?q=path%3Atesting+platform.system+Win&redirect=false&case=true
we only run talos on beta/aurora/trunk, so once this is landed successfully on trunk, then uplift to aurora/beta should be very simple.
Flags: needinfo?(jmaher)
Comment 8•9 years ago
|
||
Callek, I have a few other higher priority items this month to work on, I won't be much help here- feel free to submit a patch and do try pushes/landings. I can review the patch if needed.
Reporter | ||
Comment 9•9 years ago
|
||
Any plans here?
As long as it's not fixed, it's impossible to run talos locally on windows 8.1 or windows 10 with latest mozilla-build - That's almost for two months now.
Flags: needinfo?(jmaher)
Comment 10•9 years ago
|
||
we will use this small hack to fix this for now- the next patch that I review for :avih, will just include this- so we will duplicate this bug when appropriate.
Flags: needinfo?(jmaher)
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 12•9 years ago
|
||
This is unrelated to nor a duplicate of bug 1274992.
I'll post a hack patch for review which allows it to run locally and which generally is an incorrect solution. Joel does not know what the correct solution is and neither do I - so at least let people run it locally meanwhile.
Reporter | ||
Comment 13•9 years ago
|
||
Keep in mind that this is probably an incorrect solution, but Joel requested me to land it anyway.
Attachment #8755484 -
Flags: review?(jmaher)
Comment 14•9 years ago
|
||
Comment on attachment 8755484 [details] [diff] [review]
bug1264325.patch - don't break when running on windows 8.1/10
Review of attachment 8755484 [details] [diff] [review]:
-----------------------------------------------------------------
thanks, this is good to see!
Attachment #8755484 -
Flags: review?(jmaher) → review+
Reporter | ||
Comment 15•9 years ago
|
||
Carrying r+ just changing from git patch to mercurial patch.
Attachment #8755484 -
Attachment is obsolete: true
Attachment #8755545 -
Flags: review+
Reporter | ||
Updated•9 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 16•9 years ago
|
||
Carrying r+, forgot the commit message at the earlier patch.
Attachment #8755545 -
Attachment is obsolete: true
Attachment #8755548 -
Flags: review+
Comment 17•9 years ago
|
||
Keywords: checkin-needed
Comment 18•9 years ago
|
||
backed this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=ae4b9e58a98a seems this or the other talos change caused https://treeherder.mozilla.org/logviewer.html#?job_id=28541751&repo=mozilla-inbound
Flags: needinfo?(avihpit)
Reporter | ||
Comment 19•9 years ago
|
||
Meh, really? we can't land lines longer than 79 chars? Is this only with talos? only with python? or with any new code which lands at m-c?
I applaud the effort, though on this case it seems a bit excessive considering the current code base, but that's off topic.
How can I check this flake8 thingy next time locally - to see the same issues as if I tried to land it?
Flags: needinfo?(avihpit) → needinfo?(cbook)
Comment 20•9 years ago
|
||
run flake8 locally on files changed:
cd testing/talos/talos
pip install flake8
flake8 test.py
that should sort it out- try will catch this as well.
Flags: needinfo?(cbook)
Reporter | ||
Comment 21•9 years ago
|
||
Will it show warnings only on the lines which got modified? or on the entire file?
Comment 22•9 years ago
|
||
on all lines- but warnings should be non-existent on lines which you did not modify.
Reporter | ||
Comment 23•9 years ago
|
||
So there no way to check locally "is this patch going to trigger a flake8 error" without running it manually on all the files which the patch touches and then verify manually if the warnings relate to lines which the patch touches?
Comment 24•9 years ago
|
||
cd testing/talos
flake8 talos
Comment 25•9 years ago
|
||
According to https://groups.google.com/d/msg/mozilla.tools/LOibynYnkwU/DVCQJyYsCgAJ you can use "mach lint" and "mach lint <path(s)>" to run flake8.
Reporter | ||
Comment 26•9 years ago
|
||
Carrying r+, updated the comment to not exceed 79 chars, Joel said on IRC that the patch is OK.
try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6cc8c57dff9
Attachment #8755548 -
Attachment is obsolete: true
Attachment #8755959 -
Flags: review+
Comment 28•9 years ago
|
||
Keywords: checkin-needed
Comment 29•9 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•