1113114 - Remove metro specific code from talos

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

In the talos repository:
http://hg.mozilla.org/build/talos

there is code that was used to test firefox on metro (windows 8 specific interface).  Our work on metro has stopped, yielding extra code lying around.

This bug should be easy to solve, remove references to metro in the code and files that are metro specific.

Once you do that, create a patch (hg diff) and attach it to this bug.  Then we need to test it:
 * unit tests in talos
 * verifying that we can run a test locally

Some additional docs to run tests locally:
https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code

Comment 1

[Anmol Garg]

Hi Joel! 

Its my first time solving a bug. How do I go about it?

Thanks

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Hi Anmol, welcome to Mozilla!

There are a few easy steps:
1) get talos running locally: https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code
2) setup hg to have mercurial queues (https://developer.mozilla.org/en-US/docs/Mercurial_Queues) <- not required but recommended
3) create a new patch (queue)
4) search through the code, remove references to metro, etc.
5) test your changes (make sure at least one test runs locally for you)
6) attach a patch to this bug (hg qdiff is an easy way)

Here are some general resources for developing at Mozilla:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide

I know that is a lot to read and setup.  Steps 1 and 4 are the most important :)

I look forward to your patch, and no question is too simple.

Comment 3

[Anmol Garg]

Thanks for the pointers Joel! 

On running the test 

talos -n -d --develop --executablePath /usr/bin/firefox --activeTests ts --results_url ts.txt --datazilla-url ts.json --output ts_desktop.yml --mozAfterPaint

as mentioned in the first link, the browser opens and closes several times and ultimately gives the following error - 

Traceback (most recent call last):
  File "/home/anmol/talos/bin/talos", line 9, in <module>
    load_entry_point('talos==0.0', 'console_scripts', 'talos')()
  File "/home/anmol/talos/talos/run_tests.py", line 304, in main
    sys.exit(run_tests(parser))
  File "/home/anmol/talos/talos/run_tests.py", line 280, in run_tests
    talos_results.output(results_urls, **results_options)
  File "/home/anmol/talos/talos/results.py", line 79, in output
    _output.output(results, url, tbpl_output)
  File "/home/anmol/talos/talos/output.py", line 420, in output
    utils.info("TALOSDATA: %s" % json.dumps(results.datasets()))
  File "/home/anmol/talos/local/lib/python2.7/site-packages/datazilla-1.4-py2.7.egg/dzclient/client.py", line 136, in datasets
    'revision': self.revision[:50],
TypeError: 'NoneType' object has no attribute '__getitem__'

I am not able to figure out the reason for the error. Can you tell me what is the problem here?

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I am not sure why you get that error.  There is a chance this data was trying to upload to datazilla without proper credentials.

when you run talos, can you add this to your command line:
--results_url local.out --datazilla-url local.json

Comment 5

[Anmol Garg]

Hi Joel, 

The part you said to add was already present in the command

talos -n -d --develop --executablePath /usr/bin/firefox --activeTests ts --results_url ts.txt --datazilla-url ts.json --output ts_desktop.yml --mozAfterPaint

I will try to figure that out the error. Meanwhile can you please tell me how to identify the code specific to metro? I looked at some files which had the keyword metro. How should I ensure that I have exhaustively removed all the metro specific code?

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

search for metro in *.py files.  Also at the root of the repository is a metro/ folder with a couple files, those can be removed as well.

This is a command that works well to find all the instances:
grep -R metro * | grep -v tagcloud

Thanks for asking questions- we will get you setup!

Comment 7

[Vidya]

Hi Joel,

Under the run tests part: there is an option to specify executablePath. Is this the build path? Sorry if it is a stupid question. But I am unsure about this option.

Comment 8

[Vidya]

I tried putting my executable path as: /Applications/Firefox.app but it threw an error saying 

    mozversion.errors.AppNotFoundError: No application found

Any thoughts on this? I am currently using MAC OSX 10.9.1

Comment 9

[Vidya]

It ran successfully. I didn't take the path to the bin which I found in the path:  /Applications/Firefox.app/Contents/MacOS/firefox-bin

(In reply to Vidya from comment #8)
> I tried putting my executable path as: /Applications/Firefox.app but it
> threw an error saying 
> 
>     mozversion.errors.AppNotFoundError: No application found
> 
> Any thoughts on this? I am currently using MAC OSX 10.9.1

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

oh great, glad to hear you were able to get talos running!  And for reference, no question is too simple or basic.

Comment 11

[Vidya]

Hi Joel,

When I was running those tests, my browser closed and open several times and I was getting the message "Browser exited with error code: 0" every time it closed. I removed references to metro changes and I am getting the same message. How do I verify if the references to metro are properly removed?

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

great question.  First off, that is expected, we are measuring startup time.  I would verify by a grep command.  Also at the root of the talos repo there is a metro folder, do a 'hg rm metro/' and that should remove the files.

Thanks for this and I look forward to a patch!

Comment 13

[Vidya]

Hi Joel,

Thank you so much for the tips. I ran the command 'grep -R metro * | grep -v tagcloud' and it returned no instances after removing references to metro. 

So I ran tests again. Except for the first message all returned "Browser exited with error code: 0". First message was "Browser exited with error code: -9". Is this a concern?

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

that all sounds 100% normal!

Comment 15

[Vidya]

Attachment: Remove references to Metro

Created the patch.

Metro specific code removed from Talos. 

Locally tests are running normally.

Comment 16

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8541942 [details] [diff] [review]
Remove references to Metro

Review of attachment 8541942 [details] [diff] [review]:
-----------------------------------------------------------------

Overall this is really close.  I am curious about the issue in pageloader with the spacing, could we have some blocks incorrect?

Do take another look, if the changes are simple, I will get this uploaded and landed tomorrow!

Thanks for working on this!

::: talos/pageloader/chrome/pageloader.js
@@ +82,5 @@
>       * into the main window of the app which displays and tests content.
>       * chrome talos runs - tp-cmdline does the same however pageloader
>       * creates a new chromed browser window below for content.
>       *    
>       * Immersive firefox:

nit: please remove the line mentioning Immersive Firefox

@@ +234,5 @@
> +        if (reportRSS) {
> +          initializeMemoryCollector(plLoadPage, delay);
> +        } else {
> +          setTimeout(plLoadPage, delay);
> +        }

the spacing here looks odd.  Before we had 2 space, now we have 4 space for this one else block?

Comment 17

[Vidya]

Attachment: Remove Metro Specific Code

Updated with changes in the spacing and removed ImmersiveFirefox comment.

Comment 18

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8542079 [details] [diff] [review]
Remove Metro Specific Code

Review of attachment 8542079 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great!

Comment 19

[Joel Maher ( :jmaher ) (UTC -8)]

https://hg.mozilla.org/build/talos/rev/1d694e320697

Comment 20

[Joel Maher ( :jmaher ) (UTC -8)]

Thanks Vidya, this is landed and helps us out.  Currently on Talos we only have a few bugs currently on file:
https://wiki.mozilla.org/Auto-tools/Projects/Talos

If you check back on the wiki page in the next day or two, I know of a few things that we should get added and they will show up there.  If you are eager to hack on stuff, we have a list of projects here:
https://wiki.mozilla.org/Auto-tools/Projects/Everything#Project_Table

Pick one that is friendly and of interest.

Thanks again for fixing this bug!

Comment 21

[Vidya]

Thank you so much Joel for all the guidance. 
I will definitely check out the wiki page. Looking forward to contribute more towards this.