474478 - set up talos on winmo

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Aki Sasaki (not active)]

Set up talos on wince devices.

Comment 1

[Aki Sasaki (not active)]

Progress so far is in https://wiki.mozilla.org/User:Asasaki:TouchProSetup

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

I don't think this blocks the 1.9.2 release, actually, though it might block the WinMo Fennec release?

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

Hm, maybe I'm wrong about that; I'll check with Stuart.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #2)
> I don't think this blocks the 1.9.2 release, actually, though it might block
> the WinMo Fennec release?

It does block the WinMo Fennec release, which I suppose will be a point release of 1.9.2, not sure how to reflect that since I don't see the blocking fennec flag on this bug.

Comment 5

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

Attachment: WindowsMobile.patch

Adding a serious WIP patch to show a solution on how to get this running on a windows mobile phone (Omnia II via test-agent.exe ip shell).  We can make a stable patch if we can decide on:
1) how to import the correct python modules based on remote os
2) how to specify the remote web server
3) how to manage performance counters
4) remove hardcoded file paths for temporary transfer from device
5) how we want to setup and host the test files

enjoy this patch so far

Comment 6

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

Attachment: WindowsMobile.diff

uploading cvs diff (only of existing files, missing 3 new files) which allows for clean integration into existing talos.  Testing for basic functional regressions on Mac and Linux with a fresh cvs checkout + patch + 3files and it works.  Also this works for ts on Windows Mobile (using a presetup webserver)

Comment 7

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

Attachment: additional windows mobile files

These 3 files are required as well for this patch to work.

Comment 8

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

To clarify what it takes to run this:
1) a device with test-agent on there.  This was tested with http://people.mozilla.org/~blassey/test-agent/test-agent.exe on a winmo device (htc touch pro or omnia 2)
2) get the ip address of your device
3) setup talos like you would on a machine (I did this on winxp) so you have all the talos bits and static pageset available and hosted on a local webserver
4) copy a build of fennec to the device (my example is \tests\fennec)
5) run PerfConfigurator.py:
python PerfConfigurator.py -v -e \\tests\\fennec\\fennec.exe -t omnia2 -b mobile-browser --activeTests ts --sampleConfig mobile.config --browserWait 60 --noChrome --output local.config --remoteDevice 192.168.1.110 --remotePort 27020 --webServer 192.168.1.114:8888 --deviceRoot \\tests\\talos
 ** Note: 192.168.1.110 is the ip of my device from step #2
          --remotePort of 27020 is default for test-agent from step 1
          --deviceRoot is where to copy talos bits and profile to
          --webServer is the ip address of the webserver hosting the pageset
6) run python run_tests.py local.config

Since I am guessing at the talos setup, there could be one or two missing things here, but these are the steps I took to run this.

Comment 9

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

Running proposed patches on staging to ensure that they don't break existing talos functionality.

Comment 10

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

Ran green overnight, looks like the patch doesn't interfere with standard talos.

Comment 11

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

First pass of review (there's a lot of files, so this could take a little longer than I thought):

in ffsetup.py

    if (self._remoteWebServer <> 'localhost'):
      #TODO: p2 is hardcoded, how do we determine what prefs.js has hardcoded?
      remoteCode = """    
user_pref("capability.principal.codebase.p2.granted", "UniversalPreferencesWrite UniversalXPConnect UniversalPreferencesRead");
user_pref("capability.principal.codebase.p2.id", "http://%(server)s");
user_pref("capability.principal.codebase.p2.subjectName", "");    
""" % { "server": self._remoteWebServer }
      user_js_file.write(remoteCode)


We have settings like this in the various built in talos profiles (ie, base_profile/prefs.js) - this can probably go there as well.  Make sure that it ends up in all profiles.

in ttest.py:

          if (self.remote == True):
            self.ffprocess._platform.getFile(process, browser_config['browser_log'])
            results_file = open(browser_config['browser_log'], 'a')
            results_file.write("__startSecondTimestamp%s__endSecondTimestamp\n" % int(time.time()*1000))
            results_file.close()


This isn't a good way to generate the second time stamp - the second timestamp should reflect the time that the firefox process was completed.  Since we only check for the process closing every x seconds we are introducing a lot of noise here.  In the basic talos code bcontroller.py handles this by doing a wait on the firefox process.

        # Execute the test's tail script if there is one
        if 'tail' in test_config:
          try:
            subprocess.call(['python', test_config['tail']])
          except:
            raise talosError("error executing tail script: %s" % sys.exc_info()[0])


Head and tail script execution isn't going to work as it isn't being pushed through remote.

        if (self.remote == True):
          if ('process' in vars()):
            self.ffprocess._platform.getFile(process, browser_config['browser_log'])
            results_file = open(browser_config['browser_log'], 'a')
            results_file.write("__startSecondTimestamp%s__endSecondTimestamp\n" % int(time.time()*1000))
            results_file.close()

I don't know what this is trying to do here.  Since this is part of what is run if there is an exception regular talos attempts to open the browser_log and then dump the contents.  You are opening the log, writing to it and then doing nothing with it.

in cmanager_winmo.py:

# ***** BEGIN LICENSE BLOCK *****^M
# Version: MPL 1.1/GPL 2.0/LGPL 2.1^M


Spurious windows line endings throughout.

in   run_tests.py

def browserInfo(browser_config):
  """Get the buildid and sourcestamp from the application.ini (if it exists)
  """
  appIniFileName = "application.ini"
  appIniPath = os.path.join(os.path.dirname(browser_config['browser_path']), appIniFileName)


I'm betting this doesn't work with a remote browser quite yet.

                 'remote'       : yaml_config['remote'],
                    'host'         : yaml_config['deviceip'],
                    'port'         : yaml_config['deviceport'],
                    'webserver'    : yaml_config['webserver'],
                    'deviceroot'   : yaml_config['deviceroot']}

