Closed Bug 1318091 Opened 8 years ago Closed 5 years ago

Run GTest on Android x86_64

Categories

(GeckoView :: General, enhancement, P2)

Product:
GeckoView
Component:
General
Platform:
x86_64
Android
Type:
enhancement

Tracking

(firefox68 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: gbrown, Assigned: gbrown)

References

(Depends on 1 open bug)

Details

Attachments

(8 files)

Bug 1318091 - Disable failing android gtests; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Support logging to file in mozilla gtest; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Support env override of gtest minidump location; r=Ehsan
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Add remotegtests.py, supporting android gtests; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Skip jemalloc gtests on android; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Add Android 7.0 gtest tasks; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Add more options to remotegtests.py; r=bc
47 bytes, text/x-phabricator-request
Details | Review
Bug 1318091 - Support |mach gtest| for android; r=bc
47 bytes, text/x-phabricator-request
Details | Review 
I don't think we've every tried to run GTest on Android. Let's see what happens.
Priority: -- → P2
Sorry, never made much progress on this.
Assignee: gbrown → nobody
Priority: P2 → P3
Geoff, when I try to run gtest from the command line that's what I'm getting:

> dexter@ubuntu:~/mozilla-unified$ ./mach gtest Telemetry*
>  0:00.17 /usr/bin/make -C testing/gtest -j4 -s gtest
> Error running mach:
> 
>     ['gtest', 'Telemetry*']
> 
> The error occurred in code that was called by the mach command. This is either
> a bug in the called code itself or in the way that mach is calling it.
> 
> You should consider filing a bug for this issue.
> 
> If filing a bug, please include the full output of mach, including this error
> message.
> 
> The details of the failure are as follows:
> 
> Exception: Binary expected at /home/dexter/mozilla-unified/obj-arm-linux-androideabi/dist/bin/fennec does not exist.
> 
>   File "/home/dexter/mozilla-unified/python/mozbuild/mozbuild/mach_commands.py", line 598, in gtest
>     app_path = self.get_binary_path('app')
>   File "/home/dexter/mozilla-unified/python/mozbuild/mozbuild/base.py", line 454, in get_binary_path
>     raise Exception('Binary expected at %s does not exist.' % path)


Any clue on how to get this started?
Flags: needinfo?(gbrown)
The existing harness is desktop-only. It looks like it runs something like 'firefox -unittest --gtest_death_test_style=threadsafe'. The harness needs to be updated to launch something like that on device instead...but I'm not sure what!

I tried

adb shell am start org.mozilla.fennec_gbrown --es args "-unittest --gtest_death_test_style=threadsafe"

and 

adb shell am start org.mozilla.geckoview.test --es args "-unittest --gtest_death_test_style=threadsafe"

but I don't see any sign of gtests running.

