829211 - Use mozpool and mozharness for the Android pandas

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect, P3)

Description

[Armen [:armenzg]]

This will improve the reliability of our Android infra.

I can to hook it up to clientproxy.py or add mozpool to our factory.py and not touch clientproxy.py at all.

I will have to package mozpool and submit it to pypi before I can get to this.

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

The mozpool package is bug 827545.

Comment 3

[Kim Moir [:kmoir] ET]

Attachment: WIP patch

WIP patch for mozpool/mozharness, still lots of work to do including branch specific tests, and removing the old bits mobile_config.py etc. The tests get invoked but fail due to permission errors on the device, debugging that now.

Comment 4

[Kim Moir [:kmoir] ET]

Attachment: WIP patch #2

Comment 5

[Kim Moir [:kmoir] ET]

Attachment: patch

Here are my patches so far to run android panda tests via mozpool and mozharness. These are just the patches to run them, not to remove the existing configs that run without mozharness.  It would be great to have feedback on any areas for improvement.  The scripts to run talos tests aren't complete yet, these are just for mochitests, crashtests, jsreftests, robocop etc.

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 744783 [details] [diff] [review]
patch

From pyflakes:

scripts/android_panda.py:19: 'DEBUG' imported but unused
scripts/android_panda.py:20: 'BaseErrorList' imported but unused
scripts/android_panda.py:175: local variable 'c' is assigned to but never used
scripts/android_panda.py:176: local variable 'dirs' is assigned to but never used
scripts/android_panda.py:190: local variable 'suite_name' is assigned to but never used