Let's not make these required since they are only used on mobile platforms.  Follow the formula used for branch_name or test_name_extension.  Should also be pulled out of sample.config

in ffprocess_linux.py:

  def getFile(self, handle, localFile = ""):
    fileData = ''
    if os.path.isfile(handle):^M
      results_file = open(handle, "r")^M
      fileData = results_file.read()^M
      results_file.close()^M
    return fileData^M

More bad line endings.

Comment 12

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

Attachment: WinMo patch round 2 (WIP)

wip to address the initial feedback from Alice.  I have also used fake_add script to include my new files in the single patch.  The only thing that isn't clean is the adding of the remote web server to the prefs.js file dynamically.

Comment 13

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

Attachment: WinMo patch round 3

This is an updated patch fixing:
1) no windows newlines in patch
2) no extra files such as .manifest in patch
3) log file issues that required \\\\ fixed.  All python code uses / to talk to winmo device driver.
4) key error send_to_graph, sourcestamp is missing run_tests.py line 200 results_string.append.  Line 279, browserInfo open(application.ini).  PerfConfigurator.py will get this information and store in yaml for mobile.config based tests.


Aki, can you please give this patch a try on your setup.  You will need blassey's latest agent from http://people.mozilla.org/~blassey/test-agent/test-agent.exe.  If this works good for you to get results to the graph server, lets r? :alice on this.

Lastly, there are two other bugs: bug 519568 and bug 529137 which will cause conflict with this patch.  Once those are approved and checked in, I will update patch to work with latest cvs code branch.

Comment 14

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

Attachment: WinMo patch round 3 - no bitrot

cleaned up for ttest.py bitrot.

Comment 15

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

Attachment: Migrate python code to classes

This is the first patch of 3.  Doing this makes it simpler to review.  This patch has a lot of changes, but the majority is whitespace.  There are a few changes made, but they are only related to current functionality and making python scripts into a class.  

The second patch will clean up code to work better in classes along with greater flexibility for adding in another platform.  The third patch will be plugging in the winmo stuff.

Comment 16

[Aki Sasaki (not active)]

Attachment: add winmo-touchpro-01 and 02 to graphs database

Comment 17

[Aki Sasaki (not active)]

Comment on attachment 415974 [details] [diff] [review]
add winmo-touchpro-01 and 02 to graphs database

http://hg.mozilla.org/graphs/rev/7a51a07defb1

Comment 18

[Aki Sasaki (not active)]

justdave landed this in the graphs-stage db.

I should be able to automate Ts in staging soon. The main pieces left are

* Getting new builds onto the device.  Ideally, this would be something I could invoke from the command line:
** create the directory tree on device a la |mkdir -p| (this might be multiple steps)
** push a local .exe to device
** unzip it
* Rebooting non-interactively from the command line.  This could be an expect script, or something using the existing python test agent api.
* Verifying that the device came back up from command line... this'll probably be a poll that checks 27020 until max_wait.

Then it's a matter of putting all the steps together in buildbot, which I expect to go fast once the above is done.

Comment 19

[Aki Sasaki (not active)]

Oh, also, removing the old build is important.

I think jmaher has at least some of these pieces already written; we can work on the rest.

Comment 20

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

All of these tasks are done, and included in the devicemanager.py (I think we need to update that and agent to get zip support)  

* new builds on device: copy .exe, unzp on device
* mkdir -p, pushDir() does that automatically
* removing old build - removeDir() - does a recursive removal

As for the reboot, that can be scripted pretty easily, but there are no examples in any of my code to handle that.

Let me know if you just want a single script for it all, or a set of scripts to do each action.

Comment 21

[Aki Sasaki (not active)]

Running a while loop on 1.0a4 Ts.
Got data sent to graphs-stage; that's cool.
But how does Ts ever measure negative?

http://graphs-stage.mozilla.org/graph.html#tests=[{%22test%22:%2212%22,%22branch%22:%2211%22,%22machine%22:%22410%22}]

Hoping that's a one time anomaly.  I'll let this run for a while.

Comment 22

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #20)
> All of these tasks are done, and included in the devicemanager.py (I think we
> need to update that and agent to get zip support)  

not sure if you mean update the posted test-agent.exe build or update the version on the devices.  But the build on my people page has that. I have no idea what version is on the devices.

Comment 23

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

