allow for talos to use a remote server with the .manifest files

RESOLVED FIXED

Status

RESOLVED FIXED
9 years ago
10 months ago

People

(Reporter: jmaher, Assigned: jmaher)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 436286 [details] [diff] [review]
patch to preprocess .manifest files during PerfConfigurator.py

for many tests like tp4, tsvg, etc... there is a .manifest file that has a list of URLs to test with.  

We need to be able to have the talos harness use an arbitrary URL (not hardcoded localhost) for testing on a remote server.


Since there is not agreement on the best solution, I am throwing a patch up that helps solve this problem.  This patch modifies PerfConfigurator.py to create a remote server version of the manifest (not overwrite) and then copy to the device.

NOTE: this patch isn't perfect, but I want to get agreement on the technique before spending the additional time testing all corner cases.
(Assignee)

Comment 1

9 years ago
Comment on attachment 436286 [details] [diff] [review]
patch to preprocess .manifest files during PerfConfigurator.py

can I get your feedback on this approach.
Attachment #436286 - Flags: review?(anodelman)
Attachment #436286 - Flags: feedback?(aki)
This happens before any timing happens right?
Seems good to me.
I'm not sure if I want this in the Perfconfigurator - since it is pretty much designed to just handle config file set up.  Might be better to have another script and the mobile talos buildbot masters can just have an extra step.
(Assignee)

Comment 4

9 years ago
good idea Alice.  Let me create a remotesetup script that does the work of manifest changing and maybe extension copy/registration. We can see how that looks.  stay tuned.
(Assignee)

Comment 5

9 years ago
Here is an idea:

Currently in PerfConfigurator, we do this:
if ('url' in line) and ('url_mod' not in line) and (self._remote == True):
                    parts = line.split(' ')
                    newline = ''
                    for part in parts:
                        if ('.html' in part):
                            newline += 'http://' + self.webServer + '/' + part
                        elif ('.xul' in part):
                            newline += 'http://' + self.webServer + '/' + part
                        else:
                            newline += part
                        if (part <> parts[-1]):
                            newline += ' '
                    line = newline

What I would like to do is refactor that a little bit so it looks like this:
def convertLineToRemote(self, line):
  return line
.
:
  if ('url' in line) and ('url_mod' not in line) and (self._remote == True):
    line = self.convertLineToRemote(line)



Then for the fix, I would like to do this:
remotePerfConfigurator.py:
  class remotePerfConfigurator(PerfConfigurator):
    def convertLineToRemote(self, line):
      # Do all the magic
      return modified line


this will allow for us to do all magic to a test line and possibly any other future PerfConfigurator changes without changing PerfConfigurator.py.

Right now this would involve a small refactor to the existing PerfConfigurator.py, then adding a new .py file.
(Assignee)

Comment 6

9 years ago
Created attachment 436551 [details] [diff] [review]
[checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)

Ok, here is a patch that does what I mentioned in my comment earlier today.  I subclass PerfConfigurator and do the url mangling in there.  

One thing I would like to change in the near future is remove getopt and use optparse.  this would allow for more code reuse between remotePerfConfigurator and PerfConfigurator.  

To run this, use the same cli as PerfConfigurator.py, just use remotePerfConfigurator.py instead.  All the args map in parity.
Assignee: nobody → jmaher
Attachment #436286 - Attachment is obsolete: true
Attachment #436551 - Flags: review?(anodelman)
Attachment #436551 - Flags: feedback?(aki)
Attachment #436286 - Flags: review?(anodelman)
Attachment #436286 - Flags: feedback?(aki)
Comment on attachment 436551 [details] [diff] [review]
[checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)

cool, seems like a low-risk change that can be assimilated later if we wish.
Attachment #436551 - Flags: feedback?(aki) → feedback+
I've got another question - this looks like it just updates the links in the config file - is that correct?  What about the contents of the manifest files themselves?
(Assignee)

Comment 9

9 years ago
good question.  If you look in the patch there is a remotePerfConfigurator:buildRemoteManifest() function that takes a given manifest filename on the host computer, opens it and replaces localhost with the webserver.  Then it write that data to a manifestname.remote and copies that to the devices in a known location while changing the .config file to reference that new .manifest location.
Comment on attachment 436551 [details] [diff] [review]
[checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)

*stamp*
Attachment #436551 - Flags: review?(anodelman) → review+
Is this blocked on check-in?
(Assignee)

Comment 12

9 years ago
my understanding is this is ready to checkin.  There is nothing that this is specifically blocking, but the next talos downtime would be good to land this.
Comment on attachment 436551 [details] [diff] [review]
[checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)

Checking in PerfConfigurator.py;
/cvsroot/mozilla/testing/performance/talos/PerfConfigurator.py,v  <--  PerfConfigurator.py
new revision: 1.18; previous revision: 1.17
done
RCS file: /cvsroot/mozilla/testing/performance/talos/remotePerfConfigurator.py,v
done
Checking in remotePerfConfigurator.py;
/cvsroot/mozilla/testing/performance/talos/remotePerfConfigurator.py,v  <--  remotePerfConfigurator.py
initial revision: 1.1
done
Attachment #436551 - Attachment description: patch to preprocess .manifest files during PerfConfigurator.py (2) → [checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)
(Assignee)

Comment 14

9 years ago
thanks for landing this!
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Component: New Frameworks → General
Product: Testing → Testing
You need to log in before you can comment on or make changes to this bug.