1332084 - [geckoview] Get basic testing up

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: APIs, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

We at least need some kind of smoke test, but preferably something that does functional testing of the GeckoView public API.

Comment 2

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=07a93af8f96f201fbec68026c21bc673f7554b52

Comment 3

[Geoff Brown [:gbrown]]

The Android mozharness script tries to extract 'package-name.txt' from the apk, but that file is not included in geckoview_example.apk. We use package-name.txt because the fennec app/package name varies (org.mozilla.fennec, org.mozilla.firefox, etc), but I think there are no such complications for geckoview_example, so I have bypassed the package-name processing for geckoview tests. Now I can add tests in taskcluster, start the emulator, install geckoview_example, and attempt to run tests:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=76120ed67a85e36985d78e31bfd6f6eefa642b24

Comment 4

[Geoff Brown [:gbrown]]

Attachment: allow for apk without package-name.txt

One of the problems to be solved here is making the Android mozharness script allow for other APK files, which might not have package-name.txt included. package-name.txt is required for Fennec because we use different package names for Fennec in different build contexts (notably org.mozilla.fennec vs org.mozilla.firefox). There are no such complications for geckoview_example.

For geckoview_example, we can easily configure the package name -- we don't need android_emulator_unittest.py to query the package name by extracting it from the apk. Unfortunately, the android_emulator_unittest.py tries to do that regardless of configuration. This patch corrects that short-coming: the package name is queried if and only if the configuration requests it (via the 'app' variable).

More patches to come...

Comment 5

[Geoff Brown [:gbrown]]

As discussed on irc today, geckoview_example currently crashes on startup. Work here will soon stall unless the startup crash is resolved.

Comment 6

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

Comment on attachment 8836264 [details] [diff] [review]
allow for apk without package-name.txt

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

I like simple patches

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50321440abee
Allow alternate apk in android_emulator_unittest.py; r=jmaher

Comment 8

[Carsten Book [:Tomcat]]

backed this out for breaking android tests like https://treeherder.mozilla.org/logviewer.html#?job_id=76840666&repo=mozilla-inbound

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc3d9ed47797
Backed out changeset 50321440abee for android mass test bustage on a CLOSED TREE

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Geoff Brown [:gbrown] from comment #5)
> As discussed on irc today, geckoview_example currently crashes on startup.
> Work here will soon stall unless the startup crash is resolved.

It works locally for folks, so dunno what's going on. Dylan can you see what's different about the automation builds?

Comment 11

[Geoff Brown [:gbrown]]

Sorry, needed a slight adjustment. All's well now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=900979ba040085641e0de31d950e1e2785c0aece

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ec3544ae452
Allow alternate apk in android_emulator_unittest.py; r=jmaher

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4ec3544ae452

Comment 14