Comment on attachment 414328 [details] [diff] [review]
Migrate python code to classes

I think that this has suffered from some bit rot, any chance that I can get a refresh?

Comment 24

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

I just did a fresh cvs checkout and applied the patch via 'patch -p0 < classpatch.diff':
Macintosh-6:talos joelmaher$ patch -p0 < classpatch.diff 
patching file bcontroller.py
patching file ffprocess.py
patching file ffprocess_linux.py
patching file ffprocess_mac.py
patching file ffprocess_win32.py
patching file ffsetup.py
patching file run_tests.py
patching file ttest.py
Macintosh-6:talos joelmaher$ 


My uber patch would have bitrot, but the smaller patch for just classes is fine..phew.

Comment 25

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

Thanks for the update.

Comment 26

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

Comment on attachment 414328 [details] [diff] [review]
Migrate python code to classes

This doesn't look like it is changing any actual talos operation and the generation of objects is good.

Comment 27

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

As a note, talos code changes only get checked in during scheduled downtimes so we'll have to sync up to get this landed.

Comment 28

[Brad Lassey [:blassey] (use needinfo?)]

when is the next scheduled downtime?

Comment 29

[Brad Lassey [:blassey] (use needinfo?)]

there's a releng downtime scheduled for Friday December 18, 2009 at 9:00pm PST, so can this land during that?

Comment 30

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #29)
> there's a releng downtime scheduled for Friday December 18, 2009 at 9:00pm PST,
> so can this land during that?

We can try, but its all last minute, and hasnt yet been tested in staging. We'll see how it goes between now and the downtime, and make a last minute call at that point. Alice will update here when we know about staging.

(The last-minute downtime late tonight was for an unrelated problem that couldnt wait the weekend; I dont want to delay/compromise that work.)

Comment 31

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

let me know if there is anything I can do to help.  If you need extra testing or verification, ping me.  Otherwise, would rather keep things running smooth and get this and the other pending talos patches checked in with no hiccups.

Comment 32

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

Currently testing the 'migrate to classes' patch on talos staging.  If all goes
well it will land in tonight's scheduled downtime.

Comment 33

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

Comment on attachment 414328 [details] [diff] [review]
Migrate python code to classes

Issue found during talos staging tests:

/home/mozqa/talos-slave/talos-data/talos/ttest.py:84: SyntaxWarning: import * only allowed at module level
  def __init__(self):
/home/mozqa/talos-slave/talos-data/talos/ttest.py:84: SyntaxWarning: import * only allowed at module level
  def __init__(self):
/home/mozqa/talos-slave/talos-data/talos/ttest.py:84: SyntaxWarning: import * only allowed at module level
  def __init__(self):

Comment 34

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

Attachment: Migrate python code to classes (2)

had to adjust the import statements in ttest.py to be compliant with all versions of python.  Here is a diff of the current patch and previous patch:

> +++ ttest.py	24 Nov 2009 19:38:18 -0000
> @@ -56,283 +56,310 @@ import re
1482d1470
< +        cmanager = None
1484c1472
< +            cmanager = __import__('cmanager_linux')
---
> +            from cmanager_linux import *
1488c1476
< +            cmanager = __import__('cmanager_win32')
---
> +            from cmanager_win32 import *
1492c1480
< +            cmanager = __import__('cmanager_mac')
---
> +            from cmanager_mac import *
1753c1741
< +                    cm = cmanager.CounterManager(browser_config['process'], counters)
---
> +                    cm = CounterManager(browser_config['process'], counters)


tested on ubuntu linux with firefox and ts.

Comment 35

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

New patch on talos staging.

Comment 36

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

Comment on attachment 418459 [details] [diff] [review]
Migrate python code to classes (2)

Considering that the fix to the last patch was minor I'm going to r+ this and clear it for tonight's downtime.  There are a number of talos patches waiting for this to land and it is in our best interest to get it landed.

Comment 37

[Aki Sasaki (not active)]

Comment on attachment 418459 [details] [diff] [review]
Migrate python code to classes (2)

Checking in bcontroller.py;
/cvsroot/mozilla/testing/performance/talos/bcontroller.py,v  <--  bcontroller.py
new revision: 1.5; previous revision: 1.4
done
Checking in ffprocess.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess.py,v  <--  ffprocess.py
new revision: 1.13; previous revision: 1.12
done
Checking in ffprocess_linux.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_linux.py,v  <--  ffprocess_linux.py
new revision: 1.13; previous revision: 1.12
done
Checking in ffprocess_mac.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_mac.py,v  <--  ffprocess_mac.py
new revision: 1.16; previous revision: 1.15
done
Checking in ffprocess_win32.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_win32.py,v  <--  ffprocess_win32.py
new revision: 1.9; previous revision: 1.8
done
Checking in ffsetup.py;
/cvsroot/mozilla/testing/performance/talos/ffsetup.py,v  <--  ffsetup.py
new revision: 1.13; previous revision: 1.12
done
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.55; previous revision: 1.54
done
Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.37; previous revision: 1.36
done

Comment 38

[Aki Sasaki (not active)]

