1475256 - Add code coverage for android robocop UI tests

Status: RESOLVED FIXED

(Testing :: Code Coverage, enhancement)

Description

[Gabriel Vîjială [:gabriel-v]]

This is the test suite: https://wiki.mozilla.org/Mobile/Fennec/Android/Testing#robocop_UI_tests.

This is going to be interesting, as only some of the tests are using 'am instrument', which we can turn into 'am instrument -e coverage true -e coverageFile /sdcard/coverage.ec' and collect the report with 'adb pull' afterwards.

The other tests appar to start an activity and communicate with it in order to run the tests. This means some changes to the test runner activity will be needed (to start profiling and dump the results at the correct moments).

https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/testing/mochitest/runrobocop.py#423

Comment 1

[Gabriel Vîjială [:gabriel-v]]

In the past commit I was mistaken: the `options.autorun` switch is set for the whole test suite run, not per test.

This means we can get code coverage in the same fashion as geckoview-junit[1] with 'am instrument -e coverage true' followed by an 'adb pull'.

Geoff Brown: Is `options.autorun` switch linked in comment#1 always on for tests run on taskcluster? Can all tests be ran with `options.autorun=True`? Could we implement code coverage in the same style as the geckoview-junit patch, and not worry about the `options.autorun=False` case? 

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1473313

Comment 2

[Geoff Brown [:gbrown]]

Yes, you can expect options.autorun=True for all tests run on taskcluster.

Comment 3

[Gabriel Vîjială [:gabriel-v]]

Ran into some issues with adding code coverage for this test suite:

1. Robocop tests are currently disabled for debug builds [1]. Code coverage builds are debug builds with even slower runtime speeds, so we will need to figure out how to run Robocop tests with debug builds as a part of adding code coverage to this. I'm opening bug 1481834 about this.

2. The 'am instrument -e coverageFile /tmp/XXX` parameter seems to be ignored by the  components. This means that the JaCoCo agent will try to write the coverage files into the root directory, which fails. I have a logcat [2] and a job log [3] about this failure. Since robocop is running Fennec, I tried removing instrumentation for GeckoView, because maybe the coverageFile option doesn't reach GeckoView.

3. The scripts running the tests will do one 'am instrument' call per test, which means we'll get one coverage file per test. I'll have to change the CodeCoverageMixin to set a directory where the coverage files will be uploaded, instead of setting a file path. I'll also have to change it to collect all the files that were placed in that directory.


[1]: https://dxr.mozilla.org/mozilla-central/rev/4e56a2f51ad739ca52046723448f3129a58f1666/taskcluster/ci/test/test-sets.yml#369
[2]: https://taskcluster-artifacts.net/PDMIUt9jSNKthR7c_ryhQg/0/public/test_info//logcat-emulator-5554.log
[3]: https://taskcluster-artifacts.net/PDMIUt9jSNKthR7c_ryhQg/0/public/logs/live_backing.log

Comment 4

[Gabriel Vîjială [:gabriel-v]]

Progress.

Here's a try with code coverage support: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e0e2ef06962e9107329404bb8641897530d3b53

Half of it is orange because I forgot to skip tests that fail on debug builds.
The other half is orange because the tests did not dump the coverage files onto the emulator.

Hence the problem: the runrobocop.py test runner tries to pull the coverage files before they become available. This happens for all tests except for one (testBookmarklets in chunk 1). Since that one test class worked, I assume this happens because the `am instrument` call did not finish running when the test is detected as finished.

The gv-junit tests worked because we were running `am instrument` on the emulator through `adb shell`, which exits when the child process exits. I see that robocop tests use remoteautomation.py which starts and monitors processes. Is it supposed to return before the `am instrument` process exits?

Geoff: Would it be OK to poll if the coverage file exists for something like 1 min before running `adb pull`? Is there a smarter way to fix this?

Comment 5

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Refactor CodeCoverageMixin to pass --java-coverage-output-dir instead of a file path. r=gbrown,marco

Comment 6

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Turn archive-geckoview-coverage-artifacts into archive-coverage-artifacts. r=nalexander

Rename the command and extend it to also archive Fennec class files into
another artifact, target.app_classfiles.zip.

Depends on D4142

Comment 7

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Add code coverage for android robocop UI tests. r=gbrown

Depends on D4143

Comment 8

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Remove top-level 'apply plugin: jacoco' from android configurations. r=nalexander

It appears that we don't actually need to load this plugin, since we're not
doing any JaCoCo-related operations on the build machine (like generating
reports). This has been left over since I integrated code coverage to gv-junit.

Depends on D4144

Comment 9

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Add documentation for robocop code coverage. r=nalexander,gbrown

Depends on D4145

Comment 10

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Dump code coverage report file before quitting Fennec. r=gbrown