:jchen -- Do you have an idea of how we can / should run gtests on device? For geckoview? To give you more context, from email, :Dexter said "Some of the work, mainly bug 1453591, requires some (scary!) heavy lifting by the Telemetry c++ core. The changes that we're doing there will only be available on Fennec/GV and just enabled on GeckoView.
I would need to add c++ test coverage for these parts in gtest. These tests would live, along with the existing ones, in toolkit/components/telemetry/tests/gtest.". I haven't looked into gtests before. Will we need some build changes too?
Flags: needinfo?(gbrown) → needinfo?(nchen)
Looks like gtest requires building a separate "libxul-gtest.so" and running the test app again that. It's definitely doable but might take some extra work. I wonder if we can just run the GV gtests on Linux? Aren't gtests supposed to be self-contained unit tests? So maybe an Android environment is not strictly necessary?
Flags: needinfo?(nchen)
(In reply to Jim Chen [:jchen] [:darchons] from comment #4)
> So maybe an Android environment is not strictly necessary?

Good point. Let me see if I can work around this. Thanks!
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
P3 because we'd like to run all tests on GeckoView, but gtests are a lower priority than other tests because we don't run them for Fennec.

Bitbar ARM32 (Moto G5)
Bitbar ARM64 (Pixel 2)
Packet x86-64

We don't need to test Android x86 if we are testing Android x86-64.
OS: Unspecified → Android
Priority: P5 → P3
Whiteboard: [geckoview:p3]
We have some mfbt tests that are gtests.
We should *definitely* be running these on Android.

We should run these tests on the x86_64 servers. snorp says we don't probably need to run them on the Pixel 2 devices.

Hardware: Unspecified → x86_64
Summary: Run GTest on Android → Run GTest on Android x86_64
Whiteboard: [geckoview:p3] → [geckoview:p1]

Stuart, who could own this?

Flags: needinfo?(sphilp)

(In reply to David Bolter [:davidb] (NeedInfo me for attention) from comment #10)

Stuart, who could own this?

Geoff Brown on the Automation team has been enabling GeckoView tests as they become green, but according to comment 3 and 4, additional work is needed to compile the gtest runner for Android. I don't know if that is the responsibility of the Automation team or the GeckoView team.

Component: Testing → General
Priority: P3 → P2
Product: Firefox for Android → GeckoView
Whiteboard: [geckoview:p1]

:jchen previously (comment 4) identified that the gtest libxul library was not built for android. Today, for android x86_64, 'mach gtest' does build toolkit/library/gtest/libxul.so, apparently successfully. The test command still fails due to desktop assumptions, but I may be able to make progress now.

(Still interested in sphilp's input -- would be happy to hand this off if someone else is available.)

Assignee: nobody → gbrown

Geoff, perhaps you can you investigate and see what work is remaining? If there are specific asks for the Geckoview team we can work with dbolter to get them resolved. I'll defer to Joel re: whether to assign this to you as he knows better what's in flight/backlog, but from where I'm sitting it makes sense for you to pick this up at least for the investigation and then we can figure out how to divvy up anything that comes out of it (between yourself, dbolters team, egao, others). NI to Joel in case there are other plans here :)

Flags: needinfo?(sphilp) → needinfo?(jmaher)

we have a list of other work in progress to get running on Android. As GTest has never run on android there are many unknowns, and I would estimate a 3 week project. if this is a priority we can move it up, otherwise I suspect we will start work on this in Q1, maybe finish if there are not big hurdles. At least we can comment on the feasibility in the coming weeks as well as ask for help as needed.

Right now marionette and cppunittests are in progress. :gbrown, should this bug block bug 1473368 ?

Flags: needinfo?(jmaher)

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #14)

:gbrown, should this bug block bug 1473368 ?

No, I don't think so, because 1473368 is focused on reducing existing tests; let's go with see also.

See Also: → 1473368
Depends on: 1532695

Sorry, I haven't been finding much time for this bug. I did demonstrate, in bug 1532695, a proper android gtest build, but today builds are failing, apparently unrelated to my changes.

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=b5330a734e2f3f8641ef358b37551e7491d2f60d

I have realized that once the builds are okay and the remote script can launch TRA in gtest mode, there will be an issue with output processing. rungtests.py monitors firefox's standard output; an equivalent for android might be monitoring logcat, but that will have system messages, etc mixed in -- perhaps not appropriate. Also, we haven't tried monitoring logcat from python in real time before. I wonder if gtest output can be redirected to a file, which could be polled, similar to android mochitest/reftest? I'll look into that once I've demonstrated gtest running on android.

In addition to existing gtests as mentioned in comment #8, it would be good to be able to write new unit tests using gtest and get coverage on Android. I just ran into this with bug 1479960, where there's some Android-specific code (and I encountered some Android-specific bugs); the gtests aren't critical in that case but I was surprised that we didn't already run them.

More than a few hacks here, but got a green build, invoked the new gtest script, and started geckoview TRA with gtest arguments:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=d07883ab84fbd8950985bcbfdf781deb23421071

but no tests are run and gecko just exits:

03-29 22:16:40.270 2347 2363 I fennec : XRE_main returned 1
03-29 22:16:40.270 2347 2363 D GeckoThread: State changed to EXITED

(In reply to Geoff Brown [:gbrown] from comment #18)

but no tests are run and gecko just exits:

because the gtest entry point is not found...because the gtest libxul.so hasn't been built into the app (TRA is using the wrong libxul.so).

With the correct libxul.so hacked in, we get further, but then crash:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=304d04ce9517679718d039c607f28999bb4a75d9

(In reply to Geoff Brown [:gbrown] from comment #20)

With the correct libxul.so hacked in, we get further, but then crash:

That disappeared with harness changes + a new base revision.

Running tests, with some failures and crashes, and output only in logcat:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=090f36f2bd6e15764a7d3bc29c273c30b4fba262

Disable gtests observed to fail on Android. Some of these are simple build
failures and failures due to file permissions or paths, while other failures
are more obscure.
Once Android gtests are running on mozilla-central, I will file follow-up
bugs inviting teams to investigate the failures and re-enable Android gtests
that are important to them.

Keywords: leave-open
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c87d5720c982
Disable failing android gtests; r=bc

On Android, update mozilla gtest logging so that logging appears in the Android logcat.
Also, when MOZ_GTEST_LOG_PATH is defined in the environment, create the named file
and direct logging to that file. Android gtest will use this to collect gtest logging from
the device and copy it to the test log.

Desktop gtest creates minidumps in the current working directory. That is
problematic on Android, since the test app's cwd may not be writable, or
may not be readable by the test harness. This patch allows the test harness
to specify an alternate minidump path with environment variable
MOZ_GTEST_MINIDUMPS_PATH.

Type: defect → enhancement
Attachment #9057126 - Attachment description: Bug 1318091 - Support env override of gtest minidump location; r=bc → Bug 1318091 - Support env override of gtest minidump location; r=Ehsan
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/64deb410ad68
Support logging to file in mozilla gtest; r=bc,Ehsan

The jemalloc tests leave behind minidumps. Disable for now, for a green run.

Add Android 7.0 gtests, opt and debug, running against the geckoview
TestRunnerActivity.

Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b74ffd005577
Support env override of gtest minidump location; r=Ehsan
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/60a543a89dfa
Skip jemalloc gtests on android; r=bc
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5bd741206a8b
Add remotegtests.py, supporting android gtests; r=bc
Blocks: 1544496

Support --shuffle and <gtest_filter> for android gtest.

Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e3f0733baa2
Add more options to remotegtests.py; r=bc
See Also: → 1545217
See Also: → 1545218
See Also: → 1545219
See Also: → 1545223
See Also: → 1545226
See Also: → 1545228
See Also: → 1545230
See Also: → 1545231
See Also: → 1545233
See Also: → 1545234
See Also: → 1545235
See Also: → 1545237

Add basic support for 'mach gtest' on Android.
Handling of Android-only and desktop-only options is awkward; I hope to
re-visit this after bug 1519369.

Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98df47a219e2
Support |mach gtest| for android; r=bc
Keywords: leave-open

https://hg.mozilla.org/mozilla-central/rev/98df47a219e2

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
See Also: → 1546453
No longer blocks: 1544496
Depends on: 1544496
See Also: → 1548555
Depends on: 1550052
Blocks: 1558867
Blocks: 1558885
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: