916350 - Make it possible to run B2G reftests on B2G desktop builds

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Jonathan Griffin (:jgriffin)]

It's been decided that we want to run reftests on B2G desktop builds in TBPL, as well as emulators.  We'll need to update the test harness to make this possible.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 (mozbase) - changes needed in mozbase for b2g desktop reftests

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 (m-c) - add ability to run reftest on b2g desktop

Comment 3

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8344800 [details] [diff] [review]
Patch 1.0 (m-c) - add ability to run reftest on b2g desktop

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

::: layout/tools/reftest/b2g_desktop.py
@@ +47,5 @@
> +        if not reftestlist.startswith('file://'):
> +            reftestlist = 'file://%s' % reftestlist
> +
> +        self.profile = self.create_profile(options, reftestlist,
> +                                      profile_to_clone=options.profile)

nit: weird indenting

@@ +53,5 @@
> +        kp_kwargs = { 'processOutputLine': [self._on_output],
> +                      'onTimeout': [self._on_timeout],
> +                      'kill_on_timeout': False }
> +
> +        if not options.debugger and options.autorun:

there's no 'autorun' for reftests, is there?

@@ +65,5 @@
> +        log.info("%s | Running tests: start.", os.path.basename(__file__))
> +        cmd, args = self.build_command_line(options.app,
> +                            ignore_window_size=options.ignoreWindowSize)
> +        self.runner = FirefoxRunner(profile=self.profile,
> +                               binary=cmd,

nit: weird indenting

@@ +142,5 @@
> +        log.testFail(msg % (self.last_test, self.timeout))
> +
> +        # kill process and get stack
> +        try:
> +            os.killpg(self.runner.process_handler.pid, signal.SIGABRT)

This definitely won't work on Windows, which isn't relevant right now but may be in the future.  Can we use mozprocess' kill implementation instead?

Comment 4

[Andrew Halberstadt [:ahal]]

https://github.com/mozilla/mozbase/commit/4b86b177751d9739cbc8811c1d8e4278d82067ec

I'll need to wait for mozbase to get synced to m-c before landing the m-c bits.

Comment 5

[Andrew Halberstadt [:ahal]]

(In reply to Jonathan Griffin (:jgriffin) from comment #3)
> @@ +142,5 @@
> > +        log.testFail(msg % (self.last_test, self.timeout))
> > +
> > +        # kill process and get stack
> > +        try:
> > +            os.killpg(self.runner.process_handler.pid, signal.SIGABRT)
> 
> This definitely won't work on Windows, which isn't relevant right now but
> may be in the future.  Can we use mozprocess' kill implementation instead?

Actually I forgot to update this patch with a change I had been planning. Instead I landed https://github.com/mozilla/mozbase/commit/50bcb04fe88b053b16fa97833c252be439bed15e and replaced the try block with mozrunner.stop(sig=signal.SIGABRT).

This still won't work with windows, but I think the killAndGetStack functionality should live in mozrunner. We can add windows support for killAndGetStack to mozrunner later.

Comment 6

[Andrew Halberstadt [:ahal]]

(to clarify my last comment)

mozprocess' kill implementation won't generate a stack on windows, but the kill itself _will_. We should add the stack capability in.

Comment 7

[(not currently active) Ted Mielczarek]

kill on Windows will not generate a stack. This is why we have this sort of thing:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#666

Comment 8

[(not currently active) Ted Mielczarek]

(mozprocess' kill won't either, to clarify, it will kill the process but no minidump will be generated.)

Comment 9

[Andrew Halberstadt [:ahal]]

Yeah that's what I meant. We should use crashinject.exe somewhere in mozbase or somewhere that can be shared between harnesses.

Comment 10

[(not currently active) Ted Mielczarek]

Ideally bug 890026.

Comment 11

[Andrew Halberstadt [:ahal]]

Quick update, this patch is failing on Cedar: https://tbpl.mozilla.org/php/getParsedLog.php?id=32438814&tree=Cedar

It works locally even when using Cedar. This output precedes the timeout: 
System JS : ERROR chrome://reftest/content/reftest.jsm:393 - TypeError: CC['@mozilla.org/server/jshttp;1'] is undefined
System JS : ERROR chrome://reftest/content/reftest-content.js:110 - TypeError: initInfo is null

Comment 12

[Jonathan Griffin (:jgriffin)]

Earlier in the log I also see

System JS : ERROR chrome://satchel/content/formSubmitListener.js:15 - ReferenceError: XPCOMUtils is not defined

which seems equally strange.

Comment 13

[Andrew Halberstadt [:ahal]]

That may or may not be causing the jshttpd type error, but the odd thing to me is that that line should never even be executed. On B2G desktop we pass in reftest.remote=False which means we shouldn't even run that line: http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/reftest.js#394

I've verified that the prefs are set up correctly locally (and indeed it works) but maybe something is changing that pref when run in production?? I'll start doing some debug commits to cedar to try and figure out what's up.

Comment 14

[Andrew Halberstadt [:ahal]]

Ignore that last comment. Also, I noticed the 32bit log has the same failure but not the "formSubmitListener" error in it: https://tbpl.mozilla.org/php/getParsedLog.php?id=32438772&tree=Cedar&full=1#error0

So I think comment 12 is incidental.

Comment 15

[Andrew Halberstadt [:ahal]]

According to [1] and [2], not all Gecko applications necessarily implement autoregistration of XPCOM components. Firefox watches the profile/components directory for new components and then autoregisters them. I believe what is happening here is that b2g desktop does not implement autoregistration while the Makefile for the reftest extension assumes there is autoregistration, and therefore CC['@mozilla.org/server/jshttp;1'] is undefined.

Assuming I'm correct, then the solution is either:
A) make b2g_desktop autoregister components
B) make reftest use httpd.js as an inline script
C) register the component manually with regxpcom or nsIComponentRegistrar as described in [2]

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/test/httpserver/README?force=1#22
[2] https://developer.mozilla.org/en-US/docs/Creating_XPCOM_Components/Component_Internals#Registration_Methods_in_XPCOM