Comment on attachment 418459 [details] [diff] [review]
Migrate python code to classes (2)

backed out.

Traceback (most recent call last):
  File "run_tests.py", line 421, in <module>
    test_file(arg)
  File "run_tests.py", line 383, in test_file
    browser_dump, counter_dump, print_format = TTest().runTest(browser_config, test)
  File "/home/mozqa/talos-slave/talos-data/talos/ttest.py", line 251, in runTest
    cm = cmanager.CounterManager(browser_config['process'], counters)
NameError: global name 'cmanager' is not defined

Comment 39

[Aki Sasaki (not active)]

I'm beginning to wonder why we aren't forking, again.
We could call it "remote talos" (rtalos?) or "test-agent talos" and only use the fork for devices that can't run python.

Comment 40

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

While I think this is could be a short win to fork the code, I could see a big problem 6 months down the road when new talos changes come in and need to be ported 100% to remote clients.  With this integrated system in place we will have to do some changes to keep the remote stuff working, but a lot of the code will just work.  I could remove hundreds of lines of code if we were only running remote and not supporting the other platforms.

As for the failure above, I am make sure future patches are tested on the whole test suite, not just ts.  We should also do a full run on talos staging so these types of errors are not found, although my understanding is this previous patch was rushed in since there was a downtime and not a full run was done.

Comment 41

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

Attachment: Migrate python code to classes (3)

updated to add 'self' in front of countermanager class instance.  Tested on Linux and Mac for full standalone run instead of just ts.  I verified that This run included tests with counters and that it ran to completion without python errors.

Comment 42

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

Comment on attachment 418677 [details] [diff] [review]
Migrate python code to classes (3)

This patch also contains all the local changes to manifest files that makes standalone talos work - these changes are not wanted for production talos.

Comment 43

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

Attachment: Migrate python code to classes (4)

updated patch to work with recent e10s checkin.  Tested on full run with sample.config on linux with no python errors and real output in my output directory.  Also verified in the .diff file that there are no other files from the standalone distribution included.

This patch applies clean to the talos tree as of 1 hour previous to uploading this patch.

Comment 44

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

As we have had trouble with this patch before I'd like to do a staging run before we land, but I'm out on holiday till the new year.  If it can't wait till then please ping someone in #build.

Comment 45

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

Alice, thanks for letting me know your schedule.  I agree that running this in staging is a good idea.  If we can get it in staging next week that would be good.  If you have any indication of how this will be pushed (before or after the child process patch) that would be helpful.  I will make a patch that will apply after the child process patch just in case.

Comment 46

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

jmaher: does your latest revised patch:
a) include fixes for the problems we hit last time we roll the previous patch to production? 
b) take account of talos changes that landed and rolled out during last week's downtime?

Comment 47

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

yes, my patch resolves the previous problems (not including standalone file references, and fixed issues with counter manager which was not seen in ts tests).  Also my patch has been updated and applies cleanly as of 12-22.  The last downtime I knew about was 12-18.

Comment 48

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

I just verified that this patch applies cleanly to a fresh cvs checkout of talos.

Comment 49

[Aki Sasaki (not active)]

Joel:

python ./install.py winmo-touchpro-01 fennec-1.0a4pre.en-US.wince-arm.exe 
connecting to: winmo-touchpro-01
Traceback (most recent call last):
  File "./install.py", line 10, in ?
    dm = devicemanager.DeviceManager(sys.argv[1], 27020)
AttributeError: 'module' object has no attribute 'DeviceManager'

I'm guessing I have an old devicemanager.py ?

Comment 50

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

is this patch already on talos staging?  What is the timeframe for doing that?

Comment 51

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

Latest patch now on talos staging.  Will have results by tomorrow.

Comment 52

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

for Aki's private version of talos there are two remaining issues:
1) clock synchronization between start/stop times
2) an issue that aki found after running for a while where a variable was None instead of a string.  I have lost the pastebin of that.


In addition there was an issue Aki found after a reboot where the wifi didn't connect up (until ~30 minutes later).  

This large patch is landed on staging and once this is pushed to production, I will submit a second patch with some refactoring and additional changes to work with remote.  This second patch will get us to where Aki is so he can use production code.

Comment 53

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

Attachment: [checked in]Migrate python code to classes (5)

fixed for bitrot in bcontroller.

Comment 54

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

Comment on attachment 421538 [details] [diff] [review]
[checked in]Migrate python code to classes (5)

Checking in bcontroller.py;
/cvsroot/mozilla/testing/performance/talos/bcontroller.py,v  <--  bcontroller.py
new revision: 1.9; previous revision: 1.8
done
Checking in ffprocess.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess.py,v  <--  ffprocess.py
new revision: 1.16; previous revision: 1.15
done
Checking in ffprocess_linux.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_linux.py,v  <--  ffprocess_linux.py
new revision: 1.15; previous revision: 1.14
done
Checking in ffprocess_mac.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_mac.py,v  <--  ffprocess_mac.py
new revision: 1.18; previous revision: 1.17
done
Checking in ffprocess_win32.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_win32.py,v  <--  ffprocess_win32.py
new revision: 1.11; previous revision: 1.10
done
Checking in ffsetup.py;
/cvsroot/mozilla/testing/performance/talos/ffsetup.py,v  <--  ffsetup.py
new revision: 1.15; previous revision: 1.14
done
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.58; previous revision: 1.57
done
Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.40; previous revision: 1.39
done

