841314 - alive tests should use make target

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

we have the alive tests and they use our common python libraries.  In order to move forward, we need to use the objdir virtualenv which is defined by $(PYTHON) in the makefile land.  We have added a 'make leaktest' option and we should use this on firefox 21 and greater.

Comment 1

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

Attachment: idea of what we could do to utilize 'make leaktest'

This patch won't work as there is not a self.version, but if there was this would solve our problem.  Maybe there is a better method for doing this and I am not aware of it.

Comment 2

[Armen [:armenzg]]

Comment on attachment 713852 [details] [diff] [review]
idea of what we could do to utilize 'make leaktest'

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

Unfortunately, we don't use version values and be explicit about enabling features. Long term will be to move to mozharness.

Meanwhile, I believe that this is the way to pass a flag along:
* add the flag in here http://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/config.py#l11
* add it here as config["blah"] http://hg.mozilla.org/build/buildbotcustom/file/default/misc.py#l1138
* if you need the nightly builds to do the same in here:
** http://hg.mozilla.org/build/buildbotcustom/file/default/misc.py#l1376
* add it here with False if not set : http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l762
* set it here http://hg.mozilla.org/build/buildbotcustom/file/default/process/factory.py#l816

I'm sorry this is like this :(
I wish it wasn't.

Thanks for your help! I can help test it or ensure that other pieces are not messed up.
I can't wait to move to another realm.

Comment 3

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

Would you want me to continue on this path by keying off of a version, or go another route?

Comment 4

[Armen [:armenzg]]