Comment 16

[Andrew Halberstadt [:ahal]]

Note this doesn't explain to me why this is working locally for me.. so it's just a theory at this point.

Comment 17

[(not currently active) Ted Mielczarek]

By "locally" do you mean "from an objdir of a local B2G desktop build" or "using a downloaded B2G desktop build + test package"?

Comment 18

[Andrew Halberstadt [:ahal]]

Locally built, but yes good point, downloading the build would be a good thing to try next :p

Comment 19

[(not currently active) Ted Mielczarek]

I ask because it's certainly possible that a difference between packaged builds and objdirs is responsible.

Comment 20

[Andrew Halberstadt [:ahal]]

Yeah, you're right, I can reproduce with a downloaded build. I believe the downloaded build is using this mozconfig:
http://mxr.mozilla.org/mozilla-central/source/b2g/config/mozconfigs/linux64_gecko/nightly

My mozconfig is:

. "$topsrcdir/b2g/config/mozconfigs/common"

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/objdirs/b2g_desktop
mk_add_options MOZ_MAKE_FLAGS="-j9 -s"

ac_add_options --enable-application=b2g
ac_add_options --disable-libjpeg-turbo
 
# This option is required if you want to be able to run Gaia's tests
ac_add_options --enable-tests

# turn on mozTelephony/mozSms interfaces
# Only turn this line on if you actually have a dev phone
# you want to forward to. If you get crashes at startup,
# make sure this line is commented.
#ac_add_options --enable-b2g-ril

ENABLE_MARIONETTE=1

Comment 21

[(not currently active) Ted Mielczarek]

Okay, then your analysis from comment 15 is probably spot-on. Running from an objdir, httpd.js is installed into dist/bin/components (along with httpd.manifest). When packaged, that's no longer there and we rely on packaging it into the reftest extension:
http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/Makefile.in#38
http://mxr.mozilla.org/mozilla-central/source/netwerk/test/httpserver/jar.mn

I'm fuzzy on how reftest works on B2G, but I suspect that we don't have real extension support there and therefore not everything works properly.

Comment 22

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.1 (m-c) - manually register reftest extension's components if needed