The Android InstrumentationTestRunner calls its private 'generateCoverageReport()' method
after the test runner has finished running, in its 'onStart()' method. That method does not
get called because we never finish the test; instead we trigger a 'Robocop:Quit' event and
wait to be killed. This patch reimplements that method and calls it before triggering a quit.

See InstrumentationTestRunner implementation: https://android.googlesource.com/platform/frameworks/base/+/android-8.1.0_r43/test-runner/src/android/test/InstrumentationTestRunner.java#580

Depends on D4143

Comment 11

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Skip robocop tests that fail on debug and ccov builds. r=gbrown

Skip tests that fail on debug and code coverage builds, either intermittently
or permanently. See bug 1481834 for details about why these tests are skipped.

Depends on D4218

Comment 12

[Gabriel Vîjială [:gabriel-v]]

Fixed the issue by triggering the coverage dump before Robocop:Quit. See https://phabricator.services.mozilla.com/D4218

Comment 13

[Marco Castelluccio [:marco]]

Comment on attachment 9003592 [details]
Bug 1475256 - Refactor CodeCoverageMixin to pass --java-coverage-output-dir instead of a file path. r=gbrown,marco

Marco Castelluccio [:marco] has approved the revision.

Comment 14

[Geoff Brown [:gbrown]]

Comment on attachment 9003800 [details]
Bug 1475256 - Skip robocop tests that fail on debug and ccov builds. r=gbrown

Geoff Brown [:gbrown] has approved the revision.

Comment 15

[Geoff Brown [:gbrown]]

Comment on attachment 9003597 [details]
Bug 1475256 - Add documentation for robocop code coverage. r=nalexander,gbrown

Geoff Brown [:gbrown] has approved the revision.

Comment 16

[Geoff Brown [:gbrown]]

Comment on attachment 9003592 [details]
Bug 1475256 - Refactor CodeCoverageMixin to pass --java-coverage-output-dir instead of a file path. r=gbrown,marco

Geoff Brown [:gbrown] has approved the revision.

Comment 17

[Geoff Brown [:gbrown]]

Comment on attachment 9003799 [details]
Bug 1475256 - Dump code coverage report file before quitting Fennec. r=gbrown

Geoff Brown [:gbrown] has approved the revision.

Comment 18

[Geoff Brown [:gbrown]]

Comment on attachment 9003595 [details]
Bug 1475256 - Add code coverage for android robocop UI tests. r=gbrown

Geoff Brown [:gbrown] has approved the revision.

Comment 19

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9003593 [details]
Bug 1475256 - Turn archive-geckoview-coverage-artifacts into archive-coverage-artifacts. r=nalexander

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 20

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9003597 [details]
Bug 1475256 - Add documentation for robocop code coverage. r=nalexander,gbrown

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 21

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9003596 [details]
Bug 1475256 - Remove top-level 'apply plugin: jacoco' from android configurations. r=nalexander

Nick Alexander :nalexander [he/him] has approved the revision.

Comment 22

[Gabriel Vîjială [:gabriel-v]]

Attachment: Bug 1475256 - Download and extract app classfiles, not only geckoview ones. r=marco

We collect the classfiles from app and geckoview during the build task, but
before this patch we didn't use the app classfiles.

Depends on D4146

Comment 23

[Marco Castelluccio [:marco]]

Comment on attachment 9006979 [details]
Bug 1475256 - Download and extract app classfiles, not only geckoview ones. r=marco

Marco Castelluccio [:marco] has approved the revision.

Comment 24

[Gabriel Vîjială [:gabriel-v]]

working try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=905648adf2e4ae4125316bca60f849a86e5ac467 (ignore the gv-junit4 failure, it's unrelated)

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aad7ddbe7a42
Refactor CodeCoverageMixin to pass --java-coverage-output-dir instead of a file path. r=gbrown,marco

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/468b56809947
Turn archive-geckoview-coverage-artifacts into archive-coverage-artifacts. r=nalexander

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e21fe645d43
Dump code coverage report file before quitting Fennec. r=gbrown

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/264b18cc70e5
Skip robocop tests that fail on debug and ccov builds. r=gbrown

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d860063c477
Add code coverage for android robocop UI tests. r=gbrown

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b602c278c16
Remove top-level 'apply plugin: jacoco' from android configurations. r=nalexander

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/65484445e453
Add documentation for robocop code coverage. r=nalexander,gbrown

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd7581016f7a
Download and extract app classfiles, not only geckoview ones. r=marco

Comment 33

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/aad7ddbe7a42
https://hg.mozilla.org/mozilla-central/rev/468b56809947
https://hg.mozilla.org/mozilla-central/rev/6e21fe645d43
https://hg.mozilla.org/mozilla-central/rev/264b18cc70e5
https://hg.mozilla.org/mozilla-central/rev/1d860063c477
https://hg.mozilla.org/mozilla-central/rev/1b602c278c16
https://hg.mozilla.org/mozilla-central/rev/65484445e453
https://hg.mozilla.org/mozilla-central/rev/cd7581016f7a