>+    "mochitest_options": [
>+        "--xre-path=/tools/xre",
>+        "--utility-path=../hostutils/bin",
>+        "--certificate-path=certs",
>+        "--app=org.mozilla.fennec", "--console-level=INFO",

Hardcoding org.mozilla.fennec might be ok for now, but will bite us when we start riding the trains.
We should probably "--app=%(app_name)s" and figure out the app name from the apk.
We're currently doing that in buildbot with these two steps:
http://hg.mozilla.org/build/buildbotcustom/file/13967138af38/process/factory.py#l5323
http://hg.mozilla.org/build/buildbotcustom/file/13967138af38/process/factory.py#l5330

>+     "jsreftest_options": [
>+        "--xre-path=/tools/xre",
>+        "--utility-path=../hostutils/bin",
>+        "--app=org.mozilla.fennec",
>+        "--enable-privilege", "--ignore-window-size", "--bootstrap", 

Nit: trailing whitespace.

>+     "all_reftest_suites": {
>+        "reftest-1": ["total-chunks=4", "--this-chunk=1"],
>+        "reftest-2": ["total-chunks=4", "--this-chunk=2"],
>+        "reftest-3": ["total-chunks=4", "--this-chunk=3"],
>+        "reftest-4": ["total-chunks=4", "--this-chunk=4"],
>+     },

One step we're probably going to want to take with this config, as well as the desktop configs, is to get these test suite definitions in-tree somewhere.
This is total scope creep and not something you need to take on with this pass, just noting this since we may change config settings at some point, and we may want those changes to ride the trains.  Plus easier testing of config changes on Try.

>+    #added not default
>+    "find_links": ["http://puppetagain.pub.build.mozilla.org/data/python/packages"],
>+    "device_package_name": "org.mozilla.fennec", 

As above, we should figure this out programmatically.
And trailing whitespace :)

>+    "test_suite": "mochitest",   # mochitest, reftest, crashtest, jsreftest, xpcshell

Not sure what test_suite is for?

>+        'clobber',
>+        'read-buildbot-config',
>+        'create-virtualenv',
>+        'download-and-extract',
>+        'request-device',
>+        'run-test',
>+        'close-request',

Do we have a device reboot at some point through mozpool or something?

>+    mozbase_dir = os.path.join('tests', 'mozbase')

I'm not sure if you use this?
If you did, overriding query_abs_dirs() might be a good way to specify this.

(I'm not sure if you need all of these self.variables.  Cleaning up would be good, but not a blocker.)

>+        self.mozpool_assignee = self.config.get('mozpool_assignee', getpass.getuser())
>+        print "mozpool_assignee: "
>+        print self.mozpool_assignee

Debug statements?
You can self.debug() or self.info() these if you want to land them.  (self.debug() statements won't show up unless you --log-level debug)

>+        self.request_url = None
>+        self.mozpool_device = self.config.get("mozpool_device")
>+        self.installer_url = self.config.get("installer_url")
>+        self.app = self.config.get("device_package_name")
>+        self.test_url = self.config.get("test_url")
>+        self.symbols_url = self.config.get('symbols_url')
>+     

Trailing whitespace :)

>+    def _run_category_suites(self, suite_category, preflight_run_method=None):
>+        """run suite(s) to a specific category"""
>+
>+        env = self.query_env()
>+        env["DM_TRANS"] = "sut"
>+        env["TEST_DEVICE"] = self.mozpool_device
>+        self.info("Running tests...")

You can also do

env = self.query_env(partial_env={'DM_TRANS': "sut", 'TEST_DEVICE': self.mozpool_device})

>+                env = self.query_env()
>+                code = self.run_command(cmd, env=env, output_parser=test_summary_parser)

I don't think you have to re-set env, since you set it above.

>+                if code == 0:
>+                    tbpl_status = TBPL_SUCCESS
>+                elif code == 10: # XXX assuming this code is the right one
>+                    tbpl_status = TBPL_WARNING
>+                else:
>+                    level = ERROR
>+                    tbpl_status = TBPL_FAILURE

This logic looks like the b2g_panda / marionette logic.
Are you getting the right values in buildbot?  I think you'll have to do something more similar to desktop_unittest.py, where the DesktopUnittestParser parses the output and determines how many tests have failed.  Afaik the scripts for these suites don't have nice exit codes that you can go by.

>+    def run_test(self):
>+       # do we need to set the device time?
>+       # command doesn't work anyways
>+       # self._sut_prep_steps()
>+
>+        env = self.query_env()
>+        env["DM_TRANS"] = "sut"
>+        env["TEST_DEVICE"] = self.mozpool_device
>+        print "self.mozpool_device in run_test: "
>+        print self.mozpool_device 

self.info(), trailing whitespace.
I'll skip noting the other prints in this script.

>+        #? error_list = self.error_list
>+        #? error_list.extend(BaseErrorList)

Error lists are ways to detect errors through either substrings or regexes in the log.
desktop_unittest.py has this process crash detection:
http://hg.mozilla.org/build/mozharness/file/fb5f17f17772/scripts/desktop_unittest.py#l334
BaseErrorList is this simple "command not found":
http://hg.mozilla.org/build/mozharness/file/fb5f17f17772/mozharness/base/errors.py#l32
I can clarify further if you'd like.


This is a tricky script to get right, and you're doing a good job so far as far as I can tell.

Comment 7

[Kim Moir [:kmoir] ET]

Attachment: updated patch with aki's suggestions

Aki, I incorporated all the suggestions you made earlier.  Yes, when you request the device via mozpool it takes care of rebooting the device before it's put into a ready state, and mozpool also does a sut verify.

Comment 8

[Aki Sasaki (not active)]

Comment on attachment 748251 [details] [diff] [review]
updated patch with aki's suggestions

I think the interdiff looks good.
Have you run this yet?

>+                test_summary_parser = DesktopUnittestOutputParser(suite,
>+                                                     config=self.config,
>+                                                     error_list=error_list,
>+                                                     log_obj=self.log_obj)
>+                return_code = self.run_command(cmd, env=env, output_parser=test_summary_parser)
>+                tbpl_status, log_level = test_summary_parser.evaluate_parser(return_code)
>+                if return_code == 0:
>+                    tbpl_status = TBPL_SUCCESS
>+                elif return_code == 10: # XXX assuming this return_code is the right one
>+                    tbpl_status = TBPL_WARNING
>+                else:
>+                    level = ERROR
>+                    tbpl_status = TBPL_FAILURE

I think you want to get rid of the if/elif/else part here, since you already got tbpl_status from test_summary_parser.evaluate_parser().
http://hg.mozilla.org/build/mozharness/file/4a0d5bac9ccd/scripts/desktop_unittest.py#l359

>+    def _query_abs_base_cmd(self, suite_category):
>+

Nit: trailing whitespace.
(sorry, pet peeve)

Comment 9

[Kim Moir [:kmoir] ET]

Attachment: buildbotcustom patch

switching the order if/elif blocks so mobile tests with mozharness use ScriptFactory

Comment 10

[Kim Moir [:kmoir] ET]

Attachment: updated mozharness patches

I have tests running in staging with this latest patch. They are intermittently green then orange.  I don't know of if there is a software issue or the rack of pandas I'm testing with is wonky, I'm investigating.  Anyways, here they are for more feedback :-)  Perhaps install_app should be broken out into mozilla/testing/device.py but that file seems to be a bit out of date given that we now use mozpool, so I just left it in my script for now.

Comment 11

[Aki Sasaki (not active)]

Comment on attachment 758584 [details] [diff] [review]
updated mozharness patches

Pyflakes says:

scripts/android_panda.py:19: 'TBPL_FAILURE' imported but unused
scripts/android_panda.py:19: 'TBPL_WARNING' imported but unused
scripts/android_panda.py:23: 'ScriptMixin' imported but unused
scripts/android_panda.py:284: local variable 'status' is assigned to but never used

(This is only important for when you're done; feel free to leave these if you're planning on using them soon.)

I get that list by running ./unit.sh.

The interdiff looks good, though I admittedly skimmed:
https://bugzilla.mozilla.org/attachment.cgi?oldid=748251&action=interdiff&newid=758584&headers=1

(In reply to Kim Moir [:kmoir] from comment #10)
> I have tests running in staging with this latest patch. They are
> intermittently green then orange.  I don't know of if there is a software
> issue or the rack of pandas I'm testing with is wonky, I'm investigating.

Ok.  Sounded like an SUT version / script mismatch.

> Anyways, here they are for more feedback :-)  Perhaps install_app should be
> broken out into mozilla/testing/device.py but that file seems to be a bit
> out of date given that we now use mozpool, so I just left it in my script
> for now.

I'm fine keeping it in the script until something else needs it.

Comment 12

[Armen [:armenzg]]

Live in production.

Comment 13

[Kim Moir [:kmoir] ET]

Attachment: mozharness patches

Latest changes: add mochitest-gl tests and do a verify after requesting the device to ensure sutagent is up to date.

Comment 14

[Aki Sasaki (not active)]

Comment on attachment 759852 [details] [diff] [review]
mozharness patches

* We're going to need the mozharness changes checked into build/mozharness proper for these changes to work.
* I wasn't able to apply these changes cleanly to buildbot-configs default.
* Do these configs work for you in staging?

I applaud the various debugging approaches I see in this patch, as long as they work for you in staging.
I think to get a reviewable production patch, this should have the relevant staging changes/commented-out code removed.

(One thing that's helped me in this regard is hg mq: that allows you to |hg qpop| your staging patch, edit your production patch and |hg qrefresh|, then |hg qpush| to get your staging patch re-applied.  That separates the debugging + staging specific changes from the patch you need to apply to production.

http://mercurial.selenic.com/wiki/MqExtension
)

Let me know if you need to discuss anything in the review below?

>diff -r 71454b7b456f Makefile.master
>--- a/Makefile.master	Wed May 22 17:45:29 2013 -0700
>+++ b/Makefile.master	Fri Jun 07 10:55:58 2013 -0700
>@@ -22,3 +22,6 @@
> 	(python buildbot-configs/update-master-json.py $(PRODUCTION_MASTERS) master/master_config.json)
> version:
> 	$(BUILDBOT) --version
>+reset-db:
>+	rm -f master/state.sqlite
>+	sh -c "cd master && $(BUILDBOT) upgrade-master"

I've always guessed this wasn't in the makefile by default, since we're aiming it at production and updating for staging.

If it's harmless for production, I'm ok keeping it.

>diff -r 71454b7b456f mozilla-tests/mobile_config.py
>--- a/mozilla-tests/mobile_config.py	Wed May 22 17:45:29 2013 -0700
>+++ b/mozilla-tests/mobile_config.py	Fri Jun 07 10:55:58 2013 -0700
>@@ -11,6 +11,7 @@
> import localconfig
> reload(localconfig)
> from localconfig import SLAVES, TRY_SLAVES, GLOBAL_VARS, GRAPH_CONFIG
>+from config import MOZHARNESS_REBOOT_CMD
> 
> TALOS_REMOTE_FENNEC_OPTS = {'productName': 'fennec',
>                             'remoteTests': True,
>@@ -64,7 +65,13 @@
> PLATFORMS['android']['tegra_android'] = {'name': "Android Tegra 250"}
> PLATFORMS['android']['panda_android'] = {'name': "Android 4.0 Panda"}
> PLATFORMS['android']['stage_product'] = 'mobile'
>-PLATFORMS['android']['mozharness_config'] = {}
>+#PLATFORMS['android']['mozharness_python'] = '/tools/buildbot/bin/python'
>+PLATFORMS['android']['mozharness_config'] = {
>+    'mozharness_python': '/tools/buildbot/bin/python',
>+    'hg_bin': 'hg',
>+    'reboot_command': ['/tools/buildbot/bin/python'] + MOZHARNESS_REBOOT_CMD,
>+    #'extra_args': ['--config-file', 'android/android_panda_releng.py']
>+}

I'm ok with these things commented out in staging (this extra_args, the chunk options, etc.), but ideally these will land in production cleaned up.

>+ANDROID_MOZHARNESS_UNITTEST_DICT = {
>+    'opt_unittest_suites': [
>+         ('mochitest-1', 
>+            {'suite': 'mochitest-plain',
>+             'use_mozharness': True,
>+             'script_path': 'scripts/android_panda.py',
>+             'extra_args': ['--cfg', 'android/android_panda_releng.py', '--mochitest-suite', 'mochitest-1'],
>+             #'testManifest': 'android.json',
>+             #'totalChunks': 8,
>+             #'thisChunk': 1,

(the chunk options I was referring to above)

>+for suite in ANDROID_MOZHARNESS_UNITTEST_DICT['opt_unittest_suites']:
>+    if suite[0].startswith('reftest'):
>+        continue
>+    if suite[0].startswith('robocop'):
>+        continue
>+    ANDROID_MOZHARNESS_PLAIN_UNITTEST_DICT['opt_unittest_suites'].append(suite)

This loop, and the ones below, work.  However, we've moved to an additive solution rather than subtractive, because it's more explicit.

(I realize this is a large change.  If you feel that's too large for now, we can file a followup bug to make that change.)

From mozilla-tests/config.py:

suite definitions: http://hg.mozilla.org/build/buildbot-configs/file/2d8f420e1532/mozilla-tests/config.py#l291

defaults, using "additive" definitions: http://hg.mozilla.org/build/buildbot-configs/file/2d8f420e1532/mozilla-tests/config.py#l403

Adding additional non-default suites: http://hg.mozilla.org/build/buildbot-configs/file/2d8f420e1532/mozilla-tests/config.py#l421

Using a subset of default suites, still using "additive" definitions: http://hg.mozilla.org/build/buildbot-configs/file/2d8f420e1532/mozilla-tests/config.py#l646

I realize this is a big change, but will hopefully mean we're more easily able to see what tests are or aren't enabled for a specific branch/platform without having to read all the special loops.

We're *still* not there in config.py and b2g_config.py, but we're definitely reducing the number of special cases when we can.  If you're able to, then mobile_config.py will start to look cleaner as well.

>+for suite in ANDROID_MOZHARNESS_PLAIN_UNITTEST_DICT['opt_unittest_suites']:
>     if suite[0].startswith('reftest') or suite[0].startswith('plain-reftest'):
>         continue
>     if suite[0].startswith('xpcshell'):
>         continue
>-    ANDROID_PANDA_UNITTEST_DICT['opt_unittest_suites'].append(suite)
>+    ANDROID_MOZHARNESS_PANDA_UNITTEST_DICT['opt_unittest_suites'].append(suite)

For things like this we would add the appropriate non-reftest non-xpcshell tests.

>+#print "ANDROID_MOZHARNESS_PANDA_UNITTEST_DICT" + str(ANDROID_MOZHARNESS_PANDA_UNITTEST_DICT)

Staging cruft :)

>+#print "ANDROID_PANDA_UNITTEST_DICT : " 
>+#print ANDROID_PANDA_UNITTEST_DICT

Here too.

>+       # 'panda_android': deepcopy(ANDROID_PANDA_UNITTEST_DICT),
>+        'panda_android': deepcopy(ANDROID_MOZHARNESS_PANDA_UNITTEST_DICT),

Here too.

>@@ -513,6 +827,9 @@
> BRANCHES['mozilla-central']['pgo_strategy'] = 'periodic'
> BRANCHES['mozilla-central']['pgo_platforms'] = []
> BRANCHES['mozilla-central']['platforms']['android']['enable_debug_unittests'] = True
>+BRANCHES['mozilla-central']['mozharness_repo'] = "http://hg.mozilla.org/users/kmoir_mozilla.com/mozharness"
>+BRANCHES['mozilla-central']['mozharness_tag'] = "default"

Unless you're planning on going live with your user repo, this needs to be changed for the reviewed patch.

>+#print "BRANCHES m-c"
>+#print  BRANCHES['mozilla-central']

Here too.

>diff -r 71454b7b456f mozilla-tests/staging_config.py
>--- a/mozilla-tests/staging_config.py	Wed May 22 17:45:29 2013 -0700
>+++ b/mozilla-tests/staging_config.py	Fri Jun 07 10:55:58 2013 -0700
>@@ -7,6 +7,11 @@
>         [('tegra-%03i' % x, {'http_port': '30%03i' % x, 'ssl_port': '31%03i' % x}) for x in range(10,30)]
>     ),
>     'fedora': dict(),
>+    'panda_android': dict(
>+        [('panda-%04i' % x, {'http_port': '30%03i' % x, 'ssl_port': '31%03i' % x}) \
>+            #for x in range(61,69) + range(75,82)]
>+            for x in range(61,82)]
>+    ),
> }
> 
> STAGING_SLAVES['tegra_android-armv6'] = STAGING_SLAVES['tegra_android']
>@@ -31,8 +36,10 @@
>     'tinderbox_tree': 'MozillaTest',
>     'mobile_tinderbox_tree': 'MobileTest',
>     'build_tools_repo_path': 'build/tools',
>-    'mozharness_repo': 'http://hg.mozilla.org/build/mozharness',
>-    'mozharness_tag': 'production',
>+   # 'mozharness_repo': 'http://hg.mozilla.org/build/mozharness',
>+    'mozharness_repo': 'http://hg.mozilla.org/users/kmoir_mozilla.com/mozharness',
>+    #'mozharness_tag': 'production',
>+    'mozharness_tag': 'default',
>     'stage_server': 'dev-stage01.srv.releng.scl3.mozilla.com',
>     'stage_username': 'ffxbld',
>     'stage_ssh_key': 'ffxbld_dsa',
>diff -r 71454b7b456f mozilla-tests/tests_localconfig.py
>--- a/mozilla-tests/tests_localconfig.py	Wed May 22 17:45:29 2013 -0700
>+++ b/mozilla-tests/tests_localconfig.py	Fri Jun 07 10:55:58 2013 -0700
>@@ -26,7 +26,8 @@
> import b2g_config
> import mobile_config
> # Do everything!
>-ACTIVE_BRANCHES = BRANCHES.keys()
>+#ACTIVE_BRANCHES = BRANCHES.keys()
>+ACTIVE_BRANCHES = ['mozilla-central'] 
> ACTIVE_THUNDERBIRD_BRANCHES = thunderbird_config.BRANCHES.keys()
> ACTIVE_B2G_BRANCHES = b2g_config.BRANCHES.keys()
> ACTIVE_MOBILE_BRANCHES = mobile_config.BRANCHES.keys()
>@@ -48,4 +49,4 @@
>     ACTIVE_MOBILE_PLATFORMS = dict((k, None) for k in MOBILE_PLATFORMS.keys())
> ACTIVE_PROJECTS = PROJECTS.keys()
> 
>-QUEUEDIR = master_config.get("queuedir", "/dev/shm/queue")
>+QUEUEDIR = master_config.get("queuedir", "/dev/shm/kmoir/queue")

These are staging config changes that should be removed before any sort of push to prod.

Comment 15

[Kim Moir [:kmoir] ET]

Attachment: mozharness patch

The patch I added on Friday was the wrong one.  It was a work-in-progress buildbot-configs patch that I copied to my laptop.  I really meant to attach the updated mozharness scripts that I just attached.  I apologize for the confusion.  Obviously I didn't have enough caffeine on Friday :-)

Comment 16

[Aki Sasaki (not active)]

Comment on attachment 760266 [details] [diff] [review]
mozharness patch

Overall the previous patch looked ok, and the interdiff looks mostly good.
https://bugzilla.mozilla.org/attachment.cgi?oldid=758584&action=interdiff&newid=760266&headers=1

>+from verify import verifyDevice

What's verify?
Are we introducing a new external dependency for mozharness, or is this a typo?

If the latter, that's fine; fix and move on.
If the former, that may be problematic.  We may need to do something like SUTDeviceHandler does here:
http://hg.mozilla.org/build/mozharness/file/19e86b400db1/mozharness/mozilla/testing/device.py#l451
since we want to allow for launching mozharness with a minimum of external dependencies.

>+                self.info("cmd: " + str(cmd))
>+                self.info("suite: " + str(suite))
>+                self.info("self.abs_dirs['abs_work_dir'] " + self.abs_dirs['abs_work_dir'])

Are these still useful?  If so, let's keep em.  If they were just for debugging purposes, we can clean up.

>+                return_code = self.run_command(cmd, dirs['abs_test_install_dir'], env=env, output_parser=test_summary_parser)
>+                self.info("kim return code: " + str(return_code))
>+
>+                tbpl_status, log_level = test_summary_parser.evaluate_parser(return_code)
>+                self.info("kim tbpl_status: " + str(tbpl_status))

Leftover debugging statements?
Will you need these in production?

r- on the verify import; if it's an easily fixed typo I might + if it's fixed on checkin.

Comment 17

[Justin Wood (:Callek)]

(In reply to Aki Sasaki [:aki] from comment #16)
> Comment on attachment 760266 [details] [diff] [review]
> mozharness patch
> 
> Overall the previous patch looked ok, and the interdiff looks mostly good.
> https://bugzilla.mozilla.org/attachment.
> cgi?oldid=758584&action=interdiff&newid=760266&headers=1
> 
> >+from verify import verifyDevice
> 
> What's verify?

I presume this is: http://mxr.mozilla.org/build/source/tools/sut_tools/verify.py

> Are we introducing a new external dependency for mozharness, or is this a
> typo?

If possible I'd prefer to either call /builds/sut_tools/verify.py directly, or to operate on a clone of tools done as part of mozharness which then uses verify.py directly; (the latter is probably more likely mozharness-like). trying to import the dir structure for this to mozharness is a bad idea to me, as is not running verify at all.

Comment 18

[Kim Moir [:kmoir] ET]

The verify is needed here. 

def request_device(self):
         self.retrieve_android_device(b2gbase="")
         if verifyDevice(self.mozpool_device, checksut=True, doCheckStalled=False, watcherINI=True) == False:
               self.error("failed to run verify on %s" % (self.mozpool_device))
         else:
               self.info("Successfully verified the device")


Mozpool doesn't verify the device thus if the image doesn't have the latest sutagent, it doesn't get updated.  This is why I added this to mozharness scripts.  In any case, I'll look at making the changes you mentioned :-)

Comment 19

[Kim Moir [:kmoir] ET]

Attachment: updated mozharness patch

updated to address latest comments

Comment 20

[Aki Sasaki (not active)]

Comment on attachment 761417 [details] [diff] [review]
updated mozharness patch

r=me if this works.

If this doesn't work, instead of importing, it may be easiest to just call verify.py via self.run_command(), assuming it's callable as a script and not just a library.

Comment 21

[Kim Moir [:kmoir] ET]

Comment on attachment 761417 [details] [diff] [review]
updated mozharness patch

Yes, it works, I tested it in staging.

If you call verify.py as a script, as it's implemented today you can't update the sutagent.  That's why I imported it.

thanks Aki!

Comment 22

[Armen [:armenzg]]

Merged change to production mozharness.

Comment 23

[Kim Moir [:kmoir] ET]

Attachment: patch to fix tbpl parsing

This patch fixes issues with mochitest-gl,  robocop and jsreftests being orange in staging.  Now all green :-)

Comment 24

[Kim Moir [:kmoir] ET]

Attachment: patch so we can set script timeout via mozharness

Comment 25

[Aki Sasaki (not active)]

Comment on attachment 762901 [details] [diff] [review]
patch to fix tbpl parsing

Is the commandline order important, or just neater?
If important, it might be nice to have a comment for posterity.


 TinderBoxPrintRe = {
     "mochitest_summary": {
         'regex': re.compile(r'''(\d+ INFO (Passed|Failed|Todo):\ +(\d+)|\t(Passed|Failed|Todo): (\d+))'''),
         'pass_group': "Passed",
         'fail_group': "Failed",
         'known_fail_group': "Todo",
-    },
+    },   

Looks like you added some trailing whitespace here.

Comment 26

[Kim Moir [:kmoir] ET]

Comment on attachment 762901 [details] [diff] [review]
patch to fix tbpl parsing

The command line changes were just to make them look cleaner.

Comment 27

[Kim Moir [:kmoir] ET]

Attachment: fix spacing in mozpool.py for retrieve_android_device method

I realized the spacing in mozpool.py was incorrect this morning when I tried to run tests from the real mozharness repo instead of my user repo.  My user repo was correct, my patch was not so the retrieve_android_device method was not available.

Comment 28

[Chris Cooper [:coop] (he/him)]

In production.

Comment 29

[Kim Moir [:kmoir] ET]

Attachment: buildbot-configs patch

Aki: I was discussing how to implement this with you at the work week and you suggested sequestering the mozpool controlled and non-mozpool managed pandas into different pools.  (To get mochitests etc into production before talos). However, I tested this today on my dev-master and it's not a problem to run the non-talos tests with mozpool/mozharness and the talos tests with non-mozpool/non-mozharness. It works fine since buildbot is still the one assigning slaves for tests, mozpool just verifies that they are in a usable state. Tests are all green.

Comment 30

[Aki Sasaki (not active)]

Comment on attachment 763815 [details] [diff] [review]
buildbot-configs patch

Cool!
This has the upside of letting us treat the pandas as a single pool.
The downside is that a bug in the automation on either side can affect tests on both sides (mozharness/mozpool or sut_tools).  As long as we're aware of this and keep an eye out for it, we can go this route.

Comment 31

[Kim Moir [:kmoir] ET]

Comment on attachment 763815 [details] [diff] [review]
buildbot-configs patch

Okay thanks for the feedback Aki.  This situation probably won't last long as the patches for mozharness android talos are well underway.

Comment 32

[Aki Sasaki (not active)]

The mozharness portions are in production.

Comment 33

[Justin Wood (:Callek)]

Comment on attachment 744783 [details] [diff] [review] [diff] [review]
patch

---------AUTOMATIC COMMENT---------
--  filter pep8-callek-june-2013 --
---------AUTOMATIC COMMENT---------

$pep8 <dir> --diff --max-line-length=159 --show-source < attachment.diff
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:24:6: E123 closing bracket does not match indentation of opening bracket's line
     ],
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:25:6: E121 continuation line indentation is not a multiple of four
     # not sure if reftests other than crashtests or jsreftests are used for pandas
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:27:9: E121 continuation line indentation is not a multiple of four
        "--xre-path=/tools/xre",
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:35:6: E121 continuation line indentation is not a multiple of four
     "crashtest_options": [
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:36:9: E121 continuation line indentation is not a multiple of four
        "--xre-path=/tools/xre",
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:44:6: E121 continuation line indentation is not a multiple of four
     "jsreftest_options": [
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:45:9: E121 continuation line indentation is not a multiple of four
        "--xre-path=/tools/xre",
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:48:69: W291 trailing whitespace
        "--enable-privilege", "--ignore-window-size", "--bootstrap",
                                                                    ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:53:6: E121 continuation line indentation is not a multiple of four
     "robocop_options": [
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:54:9: E121 continuation line indentation is not a multiple of four
        "--xre-path=/tools/xre",
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:63:6: E121 continuation line indentation is not a multiple of four
     "all_mochitest_suites": {
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:64:9: E121 continuation line indentation is not a multiple of four
        "mochitest-1": ["total-chunks=8", "--this-chunk=1"],
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:73:6: E121 continuation line indentation is not a multiple of four
     "all_reftest_suites": {
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:74:9: E121 continuation line indentation is not a multiple of four
        "reftest-1": ["total-chunks=4", "--this-chunk=1"],
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:79:6: E121 continuation line indentation is not a multiple of four
     "all_crashtest_suites": {
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:80:9: E121 continuation line indentation is not a multiple of four
        #crashtest-1 is disabled for bug 728119, constant timeouts
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:84:6: E121 continuation line indentation is not a multiple of four
     "all_jsreftest_suites": {
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:85:9: E121 continuation line indentation is not a multiple of four
        "jsreftest-1": ["total-chunks=3", "--this-chunk=1"],
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:88:5: E123 closing bracket does not match indentation of opening bracket's line
    },
    ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:89:6: E121 continuation line indentation is not a multiple of four
     "all_robocop_suites": {
     ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:90:9: E121 continuation line indentation is not a multiple of four
        #plain is split
        ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:93:5: E123 closing bracket does not match indentation of opening bracket's line
    },
    ^
/tmp/tmp.gEmdYHvkVO/configs/android/android_panda_releng.py:96:49: W291 trailing whitespace
    "device_package_name": "org.mozilla.fennec",
                                                ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:28:80: E202 whitespace before ']'
SUITE_CATEGORIES = ['mochitest', 'reftest', 'crashtest', 'jsreftest', 'robocop' ]
                                                                               ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:30:1: E302 expected 2 blank lines, found 1
class PandaTest(TestingMixin, MercurialScript, VirtualenvMixin, MozpoolMixin, BuildbotMixin, SUTDeviceMozdeviceMixin):
^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:31:18: E222 multiple spaces after operator
    test_suites =  SUITE_CATEGORIES
                 ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:46:14: E121 continuation line indentation is not a multiple of four
             "action": "store",
             ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:51:14: E121 continuation line indentation is not a multiple of four
             "action": "store",
             ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:55:8: E121 continuation line indentation is not a multiple of four
       # [["--testManifest'"], {
       ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:61:14: E121 continuation line indentation is not a multiple of four
             "action": "store",
             ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:66:14: E121 continuation line indentation is not a multiple of four
             "action": "extend",
             ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:70:21: E126 continuation line over-indented for hanging indent
                    "Suites are defined in the config file.\n"
                    ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:73:10: E121 continuation line indentation is not a multiple of four
         [['--reftest-suite', ], {
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:80:10: E124 closing bracket does not match visual indentation
         ],
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:81:10: E121 continuation line indentation is not a multiple of four
         [['--crashtest-suite', ], {
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:88:10: E124 closing bracket does not match visual indentation
         ],
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:89:10: E121 continuation line indentation is not a multiple of four
         [['--jsreftest-suite', ], {
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:96:10: E124 closing bracket does not match visual indentation
         ],
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:97:10: E121 continuation line indentation is not a multiple of four
         [['--robocop-suite', ], {
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:104:10: E124 closing bracket does not match visual indentation
         ],
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:105:10: E121 continuation line indentation is not a multiple of four
         [['--run-all-suites', ], {
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:106:13: E121 continuation line indentation is not a multiple of four
            "action": "store_true",
            ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:112:10: E124 closing bracket does not match visual indentation
         ],
         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:153:1: W293 blank line contains whitespace

^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:156:65: E502 the backslash is redundant between brackets
        self.mozpool_device = self.config.get('mozpool_device', \
                                                                ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:157:17: E128 continuation line under-indented for visual indent
                self.buildbot_config.get('properties')["slavename"])
                ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:164:55: E502 the backslash is redundant between brackets
        self.info("Current time on device: %s - %s" % \
                                                      ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:165:13: E128 continuation line under-indented for visual indent
            (device_time, time.strftime("%x %H:%M:%S", time.gmtime(float(device_time)))))
            ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:197:33: E261 at least two spaces before inline comment
                elif code == 10: # XXX assuming this code is the right one
                                ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:245:5: E303 too many blank lines (2)
    def run_test(self):
    ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:254:34: W291 trailing whitespace
        print self.mozpool_device
                                 ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:280:36: E222 multiple spaces after operator
        dirs['abs_jsreftest_dir'] =  "/home/cltbld/tests/reftest"
                                   ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:286:37: E231 missing whitespace after ','
            abs_dirs['abs_work_dir'],'bin')
                                    ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:311:42: E222 multiple spaces after operator
        base_cmd.append("--deviceIP=%s" %  str(device_ip))
                                         ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:316:20: E222 multiple spaces after operator
        http_port =  '30%03i' % hostnumber
                   ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:317:19: E222 multiple spaces after operator
        ssl_port =  '31%03i' %  hostnumber
                  ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:317:31: E222 multiple spaces after operator
        ssl_port =  '31%03i' %  hostnumber
                              ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:320:17: E126 continuation line over-indented for hanging indent
                'symbols_path': self._query_symbols_url(),
                ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:323:13: E123 closing bracket does not match indentation of opening bracket's line
            }
            ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:334:27: E127 continuation line over-indented for visual indent
                          (suite_category, suite_category))
                          ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:339:39: E203 whitespace before ':'
        if not c.get('run_all_suites') :
                                      ^
/tmp/tmp.gEmdYHvkVO/scripts/android_panda.py:342:33: E222 multiple spaces after operator
            print "category: " +  category
                                ^

Comment 34

[Justin Wood (:Callek)]

Comment on attachment 763815 [details] [diff] [review] [diff] [review]
buildbot-configs patch

---------AUTOMATIC COMMENT---------
--  filter pep8-callek-june-2013 --
---------AUTOMATIC COMMENT---------

$pep8 <dir> --diff --max-line-length=159 --show-source < attachment.diff
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:282:10: E121 continuation line indentation is not a multiple of four
         ('mochitest-1',
         ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:282:25: W291 trailing whitespace
         ('mochitest-1',
                        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:283:13: E127 continuation line over-indented for visual indent
            {'suite': 'mochitest-plain',
            ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:290:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:291:9: E126 continuation line over-indented for hanging indent
        ('mochitest-2',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:291:24: W291 trailing whitespace
        ('mochitest-2',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:299:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:300:9: E126 continuation line over-indented for hanging indent
        ('mochitest-3',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:300:24: W291 trailing whitespace
        ('mochitest-3',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:308:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:309:9: E126 continuation line over-indented for hanging indent
        ('mochitest-4',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:309:24: W291 trailing whitespace
        ('mochitest-4',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:317:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:318:9: E126 continuation line over-indented for hanging indent
        ('mochitest-5',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:318:24: W291 trailing whitespace
        ('mochitest-5',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:326:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:327:9: E126 continuation line over-indented for hanging indent
        ('mochitest-6',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:327:24: W291 trailing whitespace
        ('mochitest-6',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:335:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:336:9: E126 continuation line over-indented for hanging indent
        ('mochitest-7',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:336:24: W291 trailing whitespace
        ('mochitest-7',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:344:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:345:9: E126 continuation line over-indented for hanging indent
        ('mochitest-8',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:345:24: W291 trailing whitespace
        ('mochitest-8',
                       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:353:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:357:9: E126 continuation line over-indented for hanging indent
        ('reftest-1',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:357:22: W291 trailing whitespace
        ('reftest-1',
                     ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:365:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:366:22: W291 trailing whitespace
        ('reftest-2',
                     ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:374:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:375:22: W291 trailing whitespace
        ('reftest-3',
                     ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:383:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:384:22: W291 trailing whitespace
        ('reftest-4',
                     ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:392:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:395:9: E126 continuation line over-indented for hanging indent
        ('crashtest',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:395:22: W291 trailing whitespace
        ('crashtest',
                     ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:403:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:407:8: E121 continuation line indentation is not a multiple of four
       ('jsreftest-1',
       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:408:13: E127 continuation line over-indented for visual indent
            {'suite': 'jsreftest',
            ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:416:9: E126 continuation line over-indented for hanging indent
        ('jsreftest-2',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:424:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:425:9: E126 continuation line over-indented for hanging indent
        ('jsreftest-3',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:433:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:437:9: E126 continuation line over-indented for hanging indent
        ('xpcshell',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:437:21: W291 trailing whitespace
        ('xpcshell',
                    ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:443:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:447:9: E126 continuation line over-indented for hanging indent
        ('mochitest-gl',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:447:25: W291 trailing whitespace
        ('mochitest-gl',
                        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:455:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:456:5: E123 closing bracket does not match indentation of opening bracket's line
    ]
    ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:459:8: E121 continuation line indentation is not a multiple of four
       ('reftest-1',
       ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:460:13: E127 continuation line over-indented for visual indent
            {'suite': 'reftestsmall',
            ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:468:9: E126 continuation line over-indented for hanging indent
        ('reftest-2',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:476:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:477:9: E126 continuation line over-indented for hanging indent
        ('reftest-3',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:485:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:486:9: E126 continuation line over-indented for hanging indent
        ('reftest-4',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:494:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:495:5: E123 closing bracket does not match indentation of opening bracket's line
    ]
    ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:499:9: E126 continuation line over-indented for hanging indent
        ('robocop-1',
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:507:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:516:9: E124 closing bracket does not match visual indentation
        ),
        ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:517:5: E123 closing bracket does not match indentation of opening bracket's line
    ]
    ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:597:4: E121 continuation line indentation is not a multiple of four
   'opt_unittest_suites': ANDROID_MOZHARNESS_MOCHITEST + ANDROID_MOZHARNESS_PLAIN_ROBOCOP + ANDROID_MOZHARNESS_JSREFTEST + ANDROID_MOZHARNESS_CRASHTEST + ANDROID_MOZHARNESS_MOCHITESTGL,
   ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:597:160: E501 line too long (185 > 159 characters)
   'opt_unittest_suites': ANDROID_MOZHARNESS_MOCHITEST + ANDROID_MOZHARNESS_PLAIN_ROBOCOP + ANDROID_MOZHARNESS_JSREFTEST + ANDROID_MOZHARNESS_CRASHTEST + ANDROID_MOZHARNESS_MOCHITESTGL,
                                                                                                                                                               ^
/tmp/tmp.pT76hA0FiW/mozilla-tests/mobile_config.py:598:160: E501 line too long (187 > 159 characters)
   'debug_unittest_suites': ANDROID_MOZHARNESS_MOCHITEST + ANDROID_MOZHARNESS_PLAIN_ROBOCOP + ANDROID_MOZHARNESS_JSREFTEST + ANDROID_MOZHARNESS_CRASHTEST + ANDROID_MOZHARNESS_MOCHITESTGL,
                                                                                                                                                               ^

Comment 35

[Phil Ringnalda (:philor)]

Comment on attachment 762902 [details] [diff] [review]
patch so we can set script timeout via mozharness

>+            script_timeout=suites.get('timeout', 3600),

Even though that looks like "3600 if it isn't already set," this appears to have had the accidental result of making most of our previously 1200 second timeouts (bug 820739, bug 845280, bug 840305, probably more that we haven't adjusted the summary for yet) into 3600 second timeouts. It would be nice to have those 40 minutes back.

Comment 36

[Ed Morley [:emorley]]

(In reply to Phil Ringnalda (:philor) from comment #35)
> Even though that looks like "3600 if it isn't already set," this appears to
> have had the accidental result of making most of our previously 1200 second
> timeouts (bug 820739, bug 845280, bug 840305, probably more that we haven't
> adjusted the summary for yet) into 3600 second timeouts. 

Good spot - I'd tried to look for where this had changed but hadn't succeeded :-)

Comment 37

[Kim Moir [:kmoir] ET]

In production

Comment 38

[Kim Moir [:kmoir] ET]

Note I also gracefulled the panda android masters before the reconfig and changed the state of the devices from locked_out to ready. (pandas in range 22,33 34,45 46,82 522,874 and 885,887)   

I'll also address comments 34 and 35 in subsequent patches.  Also, I'm working on patches for talos android tests via mozharness.

Comment 39

[Phil Ringnalda (:philor)]

I'm not sure whether there are any critical results to this, or just aesthetic and annoyance results, but that had the effect of making the panda unittests double-log everything, e.g.

https://tbpl.mozilla.org/php/getParsedLog.php?id=24637979&tree=Mozilla-Inbound

16:59:56     INFO -  4916 INFO TEST-PASS | /tests/dom/workers/test/test_atob.html | Saw all values
06/26/2013 16:59:56: INFO:  4916 INFO TEST-PASS | /tests/dom/workers/test/test_atob.html | Saw all values
16:59:56     INFO -  4917 INFO TEST-END | /tests/dom/workers/test/test_atob.html | finished in 449ms
06/26/2013 16:59:56: INFO:  4917 INFO TEST-END | /tests/dom/workers/test/test_atob.html | finished in 449ms

Comment 40

[Phil Ringnalda (:philor)]

Also broke crash reporting, all panda crashes now look like https://tbpl.mozilla.org/php/getParsedLog.php?id=24638997&tree=Mozilla-Inbound

17:18:38  WARNING -  PROCESS-CRASH | testNewTab | application crashed [Unknown top frame]
06/26/2013 17:18:38: WARNING:  PROCESS-CRASH | testNewTab | application crashed [Unknown top frame]
17:18:38     INFO -  Crash dump filename: /tmp/tmpQnZLKu/64ec8656-41f8-670e-7f81b921-48018510.dmp
06/26/2013 17:18:38: INFO:  Crash dump filename: /tmp/tmpQnZLKu/64ec8656-41f8-670e-7f81b921-48018510.dmp
17:18:38     INFO -  MINIDUMP_STACKWALK not set, can't process dump.
06/26/2013 17:18:38: INFO:  MINIDUMP_STACKWALK not set, can't process dump.

Comment 42

[Kim Moir [:kmoir] ET]

Attachment: WIP patch for talos mozharness

These are not fully tested because I have to modify buildbot-custom for android talos and haven't got this working yet.

There are notes here on the steps yet to do
https://releng.etherpad.mozilla.org/133

Comment 43

[Kim Moir [:kmoir] ET]

Attachment: WIP patch for talos mozharness

Comment 44

[Kim Moir [:kmoir] ET]

Attachment: WIP patch for buildbotcustom patches for talos android mozharness

obviously still a lot of work to do

Comment 45

[Armen [:armenzg]]

I will be helping with this.
It might take me a bit to catch up.

Comment 46

[Armen [:armenzg]]

I've moved the talos part of this project to bug 891595 to make it easier to work on and track.

Comment 47

[Kim Moir [:kmoir] ET]

Attachment: bug829211talosmh.patch

Comment 48

[Kim Moir [:kmoir] ET]

Attachment: bug829211talosmh.patch

I still have a few minor issues to fix but would appreciate feedback at this time.

Comment 49

[Kim Moir [:kmoir] ET]

Attachment: bug829211-talosbuildbotcustom.patch

This is how I have the tests running in staging right now.  It's very hacked up and doesn't exclude the tegra tests from running this way which obviously needs to be fixed for production. 

In the mobile_config.py I have this....again temporary

BRANCHES['mozilla-central']['mozharness_repo'] = "http://hg.mozilla.org/users/kmoir_mozilla.com/mozharness"
BRANCHES['mozilla-central']['mozharness_tag'] = "default"
BRANCHES['mozilla-central']['mozharness_talos'] = True

My question is there a better way that I should be redirecting the tests to run via mozharness for talos that setting mozharness_talos to true.  The non-talos tests use use_mozharness': True in the configs but I found this didn't work out too well for the talos tests.

Comment 50

[Kim Moir [:kmoir] ET]

Attachment: mozharness scripts for android talos

Fixed the test result parsing to be appropriate for talos, still have to fix the timeouts as several of the tests are timing out.

Comment 51

[Aki Sasaki (not active)]

Comment on attachment 799490 [details] [diff] [review]
bug829211-talosbuildbotcustom.patch

(In reply to Kim Moir [:kmoir] from comment #49)
> This is how I have the tests running in staging right now.  It's very hacked
> up and doesn't exclude the tegra tests from running this way which obviously
> needs to be fixed for production.

Yeah.

> My question is there a better way that I should be redirecting the tests to
> run via mozharness for talos that setting mozharness_talos to true.  The
> non-talos tests use use_mozharness': True in the configs but I found this
> didn't work out too well for the talos tests.

We can have another check.

>+                    if branch_config.get('mozharness_talos'):

This gets tegras as well, as you pointed out.

>-                    if branch_config.get('mozharness_talos') and not platform_config.get('is_mobile'):

This excludes pandas, which we don't want.

But we could do

    if branch_config.get('mozharness_talos') and platform_config[slave_platform].get('mozharness_talos'):

And then set mozharness_talos to False for tegra_android and panda_android-nomozpool.


>diff --git a/misc.py b/misc.py
>--- a/misc.py
>+++ b/misc.py
>@@ -1850,48 +1850,55 @@ def generateTalosBranchObjects(branch, b
>                         'builddir': builddir,
>                         'slavebuilddir': slavebuilddir,
>                     }
>
>                     def _makeGenerateMozharnessTalosBuilderArgs(suite, talos_branch, platform,
>                                                                factory_kwargs, branch_config, platform_config):
>                         mh_conf = platform_config['mozharness_config']
>
>-                        extra_args = ['--suite', suite,
>-                                      '--add-option',
>-                                      ','.join(['--webServer', 'localhost']),
>-                                      '--branch-name', talos_branch,
>-                                      '--system-bits', mh_conf['system_bits'],
>-                                      '--cfg', mh_conf['config_file']]
>+                        extra_args = []
>+                        if 'android' not in platform:

Nit: trailing whitespace :)

Comment 52

[Aki Sasaki (not active)]

Comment on attachment 799773 [details] [diff] [review]
mozharness scripts for android  talos

This is awesome, thank you.

(In reply to Kim Moir [:kmoir] from comment #50)
> Fixed the test result parsing to be appropriate for talos, still have to fix
> the timeouts as several of the tests are timing out.

>diff --git a/configs/android/android_panda_talos_releng.py b/configs/android/android_panda_talos_releng.py

That's a verbose filename :)

>+import socket
>+
>+config = {
>+    # Values for the foopies
>+    "exes": {
>+        'python': '/tools/buildbot/bin/python',
>+        'virtualenv': ['/tools/buildbot/bin/python', '/tools/misc-python/virtualenv.py'],
>+    },
>+    "run_file_names": {
>+        "preflight_talos": "remotePerfConfigurator.py",
>+        "talos": "run_tests.py",
>+    },
>+    "retry_url":  "http://build.mozilla.org/talos/zips/retry.zip",
>+    "verify_path":  "/builds/sut_tools/verify.py",
>+    "install_app_path":  "/builds/sut_tools/installApp.py",
>+    "talos_from_code_url": "http://hg.mozilla.org/%s/raw-file/%s/testing/talos/talos_from_code.py",
>+    "talos_json_url": "http://hg.mozilla.org/%s/raw-file/%s/testing/talos/talos.json",
>+    #remotePerfConfigurator.py options
>+    "preflight_talos_options": [
>+        "-v", "-e", "%(app_name)s",
>+        "-t", "%(hostname)s",
>+        "--branchName=mobile",

I think you'll need to at least unhardcode the branchname... that should be different per branch aiui.

>+    "find_links": ["http://repos/python/packages"],

Could you use this find_links list, and also set pip_index to False ?
http://hg.mozilla.org/build/mozharness/file/4494bee8f781/configs/talos/linux_config.py#l11

That will prevent us from hitting pypi which will cause occasional tree bustage if it becomes available, and will make us hit a number of puppetagain servers for the package rather than relying on a single server being up.

>+    virtualenv_modules = [
>+        'mozpoolclient',
>+        'mozcrash',
>+        'talos'
>+    ]

I think we went with cloning talos in mozharness.mozilla.testing.talos, since the in-tree talos.json specifies a revision and it's easier to specify there than create a compressed archive and upload it somewhere and then point to it.

If we need to be able to decouple the desktop and mobile talos revisions, we could add a mobile_talos_revision here: http://hg.mozilla.org/mozilla-central/file/4b5789932294/testing/talos/talos.json#l6
Or maybe it makes sense to create a mobile_talos.json.  This is something we should probably discuss with #ateam

There may be reasons why we want to keep this more limited, since foopies + the webserver are updated much less frequently.  I think we should explicitly make the decision whether to use talos.json, though, and comment/document if we decide on an alternate approach.

>+        dirs['abs_talosdatatalos_dir'] = os.path.join(
>+            dirs['shutdown_dir'], 'talos-data/talos')

I imagine we'll revisit the naming of these things if/when we get off Foopies.

>+    def _query_symbols_url(self):

Hm, this seems to be http://hg.mozilla.org/build/mozharness/file/4494bee8f781/mozharness/mozilla/testing/testbase.py#l78 ?

>+    def close_request(self):
>+        mph = self.query_mozpool_handler(self.mozpool_device)
>+        mph.close_request(self.request_url)
>+        self.info("Request '%s' deleted on cleanup" % self.request_url)
>+        self.request_url = None

You may want to add a _post_fatal() method, like
http://hg.mozilla.org/build/mozharness/file/4494bee8f781/scripts/hg_git.py#l450

except instead of running notify() and copy_logs_to_upload_dir(), you probably want to self.close_request() (if there's a request to close; either check for an open request first, or make close_request() idempotent)

That way, if you hit a fatal() mid-run, you'll check the panda back in before exiting.

>+if __name__ == '__main__':
>+    pandaTalosTest = PandaTalosTest()
>+    pandaTalosTest.run()

I think you want run_and_exit() now.

Comment 53

[Kim Moir [:kmoir] ET]

Thanks aki, really appreciate all the suggestions for improvement!

Comment 55

[Kim Moir [:kmoir] ET]

aki: I fixed all the issues you found except for unhardcoding the branchname

The branchname for mobile talos is set here 
http://hg.mozilla.org/build/buildbotcustom/file/38c9f32e09c7/misc.py#l1759

Is the best way to pass it from there to mozharness just to add it as a parameter when I call the initial mozharness script in my modified makeGenerateMozharnessTalosBuilderArg http://hg.mozilla.org/build/buildbotcustom/file/38c9f32e09c7/misc.py#l1854 or is there a better way?

Comment 56

[Aki Sasaki (not active)]

(In reply to Kim Moir [:kmoir] from comment #55)
> aki: I fixed all the issues you found except for unhardcoding the branchname
> 
> The branchname for mobile talos is set here 
> http://hg.mozilla.org/build/buildbotcustom/file/38c9f32e09c7/misc.py#l1759
> 
> Is the best way to pass it from there to mozharness just to add it as a
> parameter when I call the initial mozharness script in my modified
> makeGenerateMozharnessTalosBuilderArg
> http://hg.mozilla.org/build/buildbotcustom/file/38c9f32e09c7/misc.py#l1854
> or is there a better way?

It looks like desktop talos already gets a |--branch-name Firefox| in the commandline arguments... setting the _makeGenerateMozharnessTalosBuilderArgs talos_branch to the appropriate mobile_talos_branch, and using that value as your talos branch name should do the trick.

Comment 57

[Kim Moir [:kmoir] ET]

Attachment: bug829211talosbbcustom.patch

Comment 58

[Kim Moir [:kmoir] ET]

Attachment: mozharness scripts for android talos with aki's feedback changes

These patches incorporate all the feedback suggestions.  Still having issues with many tests timing out which I'm investigating.

Comment 59

[Kim Moir [:kmoir] ET]

Attachment: buildbot configs for talos mh

Plus pep8 changes.  Right now it just reflects this change in the m-c branch for pandas as is the case on my dev-master, might want to really use cedar when they are ready.

Comment 60

[Aki Sasaki (not active)]

Comment on attachment 800985 [details] [diff] [review]
buildbot configs for talos mh

This is personal preference, but I think >80 char lines is often more readable than breaking up the line in weird places.  This patch has a lot of that... more pep8 line-length compliant, but less readable.  IMO.

If you're looking to get this file all the way there:

mozilla-tests/mobile_config.py|76 col 41 error| E124 closing bracket does not match visual indentation [pep8]

For this, it wants the closing brace to be one space in:

    {...
     ...
     }

mozilla-tests/mobile_config.py|79 col 41 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|82 col 51 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|685 col 10 error| E121 continuation line indentation is not a multiple of four [pep8]
mozilla-tests/mobile_config.py|687 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|696 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|699 col 10 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|702 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|711 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|714 col 10 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|717 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|726 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|729 col 10 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|732 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|741 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|744 col 10 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|747 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|755 col 14 error| E122 continuation line missing indentation or outdented [pep8]
mozilla-tests/mobile_config.py|758 col 10 error| E124 closing bracket does not match visual indentation [pep8]
mozilla-tests/mobile_config.py|763 col 4 error| E121 continuation line indentation is not a multiple of four [pep8]

Comment 61

[Aki Sasaki (not active)]

Comment on attachment 800957 [details] [diff] [review]
bug829211talosbbcustom.patch

>+                        elif 'android' in platform:
>+                           scriptpath = "scripts/android_panda_talos.py"
>+                           script_maxtime = 10800

I think this hardcoded script_maxtime is ok for the time being, but ideally it would be in the platform config.  If this needs to be different than the unittest script_maxtime I'm ok with a talos_script_maxtime  (platform_config['mozharness_config'].get('talos_script_maxtime', platform_config['mozharness_config'].get('script_maxtime'], 7200)) would work)

Comment 62

[Aki Sasaki (not active)]

Comment on attachment 800963 [details] [diff] [review]
mozharness scripts for android talos with aki's feedback changes

Thanks for the changes!

I still think we want to use a talos.json (or mobile_talos.json) at some point, but I'm not going to force that at this point if #ateam doesn't.

Comment 63

[Kim Moir [:kmoir] ET]

I fixed the issues in comment #60, comment #61.  Aki, regarding comment #62 and talos.json this is used when I invoke talos_from_code.py which downloads the correct talos.zip.  Is that what you were referring to or is there something else I'm missing?

From a log
INFO - Running command: ['python', '/builds/panda-0312/talos-data/talos_from_code.py', '--talos-json-url', u'http://hg.mozilla.org/mozilla-central/raw-file/6f265af4e3d8/testing/talos/talos.json'] in /builds/panda-0312/talos-data
12:44:16     INFO - Copy/paste: python /builds/panda-0312/talos-data/talos_from_code.py --talos-json-url http://hg.mozilla.org/mozilla-central/raw-file/6f265af4e3d8/testing/talos/talos.json
12:44:17     INFO -  INFO: talos.json URL: http://hg.mozilla.org/mozilla-central/raw-file/6f265af4e3d8/testing/talos/talos.json
12:44:17     INFO -  INFO: Downloading http://talos-bundles.pvt.build.mozilla.org/zips/talos.fcbb9d7d3c78.zip as talos.zip
12:44:17     INFO - Return code: 0
12:44:17     INFO - Running command: ['unzip', '-q', '-o', '/builds/panda-0312/talos-data/talos.zip'] in /builds/panda-0312/talos-data
12:44:17     INFO - Copy/paste: unzip -q -o /builds/panda-0312/talos-data/talos.zip

I'll attach some newer patches once I determine why many of the tests are timing out and fix it.

Comment 64

[Kim Moir [:kmoir] ET]

I've made progress with the timeout issues and have hit another problem.

Aki: I have another question. In the existing non-mozharness implementation the BuildSlaves.py file downloaded to the /builds/panda-0319/talos-data/talos/BuildSlaves.py directory so the datazilla credentials are there to store the performance results.  This is done using FileDownload which is a buildbot task that copies from master to slave.

With the switch to mozharness this doesn't exist.  Not sure of the way to implement this in mozharness without the custom buildbot calls.

Comment 65

[Aki Sasaki (not active)]

(In reply to Kim Moir [:kmoir] from comment #63)
> I fixed the issues in comment #60, comment #61.  Aki, regarding comment #62
> and talos.json this is used when I invoke talos_from_code.py which downloads
> the correct talos.zip.  Is that what you were referring to or is there
> something else I'm missing?

http://hg.mozilla.org/mozilla-central/file/4d152d0220be/testing/talos/talos.json

The zip portion is deprecated and will go away when we stop using it; we want to clone from hg and install from there.

Below, you can see the suite definitions.  These are all desktop-centric, so if we go this route we probably need to either define new suites or have a mobile-talos.json.

http://hg.mozilla.org/build/mozharness/file/67bce045a06c/mozharness/mozilla/testing/talos.py is what mozharness desktop talos uses.  Ideally desktop and mobile would share as much code/workflow as possible, so we don't have to support two completely different things.

(In reply to Kim Moir [:kmoir] from comment #64)
> I've made progress with the timeout issues and have hit another problem.
> 
> Aki: I have another question. In the existing non-mozharness implementation
> the BuildSlaves.py file downloaded to the
> /builds/panda-0319/talos-data/talos/BuildSlaves.py directory so the
> datazilla credentials are there to store the performance results.  This is
> done using FileDownload which is a buildbot task that copies from master to
> slave.
> 
> With the switch to mozharness this doesn't exist.  Not sure of the way to
> implement this in mozharness without the custom buildbot calls.

It looks like we already hacked our way around this with use_credentials_file in ScriptFactory:

http://hg.mozilla.org/build/buildbotcustom/file/9c06d8dca47b/process/factory.py#l6451

Comment 66

[Kim Moir [:kmoir] ET]

Attachment: bug829211mhsept10.patch

mozharness scripts that gave green results after changes suggested by aki.  Still have to look using mobile_talos.json instead of talos.zip as discussed in #ateam today (bug 914799)

Comment 67

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbcustomsept10.patch

Comment 68

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbconfigssept10.patch

Comment 69

[Kim Moir [:kmoir] ET]

Comment on attachment 802602 [details] [diff] [review]
bug829211bbcustomsept10.patch

Even though I'm working on the mobile_talos.json file testing, it would be good to get a review to land these patches before they become bittrotten.

Comment 70

[Aki Sasaki (not active)]

Comment on attachment 802600 [details] [diff] [review]
bug829211mhsept10.patch

Thanks!
The interdiff looks good.
Nit: trailing whitespace on line 324 (above _query_abs_base_cmd)

Comment 71

[Aki Sasaki (not active)]

Comment on attachment 802602 [details] [diff] [review]
bug829211bbcustomsept10.patch

Three things:

1. I think I had a handle on why you were using |is_mobile| in some places and |'android' in platform| in others, but I lost it since.

Is there a valid difference here?  If it's important it might be good to add a comment.

2. Trailing whitespace nit :)  3 places.

3. Did you intend to ask for review for the buildbot-configs patch too?

Comment 72

[Kim Moir [:kmoir] ET]

Comment on attachment 802600 [details] [diff] [review]
bug829211mhsept10.patch

fixed whitespace issue

Comment 73

[Armen [:armenzg]]

(In reply to Kim Moir [:kmoir] from comment #69)
> Comment on attachment 802602 [details] [diff] [review]
> bug829211bbcustomsept10.patch
> 
> Even though I'm working on the mobile_talos.json file testing, it would be
> good to get a review to land these patches before they become bittrotten.

With regards to mobile_talos.json, which bug are we using for it?
Are we sure we don't want to still use talos.json and share the same revision with desktop?
Is this for the android_panda_talos.py work?

Comment 74

[Kim Moir [:kmoir] ET]

regarding comment 73

Armen:

We discussed this with the ateam in irc and decided that having a mobile_talos.json was a good idea, bug 914799. Yes, this is for the android_panda_talos.py work.  Since the tests might be different, it was decided to have a separate file.

Comment 75

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbcustomsept12.patch

This is a better patch, excludes the is_mobile bit.  I fixed the whitespace. I didn't intend to ask for review for the buildbot one because it includes m-c as the branch and it should include cedar to start.

Comment 76

[Armen [:armenzg]]

Merged to mozharness' production branch.
It is now live on tbpl.

Comment 77

[Kim Moir [:kmoir] ET]

Comment on attachment 803790 [details] [diff] [review]
bug829211bbcustomsept12.patch

bustage after reconfig because android_panda_talos.py is used to invoke desktop, will fix this

Comment 78

[Kim Moir [:kmoir] ET]

Attachment: updated bbcustom patch with correct path for desktop tests

Comment 79

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbconfigssept13.patch

buildbot configs to enable mozharness talos on cedar.  Also addresses some pep8 issues but not where the character limit make it less readable.

Comment 80

[Aki Sasaki (not active)]

Comment on attachment 804584 [details] [diff] [review]
bug829211bbconfigssept13.patch

> # Beginning Androidx86 configurations
> ANDROID_X86_MOZHARNESS_DICT = [
>-         ('androidx86-set-1',
>-             {
>-             'use_mozharness': True,
>-             'script_path': 'scripts/androidx86_emulator_unittest.py',
>-             'extra_args': [
>-                 '--cfg', 'android/androidx86.py',
>+    ('androidx86-set-1',
>+     {
>+         'use_mozharness': True,
>+         'script_path': 'scripts/androidx86_emulator_unittest.py',
>+         'extra_args': [
>+    '--cfg', 'android/androidx86.py',
>                  '--test-suite', 'mochitest-1',
>                  '--test-suite', 'mochitest-2',
>                  '--test-suite', 'mochitest-3',
>                  '--test-suite', 'mochitest-4',
>              ],
>              'timeout': 2400,
>              'script_maxtime': 14400,
>              },

The spacing for the --cfg line looks odd... looks like someone will need to get the android x86 stuff pep8 friendly at some point.
I don't think that blocks this patch from landing, though.

Comment 81

[Kim Moir [:kmoir] ET]

Attachment: fix action order issue aki mentioned in irc

Comment 82

[Kim Moir [:kmoir] ET]

Comment on attachment 804741 [details] [diff] [review]
fix action order issue aki mentioned in irc

tested in staging

Comment 83

[Aki Sasaki (not active)]

Comment on attachment 804741 [details] [diff] [review]
fix action order issue aki mentioned in irc

So, out of scope, but just fyi:

* iirc create-virtualenv for talos requires us to run clone-talos first, because we want to install talos from source
* idle timeouts for hg clones requires a virtualenv with mozprocess in it

So we have a chicken and egg scenario for talos.
At some point we may want to create a small virtualenv, with mozprocess in it, then clone talos (with an idle timeout), then populate the virtualenv with talos.
Not part of this bug, but someday.

Comment 84

[Kim Moir [:kmoir] ET]

Comment on attachment 804741 [details] [diff] [review]
fix action order issue aki mentioned in irc

Thanks Aki, I didn't know about that issue. Something to ponder.

Comment 85

[Aki Sasaki (not active)]

Landed https://hg.mozilla.org/build/buildbotcustom/rev/9fc08491a808 as a fix for desktop talos.

Comment 86

[Aki Sasaki (not active)]

I'm going to back this out, as this went live for pandas across all branches, not just cedar.

Comment 87

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbcustomsept17.patch

Comment 88

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbconfigssept17.patch

Comment 89

[Kim Moir [:kmoir] ET]

Attachment: bug829211mhsept17.patch

I tested today's patches in cedar, m-i and m-c.  I verified that m-i and m-c tests do not run via mozharness, but cedar does :-)

Comment 90

[Aki Sasaki (not active)]

Comment on attachment 806243 [details] [diff] [review]
bug829211mhsept17.patch

Looks like this'll fix the talos_from_code issue that we saw late yesterday, cool.

Comment 91

[Aki Sasaki (not active)]

Attachment: builder diff for bm29-tests1-panda

I:

* cloned a clean buildbot-configs, tools, buildbotcustom
* created test master dirs:

./setup-master.py bm19-tests1-tegra bm19-tests1-tegra
./setup-master.py bm29-tests1-panda bm29-tests1-panda
./setup-master.py bm51-tests1-linux bm51-tests1-linux
./setup-master.py bm69-tests1-windows bm69-tests1-windows
./setup-master.py bm75-tests1-macosx bm75-tests1-macosx

* did a builder dump for each:

for i in bm*; do     pushd $i;         /src/clean/braindump/buildbot-related/builder_list.py master.cfg > ../../$i-clean;     popd; done

* applied the latest r? buildbot{-configs,custom} patches
* did a builder dump for each:

for i in bm*; do     pushd $i;         /src/clean/braindump/buildbot-related/builder_list.py master.cfg > ../../$i-dirty;     popd; done

* diffed each:

for i in bm*; do pushd ..; diff $i-{clean,dirty} > $i.diff; popd; done


The linux, osx, and windows test masters have 0 diffs.

The tegra and panda diffs look like the attached patch: cedar talos changes from TalosFactory to ScriptFactory.

Comment 92

[Aki Sasaki (not active)]

Comment on attachment 806242 [details] [diff] [review]
bug829211bbconfigssept17.patch

Interdiff and builder list look good.

Comment 93

[Aki Sasaki (not active)]

Comment on attachment 806240 [details] [diff] [review]
bug829211bbcustomsept17.patch

Interdiff and builder list look good.

Comment 94

[Kim Moir [:kmoir] ET]

Attachment: bug829211mhsept18.patch

don't need the clone-talos task

Comment 95

[Kim Moir [:kmoir] ET]

Attachment: bug829211fixpoolforcedar.patch

So the mozharness talos tests enabled on cedar are retrying because they are still pointing to the panda_android-nomozpool and those devices are locked out.  Thus when mozpool tries to retrieve a device it fails because none of those are available.  This issue all go away when I enable the rest of the branches for talos mozharness because I'll revert the changes for the two pools (bug 913206) but in the interim it would be good to see green tests on cedar.  

I've tested this in staging with sendchanges for cedar and m-c and ensured that the correct tests run from the correct pools and that android tegra tests run as they should.

Comment 96

[Justin Wood (:Callek)]

Comment on attachment 807351 [details] [diff] [review]
bug829211fixpoolforcedar.patch

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

::: mozilla-tests/mobile_config.py
@@ +1032,5 @@
>      branchConfig = PROJECT_BRANCHES[projectBranch]
>      loadDefaultValues(BRANCHES, projectBranch, branchConfig)
>      loadCustomTalosSuites(BRANCHES, SUITES, projectBranch, branchConfig)
> +    
> +# start temp fix until panda android pools are collapsed again in bug 913206    

previous 2 lines has whitespace EOL

Comment 97

[Kim Moir [:kmoir] ET]

Green tests on cedar for talos mozharness for Android 4.0 
https://tbpl.mozilla.org/?noigore=1&tree=Cedar&rev=20adcc0a579d

Comment 98

[Kim Moir [:kmoir] ET]

Is there anything else that needs to be done to verify talos results once they are green on a branch?  Not sure if someone needs to verify the numbers stored in datazilla are correct.

5:26:25     INFO -  cycle time: 00:05:14
15:26:25     INFO -  panda-0772:
15:26:25     INFO -  		Stopped Fri, 20 Sep 2013 15:26:25
15:26:25     INFO -  INFO : Outputting talos results => {'results_urls': ['http://graphs.mozilla.org/server/collect.cgi'], 'datazilla_urls': ['https://datazilla.mozilla.org/talos']}
15:26:25     INFO -  Generating results file: trobopan:
15:26:25     INFO -  		Started Fri, 20 Sep 2013 15:26:25
15:26:25     INFO -  Generating results file: trobopan:
15:26:25     INFO -  		Stopped Fri, 20 Sep 2013 15:26:25
15:26:25     INFO -  INFO : Posting result 0 of 1 to http://graphs.mozilla.org/server/collect.cgi, attempt 0
15:26:25     INFO -  INFO : Outputting datazilla results to https://datazilla.mozilla.org/talos
15:26:25     INFO -  INFO : datazilla: https//datazilla.mozilla.org/talos; oauth=True
15:26:26     INFO -  INFO : Datazilla results at https://datazilla.mozilla.org/talos/summary/Cedar/20adcc0a579d?product=Fennec&branch_version=27.0a1
15:26:26     INFO -  RETURN: <a href='http://graphs.mozilla.org/graph.html#tests=[[174,26,29]]'>trobopan: 24412.00</a>
15:26:26     INFO -  Datazilla response is: {"status": "well-formed json stored", "url": "https://datazilla.mozilla.org/talos/refdata/objectstore/json_blob/2804956", "size": 618}
15:26:26     INFO -  TinderboxPrint: TalosResult: {"datazilla": {"trobopan": {"url": "https://datazilla.mozilla.org/talos/summary/Cedar/20adcc0a579d?product=Fennec&branch_version=27.0a1"}}, "graphserver": {"trobopan": {"url": "http://graphs.mozilla.org/graph.html#tests=[[174,26,29]]", "result": "24412.00"}}}

Comment 99

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbcustomsept23.patch

Found this issue while I was testing today

Comment 100

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbconfigssept23.patch

These are patches to enable mozharness tests on other branches + collapse the two pools of pandas back to one.  I don't intend to land this immediately if approved because there are some other changes that need to happen at the same time as the reconfig, for instance changing the mozpool state of the pandas that are locked out (because not being managed by mozpool) to a ready state.  I tested in in staging and verified that sendchanges for m-c and cedar talos work with mozharness/mozpool and that tests on tegras still run the old way.

Comment 102

[Aki Sasaki (not active)]

Comment on attachment 808820 [details] [diff] [review]
bug829211bbcustomsept23.patch

Looks like this change alone doesn't change builders, so we're good.

Comment 103

[Aki Sasaki (not active)]

Comment on attachment 808824 [details] [diff] [review]
bug829211bbconfigssept23.patch

Looks like we also need to remove it from BuildSlaves.py.template (and puppet+puppet-manifests patches).

Comment 104

[Kim Moir [:kmoir] ET]

Regarding comment #103, yes I'm going to remove it from the BuildSlaves.py template and puppet once we have them working on all branches, until then we can leave it there.

Comment 105

[Armen [:armenzg]]

Live on production.

(Not CCing myself)

Comment 106

[Kim Moir [:kmoir] ET]

Attachment: buildbot configs to enable on all branches

updated patch now that bug 918463 has landed

Comment 107

[Kim Moir [:kmoir] ET]

Attachment: buildbot configs to enable on all branches

updated patch now that bug 918463 has landed

Comment 108

[Kim Moir [:kmoir] ET]

In production

Comment 109

[Kim Moir [:kmoir] ET]

Attachment: tspaint.patch

Missed tspaint

Comment 110

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

Comment on attachment 809952 [details] [diff] [review]
tspaint.patch

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

::: configs/android/android_panda_talos_releng.py
@@ +50,5 @@
>          "remote-trobopan":  ["--activeTests=trobopan", "--noChrome", "--fennecIDs=../fennec_ids.txt"],
>          "remote-tsvg":  ["--activeTests=tsvg", "--noChrome"],
>          "remote-tp4m_nochrome":  ["--activeTests=tp4m", "--noChrome", "--rss"],
>          "remote-trobocheck2":  ["--activeTests=tcheck2", "--noChrome", "--fennecIDs=../fennec_ids.txt"],
> +        "remote-tspaint": ["--activeTests=ts_paint", "--mozAfterPaint", "--noChrome"],

technically we don't need the --noChrome, although it doesn't hurt anything.

Comment 111

[Kim Moir [:kmoir] ET]

Comment on attachment 809952 [details] [diff] [review]
tspaint.patch

removed noChrome, landed and merged to production

Comment 112

[Kim Moir [:kmoir] ET]

As of this morning talos tests for Pandas are running using mozpool/mozharness

There are some cleanup tasks to be done such as
-reboot only using mozpool bug 889967, bug 919533
-consume talos.json for mobile suites bug 920757 

but the main intent of this bug is in production

Comment 113

[Kim Moir [:kmoir] ET]

Attachment: bug829211template.patch

cleanup BuildSlaves.py template file

Comment 114

[Kim Moir [:kmoir] ET]

Attachment: remove old slave definition from puppet

Comment 115

[Kim Moir [:kmoir] ET]

Attachment: bug829211bbcustom.patch

clean up buildbotcustom

Comment 116

[[e-mail] (:bhearsum)]

something here is in production

Comment 118

[Kim Moir [:kmoir] ET]

This work was implemented last quarter. The only related issues that remain are bug 920757 (talos.json) and bug 889967 (all reboots via mozpool). So I'm going to close this bug since panda android tests do indeed run via mozpool and mozharness.