Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[geckoview] Get basic testing up

RESOLVED FIXED in Firefox 55

Status

()

Core
Embedding: APIs
RESOLVED FIXED
6 months ago
3 months ago

People

(Reporter: snorp, Assigned: gbrown)

Tracking

(Blocks: 1 bug)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(5 attachments, 2 obsolete attachments)

We at least need some kind of smoke test, but preferably something that does functional testing of the GeckoView public API.
Duplicate of this bug: 921664
Assignee: nobody → gbrown
Depends on: 1337523
https://treeherder.mozilla.org/#/jobs?repo=try&revision=07a93af8f96f201fbec68026c21bc673f7554b52
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
Created attachment 8836264 [details] [diff] [review]
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...
Attachment #8836264 - Flags: review?(jmaher)
As discussed on irc today, geckoview_example currently crashes on startup. Work here will soon stall unless the startup crash is resolved.
Flags: needinfo?(snorp)
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
Attachment #8836264 - Flags: review?(jmaher) → review+

Comment 7

5 months ago
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/50321440abee
Allow alternate apk in android_emulator_unittest.py; r=jmaher
Keywords: leave-open
backed this out for breaking android tests like https://treeherder.mozilla.org/logviewer.html#?job_id=76840666&repo=mozilla-inbound
Flags: needinfo?(gbrown)

Comment 9

5 months ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc3d9ed47797
Backed out changeset 50321440abee for android mass test bustage on a CLOSED TREE
(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?
Flags: needinfo?(snorp)
Sorry, needed a slight adjustment. All's well now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=900979ba040085641e0de31d950e1e2785c0aece
Flags: needinfo?(gbrown)

Comment 12

5 months ago
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ec3544ae452
Allow alternate apk in android_emulator_unittest.py; r=jmaher

Comment 13

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4ec3544ae452
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
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.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=28234519d7084f1fef6593df1ca0daef0615af41
Created attachment 8854638 [details] [diff] [review]
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.
Attachment #8854638 - Flags: review?(jmaher)
Created attachment 8854639 [details] [diff] [review]
add mozharness configuration for geckoview test
Attachment #8854639 - Flags: review?(jmaher)
Created attachment 8854640 [details] [diff] [review]
add rungeckoview.py

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2652a189559a3cc9dbc78db7bf07bb96caaf57f0
Attachment #8854638 - Flags: review?(jmaher) → review+
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!
Attachment #8854639 - Flags: review?(jmaher) → review+
Created attachment 8856783 [details] [diff] [review]
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.
Attachment #8856783 - Flags: review?(snorp)
Created attachment 8856785 [details] [diff] [review]
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
Attachment #8854640 - Attachment is obsolete: true
Attachment #8856785 - Flags: review?(jmaher)
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.
Attachment #8856785 - Flags: review?(jmaher) → review-
Yessssssssssss
(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.
Created attachment 8856982 [details] [diff] [review]
add rungeckoview.py
Attachment #8856785 - Attachment is obsolete: true
Attachment #8856982 - Flags: review?(jmaher)
Comment on attachment 8856982 [details] [diff] [review]
add rungeckoview.py

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

looks good
Attachment #8856982 - Flags: review?(jmaher) → review+
Comment on attachment 8856783 [details] [diff] [review]
use activity for GeckoProfile

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

r+ via irc
Attachment #8856783 - Flags: review?(snorp) → review+

Comment 29

3 months ago
Pushed by gbrown@mozilla.com:
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
Keywords: leave-open

Comment 30

3 months ago
bugherder
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
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox53: affected → ---
You need to log in before you can comment on or make changes to this bug.