Closed
Bug 780807
Opened 12 years ago
Closed 9 years ago
browser and other reporting statistics should not be literal strings in run_tests.py (and possibly elsewhere)
Categories
(Testing :: Talos, defect)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: k0scist, Assigned: bharathkallurs, Mentored)
Details
(Whiteboard: [good first bug][lang=py])
Attachments
(5 files)
2.86 KB,
patch
|
k0scist
:
review-
|
Details | Diff | Splinter Review |
1.81 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
433 bytes,
text/plain
|
Details | |
5.82 KB,
application/x-yaml
|
Details | |
1.69 KB,
patch
|
jmaher
:
review-
|
Details | Diff | Splinter Review |
In run_tests.py (and possibly elsewhere) we use a bunch of literal strings of 'NULL' (maybe others): http://hg.mozilla.org/build/talos/file/f55ac228921e/talos/run_tests.py#l93 http://hg.mozilla.org/build/talos/file/f55ac228921e/talos/run_tests.py#l190 These should be None (the singleton) if they should be defined at all (questionable). Output formats, such as graphserver, can format them to 'NULL' iff such is appropriate, but probably not run_tests.py's job.
Reporter | ||
Updated•12 years ago
|
Whiteboard: [good first bug][mentor=jhammel][lang=py]
Comment 1•12 years ago
|
||
I believe the NULL is needed for the graph server SQL. If we move it to output that is fine.
i'm looking to get involved with the mozilla community. anyone willing to help me get started on this bug, if not already assigned?
Attachment #654666 -
Flags: review?(jhammel)
Reporter | ||
Comment 4•12 years ago
|
||
Comment on attachment 654666 [details] [diff] [review] hope this is fine :) So this doesn't really suffice. While you've changed these in run_test.py, they are likely used/referenced in other files, such as ttest.py and output.py and perhaps more. I'm also not sure why you alter the test file at all, or why run_tests.pyc is in your diff + if browser_config.get('repository', None) == None: This should be if browser_config.get('repository') is None
Attachment #654666 -
Flags: review?(jhammel) → review-
(In reply to Jeff Hammel [:jhammel] from comment #4) > Comment on attachment 654666 [details] [diff] [review] > hope this is fine :) > > So this doesn't really suffice. While you've changed these in run_test.py, > they are likely used/referenced in other files, such as ttest.py and > output.py and perhaps more. > > I'm also not sure why you alter the test file at all, or why run_tests.pyc > is in your diff > > + if browser_config.get('repository', None) == None: > > This should be if browser_config.get('repository') is None i pushed the repo again and tried and its browser_config.get('repository', None) == None itself .. also only run_tests file and output have NULL in them and its required to be NULL in output .. hence i can only make changes in run_tests
Reporter | ||
Comment 6•11 years ago
|
||
This bug is still worth having and up to date. Any thoughts, jdev?
Flags: needinfo?(jdev005)
Hi, I would like to work on this bug. Can anybody guide me on how to get started with it? Thanks a lot.
Reporter | ||
Comment 8•11 years ago
|
||
Hi Gobelinus, Do you understand the bug? Do you understand where to get the talos code? Maybe telling what you need to know is the most expedient. Thanks for the help! Jeff
Hi Jeff, As per my understanding, we need to replace literal strings with python equivalents like 'NULL' with None, however output should remain same till graphserver uptakes such changes. Otherwise graphserver reports will break. I cloned talos code. I was just wondering, if there is any way in which I can generate and view reports without graphserver. I want to compare output before and after making these changes. Thanks Gobelinus
Reporter | ||
Comment 10•11 years ago
|
||
You should be able to use `--results_url file://$(pwd)/output.txt` to get local results, I think. If this does not suffice, you'll have to hack the code.
Assignee | ||
Comment 11•10 years ago
|
||
Hi Jeff, I am keen on working on this bug. i have the following questions to start with : 1. I need to replace literal strings like 'NULL' to None in all the places occuring in the run_tests.py. Could you please confirm if my understanding is right, else correct me? 2. Where can i clone talos code so that i can start hacking run_tests.py code? 3. Once that is done, where do i submit the diff patch? Thank you!
Comment 12•10 years ago
|
||
Bharath, I will help you out, here is how to get started with talos: https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code Once you can run a test (use 'tsvgx') as it is simple to run, I would make your change and ensure everything works. When you submit the patch, we use mercurial queues (https://developer.mozilla.org/en-US/docs/Mercurial_Queues) usually, but you can 'hg diff > patch" and attach it to this bug.
Flags: needinfo?(jdev005)
Updated•10 years ago
|
Whiteboard: [good first bug][mentor=jhammel][lang=py] → [good first bug][mentor=jmaher][lang=py]
Assignee | ||
Comment 13•10 years ago
|
||
Joel, Thanks for the quick reply! Will start having a look at it right away!
Assignee | ||
Comment 14•10 years ago
|
||
Hi Joel, I just followed the link that gave above to setup the local buildbot for talos. I am stuck at run tests phase of the article. i tried executing the command given there and i am facing an issue. Could you please help me resolve it? searching the forums for it was not really useful for me. AFAIK i am either giving a wrong format for the command or a flag incomplete. Please find the console logs as follows : Exception: Please specify a format (talos)root@ubuntu:/opt/foxproject/talos# talos -n -d --develop --print-tests --executablePath /opt/foxproject/firefox_dev/firefox --activeTests ts --results_url ts.txt --datazilla-url ts.json --output ts_des - outputName = ts_des Traceback (most recent call last): File "/opt/foxproject/talos/bin/talos", line 9, in <module> load_entry_point('talos==0.0', 'console_scripts', 'talos')() File "/opt/foxproject/talos/talos/run_tests.py", line 382, in main options, args = parser.parse_args(args) File "/opt/foxproject/talos/talos/PerfConfigurator.py", line 554, in parse_args options, args = Configuration.parse_args(self, *args, **kwargs) File "/opt/foxproject/talos/talos/configuration.py", line 482, in parse_args self.dump(options, missingvalues) File "/opt/foxproject/talos/talos/PerfConfigurator.py", line 610, in dump Configuration.dump(self, options, missingvalues) File "/opt/foxproject/talos/talos/configuration.py", line 499, in dump self.serialize(dump) File "/opt/foxproject/talos/talos/configuration.py", line 533, in serialize raise Exception('Please specify a format') Exception: Please specify a format
Comment 15•10 years ago
|
||
in the commandline you have above, there is "--output ts_des", if you make this "--output ts_des.yml" it will work. That sounds like another bug in talos.
Comment 16•10 years ago
|
||
I would like to work on this.Can someone assign it to me?
Comment 17•10 years ago
|
||
hi dweep- it looks like Bharath is working on this- would you like to take another bug: http://www.joshmatthews.net/bugsahoy/?automation=1&py=1 ?
Assignee | ||
Comment 18•10 years ago
|
||
Thanks for clarifying Joel! So would you recommend me to log a bug for the CLI as well? I will change the output arg and see how that will work for me.. Thanks!
Comment 19•10 years ago
|
||
yes, please file a bug about the cli issue! Thanks for finding it.
Assignee: nobody → bharathkallurs
Assignee | ||
Comment 20•10 years ago
|
||
Hi Joel, I just was trying the talos with the modified CLI. I am having an issue in starting the tests on firefox. As i am already running firefox for my personal use and i don't want to switch to aniother browser, the plugins running on my current session of firefox are blocking from starting the tests. I followed a few blogs on internet to start the firefox with a different user profile which still doesn't seem to help. I guess user profiles are just for accessing, firefox as multiple users with their own customizations for each profile. Could you please guide/ point me to a link which can help me setup firefox for my dev environment which does not effect my regular usage? more like a virtualenv setup? :-) Kindly have a look at the logs : (talos)root@ubuntu:/opt/foxproject/talos# talos -n -d --develop --executablePath "/usr/bin/firefox" --activeTests ts --results_url ts.txt --datazilla-url ts.json --output ts_desktop.yml - outputName = ts_desktop.yml DEBUG : using testdate: 1392049173 DEBUG : actual date: 1392049173 INFO : browserInfo: '/usr/bin/application.ini' does not exist qm-pxp01: Started Mon, 10 Feb 2014 21:49:33 Running test ts: Started Mon, 10 Feb 2014 21:49:33 DEBUG : operating with platform_type : linux_ DEBUG : firefox already running before testing started (unclean system) INFO : (process:3339): GLib-CRITICAL **: g_slice_set_config: assertion 'sys_page_size == 0' failed (firefox:3339): GLib-GIO-CRITICAL **: g_dbus_connection_register_object: assertion 'G_IS_DBUS_CONNECTION (connection)' failed . . . . (firefox:3339): GConf-WARNING **: Client failed to connect to the D-BUS daemon: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken. __start_reportNaN__end_report __startTimestamp1391709305279__endTimestamp __startBeforeLaunchTimestamp1391709304000__endBeforeLaunchTimestamp __startAfterTerminationTimestamp1391709306000__endAfterTerminationTimestamp Failed ts: Stopped Mon, 10 Feb 2014 21:49:33 Traceback (most recent call last): File "/opt/foxproject/talos/talos/run_tests.py", line 329, in run_tests talos_results.add(mytest.runTest(browser_config, test)) File "/opt/foxproject/talos/talos/ttest.py", line 293, in runTest raise talosError("Found processes still running: %s. Please close them before running talos." % running_processes_str) talosError: Found processes still running: [3614] firefox, [3679] plugin-container, [24360] plugin-container. Please close them before running talos.
Comment 21•10 years ago
|
||
In order to run talos, you need to terminate all other instances of firefox locally. It is an annoyance that we have to live with, this is in place for a reason as we need less stuff interfering with the system while running performance tests. When we launch firefox from talos, we create a temporary profile and use that specifically. This has a custom set of preferences and extensions.
Updated•10 years ago
|
Mentor: jmaher
Whiteboard: [good first bug][mentor=jmaher][lang=py] → [good first bug][lang=py]
Assignee | ||
Comment 22•9 years ago
|
||
Sorry for getting into hibernate mode on this bug for a long time. I am back at it again and will be working on this. I am currently just retesting my setup and will soon report what i face.
Comment 23•9 years ago
|
||
great, feel free to ask questions when you have them!
Assignee | ||
Comment 24•9 years ago
|
||
The diff contains the files which had NULL previously which are now changed to None (the singleton).
Assignee | ||
Comment 25•9 years ago
|
||
Hi Joel, Kindly review the diff that I posted and please do let me know if there is any change that i have to make. I could find the following files which had the NULL string instead of None singleton. (talos)bkallur@bharathkallur:/opt/talos/talos/talos$ grep -rai "Null" *.py cmanager_win32.py: # double null: we're done output.py: vals = [[x, 'NULL'] for x in values] output.py: if page == 'NULL': output.py: if results.revision and results.revision != 'NULL': results.py: result['page'] = 'NULL' Thanks!
Comment 26•9 years ago
|
||
Comment on attachment 8535998 [details] [diff] [review] NULL_to_None.diff Review of attachment 8535998 [details] [diff] [review]: ----------------------------------------------------------------- yay, this looks pretty good. Please outline what type of testing you have done and how you have verified this. Once we get some confidence that this is working, I will push it up to our try server and run the tests like we do in production :) ::: talos/output.py @@ +536,4 @@ > else: > params['x86'] = 'false' > > + if results.revision and results.revision is not None : #changing'NULL' to None in this case you just need: if results.revision:
Attachment #8535998 -
Flags: review+
Assignee | ||
Comment 27•9 years ago
|
||
ts.txt file which was obtained as result from the change made.
Assignee | ||
Comment 28•9 years ago
|
||
This is the .yml obtained after the change made in the code.
Assignee | ||
Comment 29•9 years ago
|
||
Hi Joel, Thank you for review comments.. :) I changed the line as you suggested. Seems more clean now. I ran the test using the standard command line given on the talos build/running instruction page. I have attached the results file obtained after the run. The command line i used to run the tests and verify the change is : talos -n -d --develop --executablePath /opt/foxproject/firefox/firefox --activeTests ts --results_url ts.txt --datazilla-url ts.json --output ts_desktop.yml --mozAfterPaint The last line of the result obtained on the console is which is the content of ts.json file is : INFO : TALOSDATA: [{"test_machine": {"platform": "x86_64", "osversion": "Ubuntu 14.04", "os": "linux", "name": "qm-pxp01"}, "talos_aux": {"shutdown": [275, 680, 959, 65, 500, 918, 60, 80, 314, 562, 744, 88, 274, 235, 361, 748, 104, 397, 829, 236]}, "testrun": {"date": 1418572435, "suite": "ts", "options": {"responsiveness": false, "tpmozafterpaint": true, "tpchrome": true, "tppagecycles": 1, "tpcycles": 10, "tprender": false, "shutdown": true, "extensions": [{"name": "pageloader@mozilla.org"}], "rss": false}}, "results": {"ts": [NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN]}, "test_build": {"version": "34.0.5", "revision": "8274648ad79f", "id": "20141126041045", "branch": "", "name": "Firefox"}}] TinderboxPrint: TalosResult: {} It seems to me that the tests are running fine and launching. Could you please have a look at the output files once and let me know if I am missing something to analyse? Also a point or 2 (or a wiki) which will help me analyse and run specific tests might be really helpful. This will help me understand sort of tests that i should run from talos for the change i make. Thanks again for all the help! :)
Comment 30•9 years ago
|
||
I have pushed this to try server, we will see how this looks. If all goes well, then we can land this and get it deployed. Since it looks like you have setup talos and were able to get it running locally with no problem, here is sort of a more difficult bug: https://bugzilla.mozilla.org/show_bug.cgi?id=776947 If that is too hard or seems a bit complicated, I can find one that isn't so involved. Thanks again for your work on this bug, we will get it deployed this week if all goes well.
Assignee | ||
Comment 31•9 years ago
|
||
Thanks Joel! Sure.. I shall take a look at the new bug and try comprehending. Thanks again..
Comment 32•9 years ago
|
||
I ran this on try server: https://treeherder.mozilla.org/#/jobs?repo=try&revision=882e7b276e0b the oranges are errors where we are uploading a page name None instead of NULL and graph server is complaining. I really don't like having a bug available and then getting a patch/fix to only find out there is not a big need for it. One way to hack around this would be to try making sure graphserveroutput in output.py keeps NULL when it is defining the dataset. We could try that out!
Assignee | ||
Comment 33•9 years ago
|
||
Joel, I just saw your comment. Please let me know if you are referring to the change ? --- a/talos/output.py Fri Dec 12 15:38:37 2014 -0500 +++ b/talos/output.py Sat Dec 13 16:13:16 2014 +0530 @@ -216,7 +216,8 @@ continue # counter values - vals = [[x, 'NULL'] for x in values] + #changing NULL to None + vals = [[x, None] for x in values] So restoring the NULL back in vals which is defined in GraphServerOutput class?
Comment 34•9 years ago
|
||
I believe that would do it.
Assignee | ||
Comment 35•9 years ago
|
||
Adding a version 2 of patch to improve the result in graphserveroutput. This patch restores the NULL as default values for "vals" variable, in GraphServerOutput class of output.py file. The patch is tested and has the following output INFO : TALOSDATA: [{"test_machine": {"platform": "x86_64", "osversion": "Ubuntu 14.04", "os": "linux", "name": "qm-pxp01"}, "talos_aux": {"shutdown": [116, 576, 71, 559, 46, 503, 922, 460, 853, 356, 954, 520, 141, 720, 314, 911, 460, 794, 387, 4]}, "testrun": {"date": 1418828448, "suite": "ts", "options": {"responsiveness": false, "tpmozafterpaint": true, "tpchrome": true, "tppagecycles": 1, "tpcycles": 10, "tprender": false, "shutdown": true, "extensions": [{"name": "pageloader@mozilla.org"}], "rss": false}}, "results": {"ts": [NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN]}, "test_build": {"version": "34.0.5", "revision": "8274648ad79f", "id": "20141126041045", "branch": "", "name": "Firefox"}}] TinderboxPrint: TalosResult: {}
Comment 36•9 years ago
|
||
Comment on attachment 8537836 [details] [diff] [review] NULL_to_None_v2.patch Review of attachment 8537836 [details] [diff] [review]: ----------------------------------------------------------------- this failed again: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jmaher@mozilla.com-618fbb19cf54/try-linux/try_ubuntu32_hw_test-chromez-bm104-tests1-linux-build774.txt.gz I think for the number of instances of null and what we gain from this, we should move onto more productive things. In the future where we do not deal with graph server, this might make more sense! So lets move onto some other fun bugs- there are a lot that will help us make talos better and more useful!
Attachment #8537836 -
Flags: review-
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•