Comment 55

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

Attachment: [checked in]fix for cmanager to use ffprocess as a class

additional patch for cmanager to use ffprocess as a class.  This assumes the first patch is applied as well as the patch from bug 519568.

Comment 56

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

Attachment: new files for windows mobile tests

This is only new files for talos to run on windows mobile. No changes to existing files, and much easier to review and get landed during the next talos downtime.

Comment 57

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

Attachment: config changes and new command line parameters

This is the second patch which depends on https://bugzilla.mozilla.org/attachment.cgi?id=422993.  There are minimal changes to running talos code (less chance of regression) and this patch should be easier to review for landing during the next talos downtime.

Comment 58

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

Comment on attachment 422994 [details] [diff] [review]
config changes and new command line parameters

In PerfConfigurator.py:

I'd like to see error checking when 'remote' flag is set.  Being remote seems to have a bunch of required secondary configuration options, we should fail if all those options are not present.

This code:
+        if (self._remote == True):
+            localfilename = "remoteapp.ini"
+            parts = self.deviceRoot.split("\\")
+            if (len(parts) > 1):
+                dirSlash = "\\"
+            else:
+                dirSlash = "/"
+
+            parts = self.exePath.split(dirSlash)
+            remoteFile = dirSlash.join(parts[0:-1]) + dirSlash + masterIniSubpath
+            
+            self.testAgent.getFile(remoteFile, localfilename)
+            master = open(localfilename)

would be better written with os.path.normpath.


In bcontroller.py:

I'm seeing host/port being added, but I don't see anywhere them being used.  Am I missing something here?

Comment 59

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

Comment on attachment 422993 [details] [diff] [review]
new files for windows mobile tests

Quibble:

I think that it would better to just leave out cmanager_winmo for now - since it is all noop anyway.  Just turn off counter collection in mobile.config and we should be able to get along without it.  Right now I could see it just being a source of confusion.

Comment 60

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

Attachment: config changes and new command line parameters (2)

updated patch for error handing for remote specific command line options in PerfConfigurator.py as well as cleaned up some path building.  We cannot use os.path.normpath since this is on a remote device and we are not guaranteed to be running on the same OS with same slash orientation.  Our assumption we are making is our device side agent will translate (if necessary) and slashes so we will just use / for all items.

One other questions was about bcontroller.py not using host, port.  This is because I didn't want to modify/restructure running code, so those changes are in the 3rd patch.

Comment 61

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

Alice, in regards to comment 59 above, I would have to add code to special case the counter manager in the case where we did accidentally try to run it with counters and we forgot about it.  We already have some preliminary counters working via the device agent, they just need to be implemented.  

Please let me know if you would like me to add code in ttest.py to check if we are running remote and disable the counters.

Comment 62

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

Comment on attachment 424993 [details] [diff] [review]
config changes and new command line parameters (2)

Looking this over, there is already sourcestamp/buildid collection written into talos in run_tests.py - so I don't think that it belongs in the PerfConfigurator.  Look for the browserInfo function - in fact, since that is still being run you might just end up overwriting the sourcestamp/buildid you are have added to the config file.

Comment 63

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

actually I had to add that as a result of a failure when Aki was running the tests originally.  We require the buildid sourcestamp and there are cli options to define those.  If those options don't exist or it isn't in the original file, then we actually read the application.ini file.  Believe me, it would be a lot simpler if I didn't have to do those functions in PerfConfigurator.py!  

Still, maybe I misunderstood it all when I originally wrote it...let me know if I did overlook something.

Comment 64

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

Attachment: new files for windows mobile tests (2)

updated with no cmanager_winmo and a winmo.config file!

Comment 65

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

Attachment: new files for windows mobile tests (3)

updated with a more complete winmo.config

Comment 66

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

Attachment: config changes and new command line parameters (3)

updated to remove sourcestamp/repository collection from perfconfigurator.py.  This will live in run_tests.py (as it does currently), but the code there will change live running bits, so I will do that in the next patch that will require staging.

Comment 67

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

I still see cmanager_winmo, wasn't that to be pulled out?

Also, there are still references to sourcestamp collection in winmo.config and run_tests.py that should be removed.

Comment 68

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

Attachment: [checked in]new files for windows mobile tests (4)

cleaned up config file to remove sourcestamp and no cmanager_winmo.py

Comment 69

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

Attachment: config changes and new command line parameters (4)

updated this to remove sourcestamp from run_tests.py and mobile.config.

Comment 70

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

Do you want those updates in mobile.config?

Comment 71

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

Attachment: [checked in]config changes and new command line parameters (5)

updated with removal of mobile.config changes as they are now in winmo.config.

Comment 72

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

Tried latest patches in staging.  Ran fine.

Comment 73

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