[Geoff Brown [:gbrown]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #10)
> (In reply to Geoff Brown [:gbrown] from comment #5)
> > As discussed on irc today, geckoview_example currently crashes on startup.
> > Work here will soon stall unless the startup crash is resolved.
> 
> It works locally for folks, so dunno what's going on. Dylan can you see
> what's different about the automation builds?

The startup crash should be fixed now...I'm looking at tests again.

Comment 15

[Geoff Brown [:gbrown]]

Sorry, this got stuck when I realized that robocop.apk is strongly coupled with fennec - notably the package name, but also much of the code is fennec-centric. I briefly considered creating a parallel robocop-geckoview.apk, perhaps salvaging parts of our robocop code, but I am reluctant to invest further in robotium (robocop tests have been problematic wrt intermittents and maintenance, and it's not clear to me that robotium itself has a bright future).

Rather than risk further delays crafting a test harness, I'm pushing ahead with a dead simple, dumb, new "harness" that just starts geckoview_example and verifies logcat for startup messages. That's looking promising.

Comment 16

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=28234519d7084f1fef6593df1ca0daef0615af41

Comment 17

[Geoff Brown [:gbrown]]

Attachment: taskcluster changes to add new geckoview test

I changed the apk directory to make it consistent with the fennec apk. I use 'geckoview' in the test name to trigger using the geckoview apk as the test target rather than the fennec apk - I think that will be convenient.

Otherwise, this is just adding the plumbing to kick off a new 'geckoview' test job associated with gradle builds.

Comment 18

[Geoff Brown [:gbrown]]

Attachment: add mozharness configuration for geckoview test

Comment 19

[Geoff Brown [:gbrown]]

Attachment: add rungeckoview.py

Not quite ready for review yet, but coming soon...

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2652a189559a3cc9dbc78db7bf07bb96caaf57f0

Comment 20

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

Comment on attachment 8854639 [details] [diff] [review]
add mozharness configuration for geckoview test

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

::: testing/mozharness/configs/android/androidarm_4_3.py
@@ +356,5 @@
>                  "--startup-timeout=300",
>              ],
>          },
> +        "geckoview": {
> +            "run_filename": "rungeckoview.py",

we just need this file defined!

Comment 21

[Geoff Brown [:gbrown]]

Attachment: use activity for GeckoProfile

An important one-line change that unblocks me!

To check for crash minidumps, the harness needs to change the gecko profile to an unprivileged location. For Firefox, that has always been implemented by passing "-profile <dir>" on the command line. I found that passing arguments was not working for geckoview_example because, at https://dxr.mozilla.org/mozilla-central/rev/b1364675bdf5dffe63fd60373034293de0b513d5/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoProfile.java#218, the context was not an Activity. This fixes that.

Comment 22

[Geoff Brown [:gbrown]]

Attachment: add rungeckoview.py

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cf06abe357e9b7342e82537657ed30ea6d83ee29

[task 2017-04-11T01:20:41.376683Z] 01:20:41     INFO -  mozdevice Detected adb 1.0.31
[task 2017-04-11T01:20:41.560505Z] 01:20:41     INFO -  mozdevice Detected Android sdk 18
[task 2017-04-11T01:20:41.800809Z] 01:20:41     INFO -  mozdevice will use zip to push directories
[task 2017-04-11T01:20:44.338246Z] 01:20:44     INFO -  rungeckoview built profile /tmp/tmphR1cEy.mozrunner > /storage/sdcard/tests/gv-profile
[task 2017-04-11T01:20:44.339143Z] 01:20:44     INFO -  SUITE-START | Running 1 tests
[task 2017-04-11T01:20:44.339820Z] 01:20:44     INFO -  TEST-START | geckoview_example installed
[task 2017-04-11T01:20:45.857992Z] 01:20:45     INFO -  TEST-PASS | geckoview_example installed | took 1518ms
[task 2017-04-11T01:20:45.858453Z] 01:20:45     INFO -  TEST-START | geckoview_example starts
[task 2017-04-11T01:20:49.784669Z] 01:20:49     INFO -  TEST-PASS | geckoview_example starts | took 3927ms
[task 2017-04-11T01:20:49.785830Z] 01:20:49     INFO -  TEST-START | startup logcat messages
[task 2017-04-11T01:21:20.083744Z] 01:21:20     INFO -  TEST-PASS | startup logcat messages | took 30298ms
[task 2017-04-11T01:21:20.084735Z] 01:21:20     INFO -  Passed: 3
[task 2017-04-11T01:21:20.084823Z] 01:21:20     INFO -  Failed: 0
[task 2017-04-11T01:21:20.085023Z] 01:21:20     INFO -  SUITE-END | took 35s
[task 2017-04-11T01:21:23.401758Z] 01:21:23     INFO - Return code: 0
[task 2017-04-11T01:21:23.402079Z] 01:21:23     INFO - TinderboxPrint: geckoview<br/>3/0

Comment 23

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

Comment on attachment 8856785 [details] [diff] [review]
add rungeckoview.py

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

a few questions/concerns, almost r+

::: testing/mochitest/rungeckoview.py
@@ +124,5 @@
> +            "RunGecko - args = %s" % self.appname,
> +            "Displayed %s/.GeckoViewActivity" % self.appname
> +        ]
> +        # allow startup to complete before checking logcat
> +        time.sleep(30)

is 30 seconds plenty of time to wait, or is there a chance we will have a slower build someday and have many jobs taking 35 seconds to get started up and we fail?

@@ +142,5 @@
> +        """
> +           Run simple tests to verify that the geckoview_example app starts.
> +        """
> +        self.test_name = "geckoview_example"
> +        self.log.suite_start([self.test_name])

why do we set test name and log.suite_start when we do it as the first steps in the for loop?

@@ +163,5 @@
> +        self.log.info("Passed: %d" % pass_count)
> +        self.log.info("Failed: %d" % fail_count)
> +        self.log.suite_end()
> +
> +        self.test_name = "geckoview_example"

why do we reset this?

@@ +169,5 @@
> +
> +    def check_for_crashes(self):
> +        if self.logcat:
> +            if mozcrash.check_for_java_exception(self.logcat, self.test_name):
> +                return True

here we return before checking for crashdir and rmtree(dump_dir), I would think we need to do that.

Comment 24

[James Willcox (:snorp) ([e-mail]) (he/him)]

Yessssssssssss

Comment 25

[Geoff Brown [:gbrown]]

(In reply to Joel Maher ( :jmaher) from comment #23)
> @@ +169,5 @@
> > +
> > +    def check_for_crashes(self):
> > +        if self.logcat:
> > +            if mozcrash.check_for_java_exception(self.logcat, self.test_name):
> > +                return True
> 
> here we return before checking for crashdir and rmtree(dump_dir), I would
> think we need to do that.

If check_for_java_exception() returns True, it reports an exception in Java code. In that case, there is probably a minidump and we could create a crash report, but experience shows that doing so just causes confusion: The important thing is to report the exception; we want to avoid the crash report or any other diagnostics. The rmtree is in a finally clause and will be executed.

Comment 26

[Geoff Brown [:gbrown]]

Attachment: add rungeckoview.py

Comment 27

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

Comment on attachment 8856982 [details] [diff] [review]
add rungeckoview.py

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

looks good

Comment 28

[Geoff Brown [:gbrown]]

Comment on attachment 8856783 [details] [diff] [review]
use activity for GeckoProfile

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

r+ via irc

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac1495e2252b
Add smoketest for geckoview_example - add harness; r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a0d50a7ff6d
Pass correct activity to GeckoProfile for geckoview_example; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b34a5303e91
Add smoketest for geckoview_example - mozharness changes; r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/649702f31585
Add smoketest for geckoview_example - taskcluster changes for new suite; r=jmaher

Comment 30

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/ac1495e2252b
https://hg.mozilla.org/mozilla-central/rev/2a0d50a7ff6d
https://hg.mozilla.org/mozilla-central/rev/0b34a5303e91
https://hg.mozilla.org/mozilla-central/rev/649702f31585