With this patch I was able to get the downloaded build working. I'll test it out on cedar before asking review though.

Comment 23

[Andrew Halberstadt [:ahal]]

Comment on attachment 8356732 [details] [diff] [review]
Patch 1.1 (m-c) - manually register reftest extension's components if needed

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

This got them unstuck on Cedar.. still other problems, but at least they are running now.

Comment 24

[(not currently active) Ted Mielczarek]

Comment on attachment 8356732 [details] [diff] [review]
Patch 1.1 (m-c) - manually register reftest extension's components if needed

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

::: layout/tools/reftest/reftest.js
@@ +394,5 @@
>      if (gRemote) {
>          gServer = null;
>      } else {
> +        // not all gecko applications autoregister xpcom components
> +        if (CC["@mozilla.org/server/jshttp;1"] === undefined) {

You could also write !("@mozilla.org/server/jshttp;1" in CC), but that's a little awkward. (Wish JS had Python's "not in" operator!)

Comment 25

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 (m-c) (rebased) - add ability to run reftest on b2g desktop

There was a fair amount of bitrot, so rebased and pushed a new try run:
https://tbpl.mozilla.org/?tree=Try&rev=2bfd09f07ab8

Carrying r+ forward.

Comment 26

[Andrew Halberstadt [:ahal]]

None of these patches really make sense on their own, so I qfolded the three together:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c24dc80aa00

Comment 27

[Nathan Froyd [:froydnj]]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/4a0a369d7ffb for busting Android reftest/crashtests:

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

Comment 28

[Andrew Halberstadt [:ahal]]

Hmm, try -p android -u reftest doesn't run reftests on Android? Didn't notice it didn't get run, sorry.

Comment 29

[Andrew Halberstadt [:ahal]]

I think this is just because I moved mozprofile above the sys.path.insert. New try run with proper syntax:
https://tbpl.mozilla.org/?tree=Try&rev=6c7dba6be09d

I noticed bug 664857 was filed to create a 'reftest' alias in the try syntax that runs all reftest chunks on Android, though it was resolved without being fixed. I re-opened it because I think this is tripping up a lot of people.

Comment 30

[Andrew Halberstadt [:ahal]]

Attachment: Patch 2.0 - (m-c) - add ability to run reftest on b2g desktop

This try run is green (there were a few new references to _automation I needed to update):
https://tbpl.mozilla.org/?tree=Try&rev=87eb0c601405

Carrying r+ forward

Comment 31

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/31ef6f1090fd

Comment 32

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out again in http://hg.mozilla.org/integration/mozilla-inbound/rev/91ad8c446e55

for the same failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=33356831&tree=Mozilla-Inbound

Comment 33

[Andrew Halberstadt [:ahal]]

That's what I get for not waiting for all try jobs to finish :(.. this is strange since the "import mozprofile" existed before my patch, and now that it is also on the same side of the sys.path.insert(), I can't see what could cause this.

Comment 34

[Andrew Halberstadt [:ahal]]

Attachment: Patch 3.0 (m-c) - Make it possible to run b2g desktop reftests

It turns out automation.py has a hack that makes importing mozbase work, so the import just needed to be a little lower:
http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#29

Here's an actually green try run to prove it:
https://tbpl.mozilla.org/?tree=Try&rev=aa6e3de1d6ac

I updated the fix, folded the register patch into the main one and carried r+ forward.

Comment 35

[Andrew Halberstadt [:ahal]]

Comment on attachment 8365079 [details] [diff] [review]
Patch 3.0 (m-c) - Make it possible to run b2g desktop reftests

https://hg.mozilla.org/integration/mozilla-inbound/rev/1626ce65aaf1

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1626ce65aaf1

Comment 37

[Andrew Halberstadt [:ahal]]

There's a missing hunk that's on Cedar but not m-c which is causing the jobs to fail. I must have messed up rebasing my patch on top of the parallel reftest change. Bustage fix coming shortly.

Comment 38

[Andrew Halberstadt [:ahal]]

Attachment: Patch 3.1 - missing hunk bustage fix

Ryan has graciously offered to land this directly to get it spread across branches more quickly. Thanks Ryan!

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/07021aa58289