Checking in PerfConfigurator.py;
/cvsroot/mozilla/testing/performance/talos/PerfConfigurator.py,v  <--  PerfConfigurator.py
new revision: 1.14; previous revision: 1.13
done
Checking in bcontroller.py;
/cvsroot/mozilla/testing/performance/talos/bcontroller.py,v  <--  bcontroller.py
new revision: 1.11; previous revision: 1.10
done
RCS file: /cvsroot/mozilla/testing/performance/talos/devicemanager.py,v
done
Checking in devicemanager.py;
/cvsroot/mozilla/testing/performance/talos/devicemanager.py,v  <--  devicemanager.py
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/testing/performance/talos/ffprocess_winmo.py,v
done
Checking in ffprocess_winmo.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_winmo.py,v  <--  ffprocess_winmo.py
initial revision: 1.1
done
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.59; previous revision: 1.58
done
RCS file: /cvsroot/mozilla/testing/performance/talos/winmo.config,v
done
Checking in winmo.config;
/cvsroot/mozilla/testing/performance/talos/winmo.config,v  <--  winmo.config
initial revision: 1.1
done

Comment 74

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

Attachment: last patch in series for ts on winmo (1)

final patch for talos winmo ts.  This hooks up the new functions and changes running code.  Also this will ignore 'hashlib' if it doesn't exist on the host platform.

After successful review, I would like to run this in staging for a few days.  I would also like to see Aki run this on his winmo setup when he has a chance.  This is just a fresh cvs checkout along with this patch.

Comment 75

[Aki Sasaki (not active)]

patching file devicemanager.py
Hunk #1 FAILED at 30.
Hunk #2 FAILED at 119.
Hunk #3 FAILED at 277.
Hunk #4 FAILED at 338.
Hunk #5 FAILED at 515.
Hunk #6 FAILED at 604.
6 out of 6 hunks FAILED -- saving rejects to file devicemanager.py.rej

Tried cvs up -A and noted there were no diffs.

Comment 76

[Aki Sasaki (not active)]

blassey helped me w/ this -- just had to remove the trailing ^M and the patch applied.

Comment 77

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

Attachment: last patch in series for ts on winmo (2)

updated for devicemanager ^M mixup.

Comment 78

[Aki Sasaki (not active)]

Running on staging with Joel's latest patch.
After updating to use winmo.config rather than mobile.config:

DEBUG: created profile
FIRE PROC: '/tests/fennec/fennec.exe --environ:NO_EM_RESTART=1 -profile /tests/talos/profile http://bm-foopy.build.mozilla.org/getInfo.html > /tests/talos/process.txt'
reconnecting socket
	Screen width/height:480/640


	colorDepth:16


	Browser inner width/height: 800/982


	Browser outer width/height: 480/588
DEBUG: initialized fennec.exe
DEBUG: command line: /tests/fennec/fennec.exe --environ:NO_EM_RESTART=1 -profile /tests/talos/profile http://bm-foopy.build.mozilla.org/startup_test/startup_test.html?begin=
reconnecting socket
Traceback (most recent call last):
  File "bcontroller.py", line 214, in <module>
    sys.exit(main())
  File "bcontroller.py", line 208, in main
    bcontroller.run()
  File "bcontroller.py", line 142, in run
    os.chmod(self.log, 0777)
OSError: [Errno 2] No such file or directory: 'browser_output.txt'

Comment 79

[Aki Sasaki (not active)]

Am I running an outdated SUT agent?

Comment 80

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

Comment on attachment 427259 [details] [diff] [review]
last patch in series for ts on winmo (2)

Uh, doesn't apply?

atching file devicemanager.py
Hunk #1 FAILED at 132.
Hunk #2 FAILED at 335.
Hunk #3 FAILED at 598.
3 out of 3 hunks FAILED -- saving rejects to file devicemanager.py.rej

Comment 81

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

Attachment: last patch in series for ts on winmo (3)

updated patch to avoid hunk failures.

Comment 82

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

Quick first comment:

-            if option in ("--deviceRoot",):
+            if option in ("--deviceRoot"):
                 deviceRoot = value
-            if option in ("--fast",):
+            if option in ("--fast"):
                 fast = True
-            if option in ("--symbolsPath",):
+            if option in ("--symbolsPath"):
                 symbolsPath = value

Those commas have to stay for this to work the way you want it to.  The commas were checked in as a bustage fix last time this landed, are you sure that you are generating your patch from latest code?

Comment 83

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

Latest news:

Traceback (most recent call last):
  File "run_tests.py", line 462, in ?
    test_file(arg)
  File "run_tests.py", line 424, in test_file
    browser_dump, counter_dump, print_format = mytest.runTest(browser_config, test)
  File "/Users/mozqa/talos-slave/talos-data/talos/ttest.py", line 206, in runTest
    browser_config['dirs'][dir])
  File "/Users/mozqa/talos-slave/talos-data/talos/ffsetup.py", line 164, in InstallInBrowser
    self.ffprocess.copyFile(fromfile, todir)
  File "/Users/mozqa/talos-slave/talos-data/talos/ffprocess_mac.py", line 216, in copyFile
    if not os.path.isfile(os.path.join(todir, os.path.basename(fromfile))):
