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

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

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

Attachment: 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.

Comment 1

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

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

can I get your feedback on this approach.

Comment 2

[Aki Sasaki (not active)]

This happens before any timing happens right?
Seems good to me.

Comment 3

[alice nodelman [:alice] [:anode]]

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.

Comment 4

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

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.

Comment 5

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

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.

Comment 6

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

Attachment: [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.

Comment 7

[Aki Sasaki (not active)]

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.

Comment 8

[alice nodelman [:alice] [:anode]]

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?

Comment 9

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

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 10

[alice nodelman [:alice] [:anode]]

Comment on attachment 436551 [details] [diff] [review]
[checked in] patch to preprocess .manifest files during PerfConfigurator.py (2)

*stamp*

Comment 11

[alice nodelman [:alice] [:anode]]

Is this blocked on check-in?

Comment 12

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

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 13

[Nick Thomas [:nthomas] (UTC+12)]

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

Comment 14

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

thanks for landing this!