882324 - Support reftests for pandaboards on Cedar

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Dan Minor [:dminor]]

Once manifests are prepared for reftests on panda as part of bug 877266, the next step is to get them running on Cedar to assess how stable they are. They seem solid running on a local panda.

Comment 1

[Kim Moir [:kmoir] ET]

Should they be reftests or reftestsmall?

reftestsmall have the extra-arg
--ignore-window-size

Comment 2

[Kim Moir [:kmoir] ET]

Attachment: patch

Comment 3

[Kim Moir [:kmoir] ET]

Attachment: patch

Comment 4

[Justin Wood (:Callek)]

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

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

::: mozilla-tests/mobile_config.py
@@ +629,5 @@
>                  for suite in BRANCHES[branch]['platforms'][platform][slave_plat][type][:]:
>                      if "xpcshell" in suite[0]:
>                          BRANCHES[branch]['platforms'][platform][slave_plat][type].remove(suite)
> +                    if (("plain-reftest" in suite[0]) and (branch != "cedar")) and \
> +                    BRANCHES[branch]['platforms'][platform] == "panda_android":

I don't like piggybacking on the xpcshell logic (which is riding trains) for a different suite that we're trying to turn on for just cedar.

We should instead copy this block and modify accordingly.

Comment 5

[Kim Moir [:kmoir] ET]

Attachment: patch #2

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 762939 [details] [diff] [review]
patch #2

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

sorry for another r-, this one is because I am pretty sure your "panda_android" check is wrong. See below.

::: mozilla-tests/mobile_config.py
@@ +631,5 @@
>                          BRANCHES[branch]['platforms'][platform][slave_plat][type].remove(suite)
>  
> +#bug 882324 - Support reftests for pandaboards on Cedar
> +for branch in BRANCHES:
> +    # Loop removes it from any branch that gets beyond here

if we don't do branch check here, remove this comment.

@@ +644,5 @@
> +            if not slave_plat in BRANCHES[branch]['platforms'][platform]:
> +                continue
> +            for type in BRANCHES[branch]['platforms'][platform][slave_plat]:
> +                for suite in BRANCHES[branch]['platforms'][platform][slave_plat][type][:]:
> +                    if (("plain-reftest" in suite[0]) and (branch != "cedar")) and \

I would have preferred to do the cedar check above like in the above loop (since it makes it more obvious how to deploy to other branches), but I won't block on it

@@ +645,5 @@
> +                continue
> +            for type in BRANCHES[branch]['platforms'][platform][slave_plat]:
> +                for suite in BRANCHES[branch]['platforms'][platform][slave_plat][type][:]:
> +                    if (("plain-reftest" in suite[0]) and (branch != "cedar")) and \
> +                    BRANCHES[branch]['platforms'][platform] == "panda_android":

hrm actually this will never match,

BRANCH[branch]['platforms'][platform] is a dictionary, so will never match panda_android.

I suspect what you want is to do:

if not slave_plat is "panda_android":
   continue

just above the |type| for loop.

Comment 7

[Kim Moir [:kmoir] ET]

Attachment: patch

I tested this in staging.

Comment 8

[Justin Wood (:Callek)]

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

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

r+ if my nit is addressed

::: mozilla-tests/mobile_config.py
@@ +631,5 @@
>                          BRANCHES[branch]['platforms'][platform][slave_plat][type].remove(suite)
>  
> +#bug 882324 - Support reftests for pandaboards on Cedar
> +for branch in BRANCHES:
> +    # Loop removes it from any branch that gets beyond here

I prefer to add here...

   if branch in ('cedar', ):
      continue

Then when we're ready to land to m-c its a simple flip of "not in ('m-aurora', ...)" etc

@@ +646,5 @@
> +            if not slave_plat == "panda_android":
> +                continue
> +            for type in BRANCHES[branch]['platforms'][platform][slave_plat]:
> +                for suite in BRANCHES[branch]['platforms'][platform][slave_plat][type][:]:
> +                    if (("plain-reftest" in suite[0]) and (branch != "cedar")):

then we can remove the cedar check here.... otherwise we traverse this whole list of for loops before we get an easy mental model of which branches this all applies to.

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 763517 [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.FkuKcxXoe7/mozilla-tests/mobile_config.py:651:24: E111 indentation is not a multiple of four
                       BRANCHES[branch]['platforms'][platform][slave_plat][type].remove(suite)
                       ^

Comment 10

[Dan Minor [:dminor]]

Sorry I missed this earlier, but yes it is reftestsmall.

Comment 11

[Kim Moir [:kmoir] ET]

Attachment: updated patch

Now that changes from bug 829211 have landed we need to invoke these via mozharness

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 770800 [details] [diff] [review]
updated patch

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

like before r+ if my nits (as listed in https://bugzilla.mozilla.org/show_bug.cgi?id=882324#c8 ) are addressed :-)

Comment 13

[Kim Moir [:kmoir] ET]

Attachment: patch

sorry I missed that nits comment this morning :-)

Comment 14

[Justin Wood (:Callek)]

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

https://hg.mozilla.org/build/buildbot-configs/rev/7abcd7f691b4

Comment 15

[Rail Aliiev [:rail]]

In production

Comment 16

[Justin Wood (:Callek)]

Attachment: [mozharness] ignore-window-size

I *think* this is all I need to solve this issue.

At: https://tbpl.mozilla.org/php/getParsedLog.php?id=25661003&tree=Cedar&full=1

Comment 17

[Aki Sasaki (not active)]

Comment on attachment 780497 [details] [diff] [review]
[mozharness] ignore-window-size

This will add --ignore-window-size to the reftest harness call, but we'll still hit this issue:

android_panda.py: error: no such option: --ignore-window-size

To fix that issue, we can do one of the following:

a) add --ignore-window-size to the list of commandline options that android_panda.py will accept:
http://hg.mozilla.org/build/mozharness/file/9ccb65b68374/scripts/android_panda.py#l34

Then use self.config['ignore_window_size'] when generating commandline options for the test harness.

or,

b) land this patch, and also land a patch in buildbot-configs to stop passing --ignore-window-size to android_panda.py.


I think I prefer (b).

r+ing, with the caveat that these jobs will still be red until we stop passing --ignore-window-size to the mozharness script.

Comment 18

[Justin Wood (:Callek)]

Comment on attachment 780497 [details] [diff] [review]
[mozharness] ignore-window-size

https://hg.mozilla.org/build/mozharness/rev/c624755975bc

(going with the more-patches but (b) option felt better to me as well)

Comment 19

[Justin Wood (:Callek)]

Attachment: [configs] Remove --ignore-window-size from being passed to mozharness script

Comment 20

[Justin Wood (:Callek)]

Comment on attachment 780795 [details] [diff] [review]
[configs] Remove --ignore-window-size from being passed to mozharness script

https://hg.mozilla.org/build/buildbot-configs/rev/6e6088fc8791

Comment 21

[Ed Morley [:emorley]]

(In reply to Justin Wood (:Callek) from comment #18)
> Comment on attachment 780497 [details] [diff] [review]
> [mozharness] ignore-window-size
> 
> https://hg.mozilla.org/build/mozharness/rev/c624755975bc
> 
> (going with the more-patches but (b) option felt better to me as well)

Mozharness part merged to production.

Comment 22

[Rail Aliiev [:rail]]

In production

Comment 23

[Justin Wood (:Callek)]

Soooo... we're *now* failing with:

16:00:27     INFO -  remotereftest.py: error: no such option: --certificate-path
16:00:27    ERROR - Return code: 2


Dan, can you verify what our options should be here, if there are any other issues, etc?

Current can be seen within the context of https://hg.mozilla.org/build/mozharness/rev/c624755975bc

Comment 24

[Dan Minor [:dminor]]

Callek, this works on my local panda:

_tests/reftest/remotereftest.py --dm_trans=sut --ignore-window-size --app=org.mozilla.fennec_dminor --deviceIP=172.16.0.115 --xre-path=/home/dminor/mozilla-central/obj-x86_64-unknown-linux-gnu/dist/bin --httpd-path=_tests/reftest/reftest/components --symbols-path=./dist/crashreporter-symbols  "tests/layout/reftests/reftest.list"

Once --certificate-path is removed, the other difference is --utility-path, which is a valid option.

You are also still pointing to the crashtest manifest "reftest/tests/testing/crashtest/crashtests.list" instead of the reftest manifest "tests/layout/reftests/reftest.list"

Comment 25

[Justin Wood (:Callek)]

Attachment: [mozharness] fix reftest command line

This, hopefully, is the last part of this bug

Comment 26

[Dan Minor [:dminor]]

Comment on attachment 782601 [details] [diff] [review]
[mozharness] fix reftest command line

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

I missed this in the needinfo, but --console-level=INFO is also not a valid option for remotereftest.py, remove that and it looks good.

Comment 27

[Justin Wood (:Callek)]

Comment on attachment 782601 [details] [diff] [review]
[mozharness] fix reftest command line

https://hg.mozilla.org/build/mozharness/rev/5099407c412c

With nit fixed

Comment 28

[Justin Wood (:Callek)]

And they are running (and failing in test-specific ways) now!

Calling this bug, as scoped done

Comment 29

[John Hopkins (:jhopkins)]

merged to production branch