NameError: global name 'todir' is not defined

On mac.

Comment 84

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

Attachment: last patch in series for ts on winmo (4)

realizing now that the devicemanager.py which is already checked in has \r\n, I have cleaned up this which essentially caused the whole devicemanager.py file to be readded.  There is one small change in devicemanager.py and that is the addition of getCurrentTime() at the bottom.

This applies cleanly to a fresh cvs checkout on my linux box.  In regards to the other errors found on mac last week, those were specific to mac and win32.  These errors were a typo in a variable which was not a problem for regular linux or remote winmo.

Comment 85

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

You won't like this...

patching file devicemanager.py
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file devicemanager.py.rej

Comment 86

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

Oh wait, my fault.  I had local changes.  Sorry.

Comment 87

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

This version of the patch still removes those commas mentioned in comment #82.

Comment 88

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

Real bad news:

Traceback (most recent call last):
  File "bcontroller.py", line 214, in ?
    sys.exit(main())
  File "bcontroller.py", line 207, in main
    bcontroller = BrowserController(command, mod, name, child_process, timeout, log, host, port, deviceRoot)
  File "bcontroller.py", line 130, in __init__
    self.ffprocess = ffprocess
UnboundLocalError: local variable 'ffprocess' referenced before assignment

Comment 89

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

Attachment: last patch in series for ts on winmo (5)

Alright.  Sorry for the delay.  It seems this \r\n thing keeps bugging me, so I spent more time trying to ensure my patch would apply cleanly.

As for the error reported above, I had accidentally removed a global variable reference inside of a class.  Most of these problems are surrounding me trying to fix this \r\n thing and making multiple copies of the code base, etc...

Also corrected in this patch are the ,'s that I removed.  I really do not understand why those are needed, but I will leave them in the code base.

This patch I tested on winmo (ts) and linux desktop (local and remote ts, tsvg).  Did a quick test on win32 with no hiccup.

Comment 90

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

The commas are important because python can treat strings as lists of characters.  Thus a list like ('hello') is actually the list ('h', 'e', 'l', 'l', 'o') - so you end up doing character matching.  Adding the comma ('hello',) tells python that it is a list of strings of length 1 and then does the matching correctly.

Comment 91

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

Patch #5 now on staging.

Comment 92

[Aki Sasaki (not active)]

Attachment: touch pro breakage in Ts

Running with patch 428801 on CVS trunk, with a test-agent.exe from 2/28/10.

  File "/tools/python-2.5.1/lib/python2.5/tempfile.py", line 328, in mkdtemp
    _os.mkdir(file, 0700)
OSError: [Errno 31] Too many links: '/tmp/tmp3O_0Ky'

Comment 93

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

odd, this is in code that I didn't really touch as it is in the normal ffsetup profilecreation.

Here is a reference to a bug that is similar with some os level adjustments to resolve the problem:
http://tracker.moodle.org/browse/MDLSITE-439

Would it be safe to assume the normal Talos boxes are rebooted every run and this box is not rebooted often?

Comment 94

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

Alice, how did this work on staging over the weekend?

Comment 95

[Aki Sasaki (not active)]

(In reply to comment #93)
> Here is a reference to a bug that is similar with some os level adjustments to
> resolve the problem:
> http://tracker.moodle.org/browse/MDLSITE-439
> 
> Would it be safe to assume the normal Talos boxes are rebooted every run and
> this box is not rebooted often?

Whoa!

...Yes, phones are rebooted every run. Since rebooting the linux box would bring down every phone, we'll never want to do that.

Are you the one creating all the /tmp/tmp* dirs ?  Could you please update the code to clean those up?  There are 27783 of them on bm-foopy.

Comment 96

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

there are 2 mkdtemp calls, 1 which has been in talos forever, and 1 new one that I added.  I will come up with a new patch that will remove the tmpdir flood.

Comment 97

[Aki Sasaki (not active)]

Comment on attachment 429623 [details]
touch pro breakage in Ts

Ok, now we're running and I'm getting

FAIL: Failed to send data 5 times... quitting
RETURN:send failed, graph server says:
RETURN:to determine average from 'test_run_values' for  1140143 - local variable 'values' referenced before assignment
RETURN:  File "/var/www/html/graphs2/server/pyfomatic/collect.py", line 257, in handleRequest
RETURN:    average = valuesReader(databaseCursor, databaseModule, inputStream, metadata)
RETURN:  File "/var/www/html/graphs2/server/pyfomatic/collect.py", line 215, in valuesReader
RETURN:    raise DatabaseException("to determine average from 'test_run_values' for  %s - %s" % (metadata.test_run_id, str(x)))

Comment 98

[Aki Sasaki (not active)]

Looks like it's running only one cycle when the config file says 10. Joel's looking into it.

Comment 99

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

Attachment: last patch in series for ts on winmo (6)

this is the final patch in the series.  This fixes two things that aki found:
1) removes mkdtemp directories that are created
2) fixes the cycles looping as this was accidentally changed

