talos should consume mozprofile

RESOLVED FIXED

Status

Testing
Talos
P2
normal
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: Jeff Hammel, Assigned: parkouss)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozbase])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
The profile manipulation/creation stuff from Talos should be replaced
with mozprofile:

https://github.com/mozautomation/mozmill/tree/master/mozprofile
(Reporter)

Updated

6 years ago
Whiteboard: [mozbase]
(Reporter)

Updated

6 years ago
Depends on: 662683
(Reporter)

Updated

6 years ago
Blocks: 687549
(Reporter)

Comment 1

6 years ago
Once the talos code uses mozprocess, we need to figure out how to land this. There are at least two paths forward:

1. For talos == a python package, the solution is pretty simple: add `mozprocess== someversion` to the setup.py `install_requires`
2. For talos.zip, we can write a script that packages the talos subdirectory (http://hg.mozilla.org/build/talos/file/tip/talos) with mozprocess modules (http://hg.mozilla.org/build/talos/file/tip/talos) as a subdirectory of that into the talos.zip file. We will also have to include mozinfo.py but since that is a single file, that's not really that bad.

Note that while 2 works fine for now by taking advantage of how python imports things, it will work less fine if we have a more complex chain of dependencies (say, if Talos used mozrunner and mozprofile, the former consuming the latter and mozprocess).  We need a real strategy going forward to have Talos consume python, but need to make sure we have all of the use-cases before moving to something more drastic.

However, I am not sure if 1+2 cover all of the cases here.  :bear, I believe there are some hg checkouts....for remote testing?  Anything else? Do you have any idea how to deploy python package dependencies for this/these scenarios?
(Reporter)

Comment 2

6 years ago
oops, comment 1 was meant for bug 694625. the logic still kinda applies but... ;)
(Reporter)

Updated

6 years ago
Priority: -- → P2
Whiteboard: [mozbase] → [mozbase][mozharness+talos]
(Reporter)

Updated

6 years ago
Blocks: 713055
(Reporter)

Updated

6 years ago
No longer blocks: 713055
(Reporter)

Updated

5 years ago
Whiteboard: [mozbase][mozharness+talos] → [mozbase]
(Reporter)

Updated

5 years ago
Duplicate of this bug: 655171
(Assignee)

Comment 4

3 years ago
Created attachment 8572495 [details] [diff] [review]
talos should consume mozprofile

Hey guys,

I started to look at this bug as recommended by wlach yesterday after the meeting. :)

So, this is my first step in talos. I ran talos locally and the patch seems to work fine. I would like you to look at it, and tell me if I'm on the right direction, if there's something missing, misunderstood, etc.
Attachment #8572495 - Flags: feedback?(wlachance)
Attachment #8572495 - Flags: feedback?(jmaher)
Comment on attachment 8572495 [details] [diff] [review]
talos should consume mozprofile

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

I think this is the right path, one quirk is we create talos as a .zip bundle for android, this could be problematic with adding mozprofile:
http://hg.mozilla.org/build/talos/file/64f3fd204a9a/create_talos_zip.py

please check to see if that works and you can unzip and run some tests :)
Attachment #8572495 - Flags: feedback?(jmaher) → feedback+
(Assignee)

Comment 6

3 years ago
Yeah, it does not work (when I just add mozprofile to the list of dependencies inside the create_talos_zip.py) because there is a dependency of manifestparser for mozprofile:

http://mozbase.readthedocs.org/en/latest/_modules/mozprofile/addons.html#AddonFormatError

I suspect that it should work if I explicitly add manifestparser to the dependencies in create_talos_zip.py.

Yay, I find that bad because:

1. we have to declare dependencies in two files (knowledge duplication)
2. it seems that we need to handle the dependencies of dependencies on our own... (ouch)

Anyway, that's outside the scope of this bug.

Maybe I can just add the two dependencies manifestparser and mozprofile in the create_talos_zip.py file for now.

hmm, how can I run tests in an unzipped talos zip environment ?
I really want to get rid of talos.zip, it just isn't a priority.  Although of the 6 tests we run on android (where we use talos.zip) I am trying to get 3 of them shut off.  But that still leaves 3 around for a while (6+ months).

Maybe we could have create_talos_zip.py read from requirements.txt?  Again, out of the scope of this bug, but a thought!

to run tests locally:
unzip talos.xxx.zip
cd talos
python PerfConfigurator.py -a tresize -e <path>/firefox --develop --output j.yml
python run_tests.py -d -n j.yml
(Assignee)

Comment 8

3 years ago
Created attachment 8572591 [details] [diff] [review]
talos should consume mozprofile

Ok, patch is updated so it should work with talos.zip. I ran your test commands and everything runs fine. (Thanks for those commands BTW!)

(In reply to Joel Maher (:jmaher) from comment #7)
> I really want to get rid of talos.zip, it just isn't a priority.  Although
> of the 6 tests we run on android (where we use talos.zip) I am trying to get
> 3 of them shut off.  But that still leaves 3 around for a while (6+ months).
> 
> Maybe we could have create_talos_zip.py read from requirements.txt?  Again,
> out of the scope of this bug, but a thought!

Hm, that would resolve only the first point, we still have to handle explicitly the manifestparser dependency.

Well, if the goal is to remove that talos.zip, maybe we can live with that until that time. :)
Assignee: nobody → j.parkouss
Attachment #8572495 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8572495 - Flags: feedback?(wlachance)
Attachment #8572591 - Flags: review?(jmaher)
Comment on attachment 8572591 [details] [diff] [review]
talos should consume mozprofile

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

testing on try server now.

::: talos/ffsetup.py
@@ +84,5 @@
>              remote_dir = self.ffprocess.copyDirToDevice(profile_dir)
>              profile_dir = remote_dir
>          return temp_dir, profile_dir
>  
>      def InstallInBrowser(self, browser_path, dir_path):

how does this work with mozprofile?  I suspect it will work just fine, but this is something we should look at in talos to see if we can remove this or make it cleaner.
Attachment #8572591 - Flags: review?(jmaher) → review+
Depends on: 1139488
Depends on: 1146005
https://hg.mozilla.org/build/talos/rev/6d6b290cbe09
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Depends on: 1151466
You need to log in before you can comment on or make changes to this bug.