(In reply to Joel Maher (:jmaher) from comment #3)
> Would you want me to continue on this path by keying off of a version, or go
> another route?

Using a flag like runMakeAliveTests (for example). Thanks!

Comment 5

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

Attachment: builbotcustom patch to add runMakeAliveTest (2.0)

Comment 6

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

Attachment: buildbot-config patch to use runMakeAliveTest (1.0)

Comment 7

[Armen [:armenzg]]

I will review this in the morning.
I got derailed with merge day mechanics.

Comment 8

[Armen [:armenzg]]

Attachment: builbotcustom patch to add runMakeAliveTest

Comment 9

[Armen [:armenzg]]

Attachment: buildbot-configs patch to use runMakeAliveTest

Comment 10

[Armen [:armenzg]]

Attachment: differences for a build master

This patch shows how the leak builders for mozilla-central get modified.

Comment 11

[Aki Sasaki (not active)]

Comment on attachment 716102 [details] [diff] [review]
buildbot-configs patch to use runMakeAliveTest

Is there a reason you're creating a merge day loop and then commenting it out and not using it?

I would guess we'd want this for all Firefox 22 based branches, which means we should turn it on by default and uncomment the loop.

If you're not sure if this'll work, we should test this in staging rather than turning it on for just m-c.

r=me for using the loop and setting run_make_alive_tests to True by default.

Comment 12

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

Comment on attachment 716102 [details] [diff] [review]
buildbot-configs patch to use runMakeAliveTest

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

::: mozilla/config.py
@@ +1846,5 @@
> +# When Firefox 22 merges into these branches, they can be removed from the list
> +#for b in in ('mozilla-aurora', 'mozilla-beta', 'mozilla-release', 'mozilla-esr17',
> +#             'mozilla-b2g18', 'mozilla-b2g18_v1_0_0', 'mozilla-b2g18_v1_0_1'):
> +#    BRANCHES[b]["run_make_alive_tests"] = False
> +BRANCHES["mozilla-central"]["run_make_alive_tests"] = True

will this work for inbound and other m-c sibling branches?

Comment 13

[Armen [:armenzg]]

Attachment: make alive failures

We got this failure in the step.

Do you want to borrow the build machine?

Comment 14

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

give me access to the machine and I will fiddle with the right settings.

This appears to be the root of the problem:
\'LEAKTEST_ARGS=" -- [\'"\'"\'-register\'"\'"\'] "\' make leaktest'

it should be:
LEAKTEST_ARGS="-- -register" make leaktest

Comment 15

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

the --cwd is set to objdir/_leaktest (as it is for the normal case), but for the makefile, we need to either set the --cwd to objdir, or 'make -C .. leaktest'

Comment 16

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

in buildbotcustom, we want to do:

+class AliveMakeTest(ShellCommand):
+    name = "alive test"
+    description = ["alive test"]
+    haltOnFailure = True
+    flunkOnFailure = False
+    warnOnFailure = True
+
+    def __init__(self, extraArgs=None, logfile=None, **kwargs):
+        self.super_class = ShellCommand
+        self.super_class.__init__(self, **kwargs)
+
+        self.addFactoryArguments(extraArgs=extraArgs,
+                                 logfile=logfile)
+        self.extraArgs = extraArgs
+        self.logfile = logfile
+
+        # build the command
+        leakargs = ''
+        if extraArgs:
+            leakargs += ' -- %s ' % ' '.join(extraArgs)

^ note the ' '.join(extraArgs)


+        if logfile:
+            leakargs += ' -l %s ' % logfile
+        self.command = ['LEAKTEST_ARGS="%s"' % leakargs, 'make', '-C', '..', 'leaktest']
+

^ note the added -C .., to the make leaktest command.

Comment 17

[Armen [:armenzg]]

You can follow the working job in http://dev-master01.build.scl1.mozilla.com:8040/builders/Linux%20x86-64%20mozilla-central%20leak%20test%20build/builds/9

So far the make alive step is running.

Comment 18

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

very small tweak to what I believe is the command line.

this is what I see in the log:
mock_mozilla -v -r mozilla-centos6-x86_64 --cwd /builds/slave/m-cen-l64-d-000000000000000000/build/obj-firefox/_leaktest --unpriv --shell '/usr/bin/env HG_SHARE_BASE_DIR="/builds/hg-shared" XPCOM_DEBUG_BREAK="stack-and-abort" MOZ_CRASHREPORTER_NO_REPORT="1" CCACHE_BASEDIR="/builds/slave/m-cen-l64-d-000000000000000000" MOZ_OBJDIR="obj-firefox" LD_LIBRARY_PATH="/tools/gcc-4.3.3/installed/lib64:obj-firefox/dist/bin" PATH="/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin" CCACHE_UMASK="002" CCACHE_DIR="/builds/ccache" LC_ALL="C" CCACHE_COMPRESS="1" MINIDUMP_STACKWALK="/builds/slave/m-cen-l64-d-000000000000000000/tools/breakpad/linux64/minidump_stackwalk" DISPLAY=":2" MINIDUMP_SAVE_PATH="/builds/slave/m-cen-l64-d-000000000000000000/minidumps" '"'"'LEAKTEST_ARGS=" -- -register "'"'"' make -C .. leaktest'


this is what I use to run the command:
mock_mozilla -v -r mozilla-centos6-x86_64 --cwd /builds/slave/m-cen-l64-d-000000000000000000/build/obj-firefox/_leaktest --unpriv --shell '/usr/bin/env HG_SHARE_BASE_DIR="/builds/hg-shared" XPCOM_DEBUG_BREAK="stack-and-abort" MOZ_CRASHREPORTER_NO_REPORT="1" DISPLAY=":2" LD_LIBRARY_PATH="/tools/gcc-4.3.3/installed/lib64:obj-firefox/dist/bin" CCACHE_BASEDIR="/builds/slave/m-cen-l64-d-000000000000000000" MOZ_OBJDIR="obj-firefox" CCACHE_UMASK="002" CCACHE_DIR="/builds/ccache" LC_ALL="C" CCACHE_COMPRESS="1" PATH="/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin" MINIDUMP_STACKWALK="/builds/slave/m-cen-l64-d-000000000000000000/tools/breakpad/linux64/minidump_stackwalk" MINIDUMP_SAVE_PATH="/builds/slave/m-cen-l64-d-000000000000000000/minidumps" LEAKTEST_ARGS=" -- -register " make -C .. leaktest'



The difference is the last bit here, we need to make this:
'"'"'LEAKTEST_ARGS=" -- -register "'"'"' make -C .. leaktest'


look like:
LEAKTEST_ARGS=" -- -register " make -C .. leaktest'


I am not sure what the patch we have uploaded is, but I assume removing a set of quotes or two will solve all our problems.

Comment 19

[Armen [:armenzg]]

I adjusted the patch.
I will be rebuilding until we get it right.

Comment 20

[Armen [:armenzg]]

Attachment: buildbotcustom patch to use runMakeAliveTest

Lists can be evil.
We switched self.command to be a string instead.

After a success on Linux I decided to do a end to end testing for both Linux and Windows just to be 100% before landing.

Comment 21

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

Comment on attachment 718427 [details] [diff] [review]
buildbotcustom patch to use runMakeAliveTest

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

this one addition should fix it!

::: steps/test.py
@@ +92,5 @@
> +            leakargs.extend(extraArgs)
> +        if logfile:
> +            leakargs.append('-l')
> +            leakargs.append(logfile)
> +        self.command = 'LEAKTEST_ARGS="%s" make -C .. leaktest' % leakargs

self.command = 'LEAKTEST_ARGS="%s" make -C .. leaktest' % ' '.join(leakargs)

Comment 22

[Armen [:armenzg]]

(In reply to Joel Maher (:jmaher) from comment #21)
> > +        self.command = 'LEAKTEST_ARGS="%s" make -C .. leaktest' % leakargs
> 
> self.command = 'LEAKTEST_ARGS="%s" make -C .. leaktest' % ' '.join(leakargs)

True. This is needed and it has successfully completed.

Comment 23

[Aki Sasaki (not active)]

Comment on attachment 718427 [details] [diff] [review]
buildbotcustom patch to use runMakeAliveTest

r=me if you include the fix in comment 21.

Comment 24

[Armen [:armenzg]]

Attachment: Windows alive log

Windows fails :(

LEAKTEST_ARGS="-- -register" make -C .. leaktest
 in dir e:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build/obj-firefox/_leaktest (timeout 1200 secs)
...
'LEAKTEST_ARGS' is not recognized as an internal or external command,
operable program or batch file.

Comment 25

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

hmm, I am confused by this.  Is it not in quotes as part of the shell args to setup environment variables?  I could debug on a windows slave if needed.

Comment 26

[Armen [:armenzg]]

I'm trying this:
        cmd = "LEAKTEST_ARGS='%s' make -C .. leaktest" % ' '.join(leakargs)
        self.command = 'bash -c "%s"' % cmd
I probably could have made to be one line (I was fiddling with the single vs double quotes scenarios).

Comment 27

[Armen [:armenzg]]

jmaher says that this works on the Windows machine:
bash -c "LEAKTEST_ARGS='-- -register' python /e/builds/moz2_slave/m-cen-w32-d-000000000000000/build/build/pymake/make.py leaktest"

I'm now testing this:
        self.command = ['bash', '-c', \
            '%s' % "LEAKTEST_ARGS='%s' python %s/build/pymake/make.py leaktest" % \
            (' '.join(leakargs), os.path.join(os.getcwd(), "../.."))]
I think I should make that line of code cleaner :/

Comment 28

[Armen [:armenzg]]

Attachment: Linux failure

Any idea on why this is failing?

Windows also fails in a similar manner:
'bash' '-c' "LEAKTEST_ARGS='-- -register' python ../../build/pymake/make.py leaktest"
...
make.py[0]: Entering directory 'e:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\obj-firefox\_leaktest'
No makefile found

Comment 29

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

the path to make.py needs to be a real path.  Can you find what path is used for 'make package-tests'?

Comment 30

[(not currently active) Ted Mielczarek]

It should just be the object directory.

I'm starting to wonder if implementing this as a mach command wouldn't have been cleaner...

Comment 31

[Armen [:armenzg]]

I will be back to this tomorrow morning.

Comment 32

[Armen [:armenzg]]

I'm trying to use this without luck:
        self.command = WithProperties(
            "bash -c \"LEAKTEST_ARGS='" + ' '.join(leakargs) + "' python %(basedir)s/build/pymake/make.py leaktest\""
        )

mock_mozilla -v -r mozilla-centos6-x86_64 --cwd /builds/slave/m-cen-l64-d-000000000000000000/build/obj-firefox/_leaktest --unpriv --shell '/usr/bin/env HG_SHARE_BASE_DIR="/builds/hg-shared" XPCOM_DEBUG_BREAK="stack-and-abort" MOZ_CRASHREPORTER_NO_REPORT="1" CCACHE_BASEDIR="/builds/slave/m-cen-l64-d-000000000000000000" MOZ_OBJDIR="obj-firefox" LD_LIBRARY_PATH="/tools/gcc-4.3.3/installed/lib64:obj-firefox/dist/bin" PATH="/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin" CCACHE_UMASK="002" CCACHE_DIR="/builds/ccache" LC_ALL="C" CCACHE_COMPRESS="1" MINIDUMP_STACKWALK="/builds/slave/m-cen-l64-d-000000000000000000/tools/breakpad/linux64/minidump_stackwalk" DISPLAY=":2" MINIDUMP_SAVE_PATH="/builds/slave/m-cen-l64-d-000000000000000000/minidumps" bash -c "LEAKTEST_ARGS='"'"'-- -register'"'"' python /builds/slave/m-cen-l64-d-000000000000000000/build/pymake/make.py leaktest"'

Comment 33

[Armen [:armenzg]]

Path seems wrong (I will try again):
python /builds/slave/m-cen-l64-d-000000000000000000/build/pymake/make.py leaktest"'
python /builds/slave/m-cen-w32-d-000000000000000/build/build/pymake/make.py leaktest"

Comment 34

[Armen [:armenzg]]

(In reply to Joel Maher (:jmaher) from comment #15)
> the --cwd is set to objdir/_leaktest (as it is for the normal case), but for
> the makefile, we need to either set the --cwd to objdir, or 'make -C ..
> leaktest'
>
According to this comment, what should I modify from below?
Should I try without "_leaktest" on the working directory?

The workdir is this:
/builds/slave/m-cen-l64-d-000000000000000000/build/obj-firefox/_leaktest

The command run is this:
bash -c "LEAKTEST_ARGS='"'"'-- -register'"'"' python /builds/slave/m-cen-l64-d-000000000000000000/build/build/pymake/make.py leaktest"'

mock_mozilla -v -r mozilla-centos6-x86_64 --cwd /builds/slave/m-cen-l64-d-000000000000000000/build/obj-firefox/_leaktest --unpriv --shell '/usr/bin/env HG_SHARE_BASE_DIR="/builds/hg-shared" XPCOM_DEBUG_BREAK="stack-and-abort" MOZ_CRASHREPORTER_NO_REPORT="1" CCACHE_BASEDIR="/builds/slave/m-cen-l64-d-000000000000000000" MOZ_OBJDIR="obj-firefox" LD_LIBRARY_PATH="/tools/gcc-4.3.3/installed/lib64:obj-firefox/dist/bin" PATH="/tools/buildbot/bin:/usr/local/bin:/usr/lib64/ccache:/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/sbin:/tools/git/bin:/tools/python27/bin:/tools/python27-mercurial/bin:/home/cltbld/bin" CCACHE_UMASK="002" CCACHE_DIR="/builds/ccache" LC_ALL="C" CCACHE_COMPRESS="1" MINIDUMP_STACKWALK="/builds/slave/m-cen-l64-d-000000000000000000/tools/breakpad/linux64/minidump_stackwalk" DISPLAY=":2" MINIDUMP_SAVE_PATH="/builds/slave/m-cen-l64-d-000000000000000000/minidumps" bash -c "LEAKTEST_ARGS='"'"'-- -register'"'"' python /builds/slave/m-cen-l64-d-000000000000000000/build/build/pymake/make.py leaktest"'

Comment 35

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

if we are in obj-firefox/_leaktest, we need to do a 'make -C .. leaktest', otherwise be in the obj-firefox/ dir for a regular 'make leaktest'

Comment 36

[Armen [:armenzg]]

Attachment: buildbotcustom patch to use runMakeAliveTest

This has worked for Linux.

I'm running it now for Windows.

Comment 37

[Armen [:armenzg]]

:(

(In reply to Armen Zambrano G. [:armenzg] from comment #27)
> jmaher says that this works on the Windows machine:
> bash -c "LEAKTEST_ARGS='-- -register' python
> /e/builds/moz2_slave/m-cen-w32-d-000000000000000/build/build/pymake/make.py
> leaktest"
> 
This is what I get:
bash -c "LEAKTEST_ARGS='-- -register' python e:/builds/moz2_slave/m-cen-w32-d-000000000000000/build/build/pymake/make.py leaktest"

s|/e/builds|e:\builds|

Comment 38

[Armen [:armenzg]]

Both of these commands work if I run them manually (from E:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\obj-firefox) but not through buildbot:

bash -c "LEAKTEST_ARGS='-- -register' python /e/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/build/pymake/make.py leaktest"
bash -c "LEAKTEST_ARGS='-- -register' python e:/builds/moz2_slave/m-cen-w32-d-000000000000000000/build/build/pymake/make.py leaktest"

They only work if I start it through start-l10n.bat and not cmd.exe
If I use cmd.exe they will only say:
> make.py[0]: Entering directory 'E:\builds\moz2_slave\m-cen-w32-d-000000000000000
000\build\obj-firefox'
and start there forever.

Comment 39

[Armen [:armenzg]]

Attachment: alive configs

Comment 40

[Armen [:armenzg]]

Attachment: alive custom

Comment 41

[Armen [:armenzg]]

I'm doing a clean last run and hopefully both Linux and Windows should pass.

What solved the last issues was switching self.command to be a list rather than a string.

Comment 42

[Armen [:armenzg]]

The jobs are still running. I will check after dinner.

Comment 43

[Armen [:armenzg]]

It looks good on staging for Linux and Windows.

Comment 44

[Armen [:armenzg]]

Comment on attachment 721429 [details] [diff] [review]
alive configs

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

Comment 45

[Armen [:armenzg]]

Comment on attachment 721430 [details] [diff] [review]
alive custom

http://hg.mozilla.org/build/buildbotcustom/rev/225682acb2a8

Comment 46

[Armen [:armenzg]]

Merged and reconfiguration completed.

Comment 47

[Armen [:armenzg]]

Attachment: disable make alive on birch since it tracks mozilla-beta

Comment 48

[Phil Ringnalda (:philor)]

Comment on attachment 723952 [details] [diff] [review]
disable make alive on birch since it tracks mozilla-beta

r=me with the comment saying to just take it out on the next merge, since either per-window privatebrowsing ships then and birch is done with that, or it gets kicked out clear back to something that does know about make leaktest

Comment 49

[Armen [:armenzg]]

Comment on attachment 723952 [details] [diff] [review]
disable make alive on birch since it tracks mozilla-beta

4408e8bc89b5

Comment 50

[Aki Sasaki (not active)]

This is in production.