1116220 - talos whitelist.py has duplicated code with mainthreadio.py

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

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

We should remove the duplicated code in between whitelist.py and mainthreadio.py:
http://hg.mozilla.org/build/talos/file/tip/talos/whitelist.py#l12

Ideally this would live in utils.py or some other common place.  

To test this we would need to run xperf (windows 7 only), so I would be happy to test this on our try server instead of testing this fully locally.

to learn more about Talos and how to get started here is a wiki outlining the work:
https://wiki.mozilla.org/Auto-tools/Projects/Talos

Comment 1

[Vidya]

Hi Joel,

Do I need to have windows 7 as my OS to work on this?

Comment 2

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

you will not need windows 7 to work on this, in fact, I think you could easily work on it locally, test it as best as possible and I would be happy to test it on the live test which requires windows 7.

Comment 3

[Vidya]

Hi Joel

This is the path I am following:
1. Remove indexed_items function from whitelist.py and mainthreadio.py
2. Place this function in utils.py
3. Test it as best as possible

Is this correct?

Comment 4

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

yes, this seems great.  Please ensure that whitelist.py and mainthreadio.py both have 'import utils' at the top :)

Thanks for working on this bug!

Comment 5

[Vidya]

Great, thanks. :)

Also when you said "test it as best as possible" what all cases needs to be considered here?

I will be running the tests as mentioned on https://wiki.mozilla.org/Buildbot/Talos/Running#Running_locally_-_Source_Code

But what else can be done to verify that the changes made are correct or not?

Comment 6

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

this is sort of hard to verify, these files require inputs from the xperf toolchain.  If you can run them locally and make sure the errors/output (without the appropriate file) are identical before/after your change that will be good enough :)

Thanks for checking on that detail.

Comment 7

[Vidya]

Attachment: Remove function indexed_items and put it in utils.py

I ran the test mentioned in https://wiki.mozilla.org/Buildbot/Talos/Running#Testing_Locally. Worked perfectly fine before and after change.

Comment 8

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

Comment on attachment 8543142 [details] [diff] [review]
Remove function indexed_items and put it in utils.py

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

Thank you, this patch looks great!  Let me test it on the xperf job.

Comment 9

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

https://hg.mozilla.org/build/talos/rev/55e869d6e5f3

Comment 10

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

Thanks Vidya, I tested this out and it works great!  If you are interested in another bug, do check out bug 1116222 :)

Comment 11

[Vidya]

Thanks Joel. :)