Closed
Bug 779983
Opened 12 years ago
Closed 9 years ago
mozharness device_talosrunner has some minor issues
Categories
(Release Engineering :: Applications: MozharnessCore, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: wlach, Assigned: wlach)
Details
(Whiteboard: [mozharness])
Attachments
(1 file)
2.55 KB,
patch
|
mozilla
:
review+
|
Details | Diff | Splinter Review |
(1) You need to specify a whole whack of configuration options (results server, etc.) even if you only want to run talos in local development mode for test purposes (as I do in bug 776728) (2) The option to turn on development mode is named "start_python_webserver", which is confusing (that's not all it does) (3) We use the configuration option "active_suites" to specify tests, but this requires you to specify suites twice with two different names. The name used by the underlying talos mozharness script is just "tests". Why not use that?
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #648483 -
Flags: review?(aki)
Updated•12 years ago
|
Whiteboard: [mozharness]
Comment 2•12 years ago
|
||
Comment on attachment 648483 [details] [diff] [review] Patch to hopefully address issues pointed out in bug In principle, I don't disagree with this. * Have you run this to verify it still works? Might be harmless, but I tend to try to test when making changes. Not sure if you've got this working locally yet? * unit.sh points out "scripts/device_talosrunner.py:142: undefined name 'additional'", probably from > + else: > + additional.options.extend(['--webServer', c['talos_webserver']]) * You're renaming a script config setting, but not updating the config files (I only found configs/users/aki/tablet1.py start_python_webserver, which isn't obviously tied to this script, but I tend to recursive grep when making a change.) r=me with the additional s,start_python_webserver,develop_mode, in configs/users/aki/tablet1.py and the additional_options fix above. A test run would be good but isn't strictly blocking here.
Attachment #648483 -
Flags: review?(aki) → review+
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Aki Sasaki [:aki] from comment #2) > Comment on attachment 648483 [details] [diff] [review] > Patch to hopefully address issues pointed out in bug > > In principle, I don't disagree with this. > > * Have you run this to verify it still works? > Might be harmless, but I tend to try to test when making changes. Not sure > if you've got this working locally yet? I have, yes. Of course it's somewhat hard to say whether it's work 100% perfectly, since it's Talos, but my phone did seem to be running everything ok.
Assignee | ||
Comment 4•12 years ago
|
||
(oh, and thanks for the other feedback -- all valid issues and I will address before checking in)
Comment 5•11 years ago
|
||
Comment on attachment 648483 [details] [diff] [review] [diff] [review] Patch to hopefully address issues pointed out in bug ---------AUTOMATIC COMMENT--------- -- filter pep8-callek-june-2013 -- ---------AUTOMATIC COMMENT--------- $pep8 <dir> --diff --max-line-length=159 --show-source < attachment.diff /tmp/tmp.6sQOVBKBmy/scripts/device_talosrunner.py:168:19: E124 closing bracket does not match visual indentation ] + additional_options ^
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Comment 6•11 years ago
|
||
We aren't using device_talosrunner.py (:kmoir wrote http://hg.mozilla.org/build/mozharness/file/f374f4bc03ee/scripts/android_panda_talos.py instead), so I removed it a while ago in a general cleanup sweep. -> WONTFIX/WFM ?
Updated•10 years ago
|
Component: Other → Mozharness
Assignee | ||
Comment 7•9 years ago
|
||
This almost certainly isn't valid anymore.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•