with the fix in #2, I would request that this patch be run in staging again.

Comment 100

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

Patch #6 now on talos staging.

Comment 101

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Adding dep.bugs while talking with bobmoss

Comment 102

[Aki Sasaki (not active)]

Woo woo!
WinMo talos just went green using final6.diff.

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1267659913.1267662908.23784.gz&fulltext=1

ts: 35216.22  <-- is this 35 seconds? If so, it's in the ballpark of correct.
ts_shutdown: 123274.67  <-- uh ?

Trying another run because I'm crazy like that.

Comment 103

[Aki Sasaki (not active)]

2nd run died during PerfConfigurator with a remoteapp.ini error message.

3rd and 4th runs on two separate Touch Pros:

ts: 37423.56
ts_shutdown: 569576.0
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1267664069.1267667094.3969.gz

ts: 36478.67
ts_shutdown: 123742.11
http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTest/1267664593.1267667633.5803.gz

These are all measuring the same build.

I think that's the least amount of noise we've ever seen on WinMo Ts (and no negative numbers!)  So things are definitely looking much better.  Ts_shutdown is definitely noisy but it's noisy on some of the other platforms too.

Comment 104

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

Comment on attachment 430021 [details] [diff] [review]
last patch in series for ts on winmo (6)

+user_pref("capability.principal.codebase.p2.granted", "UniversalPreferencesWrite UniversalXPConnect UniversalPreferencesRead");
+user_pref("capability.principal.codebase.p2.id", "http://%(server)s");
+user_pref("capability.principal.codebase.p2.subjectName", "");  

These lines should be in the base_profiles (see base_profile, place_generate_max, etc) prefs.js.  We already have lines like it in the prefs.js for each of these profiles, so we should keep all of this info together.

I'd devicemanager.py is under-commented.  I'd like to see some explanation associated with each defined procedure so that it will be easier to update later.

Comment 105

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

Attachment: [checked in]last patch in series for ts on winmo (7)

updating patch for review feedback:
1) added a slew of comments in devicemanager.py
2) moved the user_pref stuff to devicemanager.py instead of ffsetup.  

There was some discussion around creating a separate winmo profile that would have this information in it.  I opted to avoid that and when we encounter another profile change for winmo or remote devices, I would push for a unique profile then.  

I think this is a bit cleaner and pushes the code to places that is less confusing for the end user.  I verified on a fresh checkout that this applies (even devicemanager.py) with this uploaded patch and runs my basic tests.

Thanks for the quick turnaround and testing on this patch.

Comment 106

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

Patch #7 not onw talos staging.

Comment 107

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

Comment on attachment 430422 [details] [diff] [review]
[checked in]last patch in series for ts on winmo (7)

Looking green on stage, will land this in tomorrow morning's downtime.

Comment 108

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

Comment on attachment 430422 [details] [diff] [review]
[checked in]last patch in series for ts on winmo (7)

Checking in PerfConfigurator.py;
/cvsroot/mozilla/testing/performance/talos/PerfConfigurator.py,v  <--  PerfConfigurator.py
new revision: 1.17; previous revision: 1.16
done
Checking in bcontroller.py;
/cvsroot/mozilla/testing/performance/talos/bcontroller.py,v  <--  bcontroller.py
new revision: 1.13; previous revision: 1.12
done
Checking in devicemanager.py;
/cvsroot/mozilla/testing/performance/talos/devicemanager.py,v  <--  devicemanager.py
new revision: 1.2; previous revision: 1.1
done
Checking in ffprocess.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess.py,v  <--  ffprocess.py
new revision: 1.18; previous revision: 1.17
done
Checking in ffprocess_linux.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_linux.py,v  <--  ffprocess_linux.py
new revision: 1.19; previous revision: 1.18
done
Checking in ffprocess_mac.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_mac.py,v  <--  ffprocess_mac.py
new revision: 1.19; previous revision: 1.18
done
Checking in ffprocess_win32.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_win32.py,v  <--  ffprocess_win32.py
new revision: 1.12; previous revision: 1.11
done
Checking in ffprocess_winmo.py;
/cvsroot/mozilla/testing/performance/talos/ffprocess_winmo.py,v  <--  ffprocess_winmo.py
new revision: 1.2; previous revision: 1.1
done
Checking in ffsetup.py;
/cvsroot/mozilla/testing/performance/talos/ffsetup.py,v  <--  ffsetup.py
new revision: 1.17; previous revision: 1.16
done
Checking in run_tests.py;
/cvsroot/mozilla/testing/performance/talos/run_tests.py,v  <--  run_tests.py
new revision: 1.61; previous revision: 1.60
done
Checking in ttest.py;
/cvsroot/mozilla/testing/performance/talos/ttest.py,v  <--  ttest.py
new revision: 1.43; previous revision: 1.42
done
Checking in winmo.config;
/cvsroot/mozilla/testing/performance/talos/winmo.config,v  <--  winmo.config
new revision: 1.2; previous revision: 1.1
done

Comment 109

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

with all these patches landed, we can successfully run talos ts on winmo, marking as resolved!