Closed
Bug 735502
Opened 13 years ago
Closed 12 years ago
run_tests.py (or similar) should copy twinopen files to android, not remotePerfConfigurator.py
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 798532
People
(Reporter: k0scist, Unassigned)
References
Details
(Whiteboard: [good first bug][mentor=jhammel][lang=py])
Attachments
(2 files, 2 obsolete files)
2.73 KB,
patch
|
Details | Diff | Splinter Review | |
6.43 KB,
patch
|
Details | Diff | Splinter Review |
http://hg.mozilla.org/build/talos/file/52063311813e/talos/remotePerfConfigurator.py#l63
This instead should be done in run_tests.py or similar as it blocks
bug 733583
Reporter | ||
Updated•13 years ago
|
Whiteboard: [good first bug][mentor=jhammel][lang=py]
I wouldn't mind trying my hand at this bug.
From what I read this is whated:
Take the function at line 63 in 'remotePerfConfigurator.py' and move it to the file 'run_tests.py'.
Where I am confused:
Will the change cause a regression for any program that wants to call the function from 'remotePeafConfigurator.py' instead of 'run_tests.py'?
Do I need an android phone to verify/run tests on any of these changes?
Thanks
Comment 2•12 years ago
|
||
essentially that is what you want to do.
The work flow now is:
* call *perfConfigurator.py
* call run_tests.py
So moving it to run_tests.py will be just fine.
You don't need an android phone (although that is ideal), you can run a python agent that acts as the same protocol we have for the test sutagent on the android phones under test:
http://people.mozilla.org/~jmaher/remotetesting/
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to kanbie from comment #1)
> I wouldn't mind trying my hand at this bug.
>
> From what I read this is whated:
> Take the function at line 63 in 'remotePerfConfigurator.py' and move it
> to the file 'run_tests.py'.
>
> Where I am confused:
> Will the change cause a regression for any program that wants to call the
> function from 'remotePeafConfigurator.py' instead of 'run_tests.py'?
Nope; nothing should be calling this function at the moment so it should be safe. You should call the function if config['remote'] is set
> Do I need an android phone to verify/run tests on any of these changes?
As Joel said, that would be ideal. However, we can help test if you get up a patch.
Reporter | ||
Comment 5•12 years ago
|
||
(In reply to kanbie from comment #4)
> Created attachment 635451 [details] [diff] [review]
> Moves 'buildRemoteTwin' function from 'remotePerfConfigurator.py' to
> 'run_tests.py'
So this isn't quite there. While the function is moved over, it isn't called anywhere. It should be called with the same logic as is in remotePerfConfigurator.py : http://hg.mozilla.org/build/talos/file/7d28f2533eb5/talos/remotePerfConfigurator.py#l165
If 'winopen.xul' is in the test's URL and config['remote'] is True then we should call this function from the run_tests function: http://hg.mozilla.org/build/talos/file/7d28f2533eb5/talos/run_tests.py#l139
'self' won't be defined, since the function no longer lives on a class. So the references to self should be replaced and instead the need parameters should be passed to the function (we won't need remote since the run_tests function can call conditionally based on that, but we should find/pass in deviceroot.
Also, we should raise a talosError instead of a configurationError
I hope this helps! Let me know if this is clear
Points of confusion:
Is 'buildRemoteTwinopen' building a virtual device? what is the purpose of that device?
'self' was in the 'buildRemoteTwinopen' function, does it need to change even though it is defined as a input?
The comment about the deviceRoot confuses me, possible to elaborate?
Thanks for your input!
Attachment #635451 -
Attachment is obsolete: true
Attachment #635778 -
Flags: feedback?(jhammel)
Submittal for review.
Attachment #635778 -
Attachment is obsolete: true
Attachment #635778 -
Flags: feedback?(jhammel)
Reporter | ||
Comment 8•12 years ago
|
||
Comment on attachment 635788 [details] [diff] [review]
Moves 'buildRemoteTwin' function from 'remotePerfConfigurator.py' to 'run_tests.py', changes 'self' in 'run_tests.py' with 'browser_config' in appropiate spots. Comment on line 301
>diff -r 7d28f2533eb5 talos/remotePerfConfigurator.py
>--- a/talos/remotePerfConfigurator.py Thu Jun 21 12:19:24 2012 -0400
>+++ b/talos/remotePerfConfigurator.py Fri Jun 22 11:45:23 2012 -0500
>@@ -163,7 +163,6 @@
> if self.remote == False:
> return url
> if 'winopen.xul' in url:
>- self.buildRemoteTwinopen()
> url = 'file://' + self.deviceroot + '/talos/' + url
>
> # Take care of tpan/tzoom tests
>@@ -202,27 +201,6 @@
> self.config['deviceroot'] = self.deviceroot
> self.remote = True
>
>- def buildRemoteTwinopen(self):
>- """
>- twinopen needs to run locally as it is a .xul file.
>- copy bits to <deviceroot>/talos and fix line to reference that
>- """
>- # XXX this should live in run_test.py or similar
>-
>- if self.remote == False:
>- return
>-
>- files = ['page_load_test/quit.js',
>- 'scripts/MozillaFileLogger.js',
>- 'startup_test/twinopen/winopen.xul',
>- 'startup_test/twinopen/winopen.js',
>- 'startup_test/twinopen/child-window.html']
>-
>- talosRoot = self.deviceroot + '/talos/'
>- for file in files:
>- if self.testAgent.pushFile(file, talosRoot + file) == False:
>- raise pc.ConfigurationError("Unable to copy twinopen file "
>- + file + " to " + talosRoot + file)
>
> def main(args=sys.argv[1:]):
>
>diff -r 7d28f2533eb5 talos/run_tests.py
>--- a/talos/run_tests.py Thu Jun 21 12:19:24 2012 -0400
>+++ b/talos/run_tests.py Fri Jun 22 11:45:23 2012 -0500
>@@ -298,6 +298,10 @@
> else:
> print "WARNING: unable to start web server without custom port configured"
>
>+ # place files into the remote device (if it exists) for testing
>+ if browser_config['remote'] and 'winopen.xul' in url:
>+ buildRemoteTwinopen(browser_config)
>+
You probably want test['url'] which is around here: http://hg.mozilla.org/build/talos/file/3607fcd7ce14/talos/run_tests.py#l167
You'll want to pass deviceroot and the testAgent to the function, not browser_config
> # run the tests
> utils.startTimer()
> utils.stamped_msg(title, "Started")
>@@ -354,5 +358,27 @@
> # run tests
> run_tests(parser.config)
>
>+
>+def buildRemoteTwinopen(self):
>+ """
>+ twinopen needs to run locally as it is a .xul file.
>+ copy bits to <deviceroot>/talos and fix line to reference that
>+ """
>+
>+ if self.remote == False:
>+ return
You won't need this if we test for remote above
>+ files = ['page_load_test/quit.js',
>+ 'scripts/MozillaFileLogger.js',
>+ 'startup_test/twinopen/winopen.xul',
>+ 'startup_test/twinopen/winopen.js',
>+ 'startup_test/twinopen/child-window.html']
>+
>+ talosRoot = self.deviceroot + '/talos/'
No 'self.'
>+ for file in files:
>+ if self.testAgent.pushFile(file, talosRoot + file) == False:
>+ raise talosError("Unable to copy twinopen file "
>+ + file + " to " + talosRoot + file)
>+
> if __name__=='__main__':
> main()
Reporter | ||
Comment 9•12 years ago
|
||
So something like this should work. I haven't been able to test as I don't have a device with me today. Does this sorta make sense?
Reporter | ||
Comment 10•12 years ago
|
||
we now don't copy these files for remote at all, Bug 798532
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•