Closed Bug 694638 Opened 13 years ago Closed 9 years ago

talos should consume mozprofile

Categories

(Testing :: Talos, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: k0scist, Assigned: parkouss)

References

Details

(Whiteboard: [mozbase])

Attachments

(1 file, 1 obsolete file)

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

https://github.com/mozautomation/mozmill/tree/master/mozprofile
Whiteboard: [mozbase]
Depends on: 662683
Blocks: 687549
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?
oops, comment 1 was meant for bug 694625. the logic still kinda applies but... ;)
Priority: -- → P2
Whiteboard: [mozbase] → [mozbase][mozharness+talos]
Blocks: 713055
No longer blocks: 713055
Whiteboard: [mozbase][mozharness+talos] → [mozbase]
Attached patch talos should consume mozprofile (obsolete) — Splinter Review
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+
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
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+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: