511282 - run fennec unittests on intel linux machines

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Aki Sasaki (not active)]

on change, then change the n810s to run on nightlies.

This would apply to m-c, m-1.9.2, and tm currently.

Adding more linux slaves to deal with the added load would be the Right Thing To Do.

Comment 1

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

(In reply to comment #1)
> Adding more linux slaves to deal with the added load would be the Right Thing
> To Do.
I filed bug#510604 last week for exactly this.

Comment 2

[Aki Sasaki (not active)]

I think this is the right bug:

Joel: Did you run unittests on Fennec desktop with Maemkit, or without?

Comment 3

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

you don't need maemkit, but it helps when mochitest crashes.  I developed maemkit on desktop fennec linux

Comment 4

[Aki Sasaki (not active)]

We're planning on having Maemo4 (done), Maemo5, and WinMo unit tests all on device by EOQ1.  The Mobile team has a Q1 goal of getting unit tests green and useful.  If the unit tests on device are actually looked at, it makes desktop Fennec unit tests less urgent.

Not on Q1 goals list; futuring.

Comment 5

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

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 6

[John Ford [:jhford] CET/CEST Berlin Time]

I just did a test using a slightly modified UnittestPackagedBuildFactory with some basic information of fennec hardcoded (i.e. --appname=fennec/fennec instead of firefox/firefox-bin).  Basically, I ran the standard firefox unit test commands on a fennec tarball + tests zip.

Non-mochitest actually went pretty well:
jsreftest - green 53990/0/1055
reftest - orange 4713/74/277
'opengl' - lots of failures and eventually a crash
xpcshell - lots of passes until [1], then 1200 seconds without output
crashtest - orange 1648/2/10

Mochitest, however, didn't work so well:
mochitest 1-5 - don't start properly
mochitest-chrome - doesn't start properly [2]
mochitest-a11y - doesn't start properly
mochitest-browser-chrome - orange 320/0/9 (error was rc:245) [3]

There were lots of crashses.  Because symbols aren't uploaded beside the dep-linux builds, I was using a dummy symbols archive resulting in not being able resolve symbols.  Uploading symbols archives will begin before this would land.

This also looks like a good usage for maemkit :)


Notes Below:
================================================================================

[1] '/home/cltbld/talos-slave/mozilla-central_fedora_test-xpcshell/build/xpcshell/tests/test_extensionmanager/xpcshell/test_AddonRepository.js

[2]
INFO | runtests.py | Server pid: 1969
INFO | runtests.py | Websocket server pid: 1974
INFO | runtests.py | Running tests: start.

pk12util: PKCS12 IMPORT SUCCESSFUL
INFO | automation.py | SSL tunnel pid: 1982
INFO | automation.py | Application pid: 1983
Server listening on port 4443 with cert pgo server certificate
creating 1!
[TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (0d, 0d)
###################################### forms.js loaded
###################################### content loaded
!! remote browser loaded
loading about:blank, 1
loading http://mochi.test:8888/redirect.html?autorun=1&closeWhenDone=1&consoleLevel=INFO, 1
[TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (0d, 0d)
[TabChild] MOVE to (x,y)=(0d, 0d), (w,h)= (800d, 500d)

###!!! [Child][SyncChannel] Error: Channel error: cannot send/recv


###!!! [Child][AsyncChannel] Error: Channel error: cannot send/recv

TEST-UNEXPECTED-FAIL | automation.py | Exited with code 1 during test run
INFO | automation.py | Application ran for: 0:00:03.171138
INFO | automation.py | Reading PID log: /tmp/tmpwd0awypidlog
==> process 1983 launched child process 1999
INFO | automation.py | Checking for orphan process with PID: 1999
PROCESS-CRASH | automation.py | application crashed (minidump found)
Operating system: Linux
                  0.0.0 Linux 2.6.31.5-127.fc12.i686.PAE #1 SMP Sat Nov 7 21:25:57 EST 2009 i686
CPU: x86
     GenuineIntel family 6 model 23 stepping 10
     2 CPUs

Crash reason:  SIGSEGV
Crash address: 0x0


[3]
TEST-START | Shutdown
NOTE: child process received `Goodbye', closing down
TEST-UNEXPECTED-FAIL | Shutdown | Exited with code -11 during test run
INFO | automation.py | Application ran for: 0:00:29.018338
INFO | automation.py | Reading PID log: /tmp/tmpic9pD2pidlog
==> process 2025 launched child process 2041
INFO | automation.py | Checking for orphan process with PID: 2041
WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!
INFO | runtests.py | Running tests: end.
program finished with exit code 245
elapsedTime=30.354912

Comment 7

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

problem [2] is bug 590121.  
problem [3] is something that we need to fix.  There are a few random oranges in browser-chrome for mobile and we are working on fixing those.  

With the normal chunking (M1-M5) we don't need to use maemkit on a linux desktop (or VM)

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #7)
> problem [2] is bug 590121. 

gtk, thanks :)
 
> problem [3] is something that we need to fix.  There are a few random oranges
> in browser-chrome for mobile and we are working on fixing those.  

Cool,  the weird thing there was that it had 0 unexpected failures.

> With the normal chunking (M1-M5) we don't need to use maemkit on a linux
> desktop (or VM)

Is it that we don't want maemkit or is it that maemkit just wouldn't do anything useful?

Comment 9

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

we wouldn't need maemkit, but there would be no harm in using it.  Ideally we would move away from maemkit as we get more serious about our unittests.  Right now the majority of maemkit functionality is already built into the core test harnesses.  There are some additional things like changing timeouts that we can do with extra cli options to the standard harnesses now.

But with the chunking and process management we have in all harnesses by default, we can do X # of chunks and produce results with the default mozilla-central harnesses.

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

Going to be working on these bugs

Comment 11

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

one thing you might want to consider for mochitest 1-5 is to turn off IPC by doing a:
python runtests.py --browser-arg="browser.tabs.remote=false" ...

Currently Fennec is the only product with IPC on by default and there are hundreds of individual test case files that will be broken in an unkind fashion.  

For browser-chrome we have worked around the issues and for chrome I don't know how many of the tests cross the chrome<->content boundaries.

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbot-configs v1

This patch adds the required configuration logic to add mobile builds to test-master02.

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbotcustom v1

This patch adds the require logic in the factory to run fennec unit tests on talos masters.  

A couple notes:
-checking for 'mobile' in the 'name_prefix' variable is a bit of a hack, but I think a reasonable one.
-productName logic that is currently there is only valid for Firefox.  On linux, fennec uses 'fennec' as the binary and there is no 'fennec-bin'.  Instead of setting the exepath based on the productName+'-bin', we could
  -optionally give an explicit binary name to the __init__ method that if specified, overrides any '%s-bin' hardcoded strings
  -have a different setting based on whether productName is 'firefox' or 'fennec'
  -ask mobile to include a fennec-bin (this is a terrible option imo)

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

mobile browser-chrome tests are important.



Notes from IRC:

<mbrubeck> jhford:  I use: python runtests.py --browser-chrome --appname ../../../dist/bin/fennec --test-path mobile
<mfinkle> jhford: python runtests.py --browser-chrome --appname=../../../dist/bin/fennec --xre-path=../../../dist/bin
<jhford> mfinkle: cool, i will teach UnittestPackagedBuildFactory how to do that :)
<mfinkle> oh yeah, --test-path is handy
<jmaher> jhford: and please add --test-path=mobile
<jmaher> ;)
<jmaher> jhford: then you will need something closer to 'python mochitest/runtests.py --browser-chrome --appname=fennec/fennec --certificate-path=certs --utiility-path=bin --test-path=mobile --autorun --close-when-done'
<jhford> jmaher, mfinkle, mind if i copy/paste this into the bug?
<jmaher> jhford: go for it...really just change the firefox stuff to be --appname=fennec/fennec and add --test-path=mobile

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

working on this again.

What coverage do we want?  Options are:

-reftest
-crashtest
-xpcshell
-jsreftest
-mochitest-plain (1-5)
-mochitest-chrome
-mochitest-a11y
-mochitest-ipcplugins

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

mobile specific browser chrome is assumed to be wanted

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #16)
> mobile specific browser chrome is assumed to be wanted

Yes, please use --test-path=mobile

Everything else is junk :)

(In reply to comment #15)
> working on this again.
> 
> What coverage do we want?  Options are:
> 
> -reftest
> -crashtest
> -xpcshell
> -jsreftest
> -mochitest-plain (1-5)
> -mochitest-chrome
> -mochitest-a11y
> -mochitest-ipcplugins

We should prepare to have all of these working, but to start, I think we should be prepared to run the following with 100% expected success:
-browser-chrome

Beyond that, we will likely need some for of test-subsetting. Not all of these will "just work" in Fennec and we will be flooded with permanent orange. I think we should try to get some subset of tests working for:
-reftest
-crashtest
-jsreftest
-mochitest-plain (1-5)
-mochitest-chrome

A11y and plugins are not quite ready for Mobile. Lets skip those for now.

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbotcustom v2

about to throw this on testing, uploading for those following along.

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbot-configs v2

matching configs patch with all suites enabled for testing

Comment 20

[John Ford [:jhford] CET/CEST Berlin Time]

Can someone please confirm that this is the correct command line for mobile-browser-chrome

 argv: ['python', 'mochitest/runtests.py', '--appname=fennec/fennec', '--utility-path=bin', '--extra-profile-file=bin/plugins', '--certificate-path=certs', '--autorun', '--close-when-done', '--console-level=INFO', '--test-path=mobile', '--symbols-path=symbols', '--browser-chrome']

Comment 21

[cmtalbert]

That indeed looks correct for browser-chrome and desktop fennec on linux.

Comment 22

[John Ford [:jhford] CET/CEST Berlin Time]

ok, that resulted in 

TinderboxPrint: mochitest-browser-chrome<br/>301/<em class="testfail">5</em>/16

I am going to finish cleaning this patch up and submit for review.

Should we enable browser-chrome tests with test-failures or wait for them to be green?

Comment 23

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to comment #22)

> Should we enable browser-chrome tests with test-failures or wait for them to be
> green?

I would enable with failures. The mobile team owns those and should get them green.

Comment 24

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbotcustom v3

This patch adds support for running mobile mochitests on the desktop builds of our 32bit linux fennec builds.

This has been tested and works.  jsreftests, crashtest and browserchrome (for mobile) will be enabled in the first go around.

Comment 25

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbot-configs v3

This patch enables jsreftest, crashtest and browser-chrome for fennec.  All of these suites are either green or are expected to be or become green.

Comment 26

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 487675 [details] [diff] [review]
buildbotcustom v3

@@ -6678,7 +6691,7 @@ class MozillaTestFactory(MozillaBuildFac
         pass
 
     def addTearDownSteps(self):
-        self.addCleanupSteps()
+        #self.addCleanupSteps()
         if self.buildsBeforeReboot and self.buildsBeforeReboot > 0:
             self.addPeriodicRebootSteps()

this section shouldn't be in the patch and I don't know why it was included.

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

Attachment: buildbotcustom v4

fixed patch

Comment 28

[John Ford [:jhford] CET/CEST Berlin Time]

this has been running in staging for a while and has affected non-mobile test runs.

Comment 29

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #28)
> this has been running in staging for a while and has affected non-mobile test
> runs.

that should read "has *not* affected"

Comment 30

[Chris AtLee [:catlee]]

Comment on attachment 487679 [details] [diff] [review]
buildbotcustom v4

>diff --git a/process/factory.py b/process/factory.py
>--- a/process/factory.py
>+++ b/process/factory.py
>@@ -6486,9 +6486,21 @@ def parse_sendchange_files(build, includ
> 
> class MozillaTestFactory(MozillaBuildFactory):
>     def __init__(self, platform, productName='firefox',
>-                 downloadSymbols=True, downloadTests=False, **kwargs):
>+                 downloadSymbols=True, downloadTests=False,
>+                 posixBinarySuffix='-bin', **kwargs):
>+        ''''''
>+        '''Note: the posixBinarySuffix is needed because some products (firefox)
>+        use 'firefox-bin' and some (fennec) use 'fennec' for the name of the
>+        actual application binary.  This is only applicable to posix-like
>+        systems.  Windows always uses productName.exe (firefox.exe and
>+        fennec.exe)'''

What's the purpose of the empty docstring?

>         self.platform = platform.split('-')[0]
>         self.productName = productName
>+        if not posixBinarySuffix:
>+            '''all forms of no should result in empty string'''

This should be a comment, not a docstring

r+ with those cleaned up

Comment 31

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to comment #30)
> Comment on attachment 487679 [details] [diff] [review]
> buildbotcustom v4
> 
> >diff --git a/process/factory.py b/process/factory.py
> >--- a/process/factory.py
> >+++ b/process/factory.py
> >@@ -6486,9 +6486,21 @@ def parse_sendchange_files(build, includ
> > 
> > class MozillaTestFactory(MozillaBuildFactory):
> >     def __init__(self, platform, productName='firefox',
> >-                 downloadSymbols=True, downloadTests=False, **kwargs):
> >+                 downloadSymbols=True, downloadTests=False,
> >+                 posixBinarySuffix='-bin', **kwargs):
> >+        ''''''
> >+        '''Note: the posixBinarySuffix is needed because some products (firefox)
> >+        use 'firefox-bin' and some (fennec) use 'fennec' for the name of the
> >+        actual application binary.  This is only applicable to posix-like
> >+        systems.  Windows always uses productName.exe (firefox.exe and
> >+        fennec.exe)'''
> 
> What's the purpose of the empty docstring?

I put the empty first comment there so that the clarification for posixBinarySuffix isn't the docstring for the class.  A quick check tells by that if i use # instead of triple comma quotes, it won't show as a docstring.

> 
> >         self.platform = platform.split('-')[0]
> >         self.productName = productName
> >+        if not posixBinarySuffix:
> >+            '''all forms of no should result in empty string'''
> 
> This should be a comment, not a docstring
> 
> r+ with those cleaned up

Sure, I will make these changes.  Thanks for the review :)

Comment 32

[John Ford [:jhford] CET/CEST Berlin Time]

Ready for reconfig, but I'd like to be around for the reconfig

Comment 33

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 487677 [details] [diff] [review]
buildbot-configs v3

http://hg.mozilla.org/build/buildbot-configs/rev/509c45e5b106

Comment 34

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 487679 [details] [diff] [review]
buildbotcustom v4

http://hg.mozilla.org/build/buildbotcustom/rev/ad10ae2a8e1a

Comment 35

[John Ford [:jhford] CET/CEST Berlin Time]

updated on all masters between 1823-1855 Pacific.

Comment 36

[Aki Sasaki (not active)]

Where are these reporting? Not on mobile tinderbox.