Add code coverage for android robocop UI tests

RESOLVED FIXED in Firefox 64

Status

enhancement
RESOLVED FIXED
9 months ago
8 months ago

People

(Reporter: gabriel-v, Assigned: gabriel-v)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(8 attachments)

46 bytes, text/x-phabricator-request
marco
: review+
gbrown
: review+
Details | Review
46 bytes, text/x-phabricator-request
nalexander
: review+
Details | Review
46 bytes, text/x-phabricator-request
gbrown
: review+
Details | Review
46 bytes, text/x-phabricator-request
nalexander
: review+
Details | Review
46 bytes, text/x-phabricator-request
gbrown
: review+
nalexander
: review+
Details | Review
46 bytes, text/x-phabricator-request
gbrown
: review+
Details | Review
46 bytes, text/x-phabricator-request
gbrown
: review+
Details | Review
46 bytes, text/x-phabricator-request
marco
: review+
Details | Review
(Assignee)

Description

9 months ago
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
(Assignee)

Comment 1

9 months ago
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
Flags: needinfo?(gbrown)
Yes, you can expect options.autorun=True for all tests run on taskcluster.
Flags: needinfo?(gbrown)
(Assignee)

Updated

9 months ago
Depends on: 1481834
(Assignee)

Comment 3

9 months ago
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
(Assignee)

Comment 4

8 months ago
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?
Flags: needinfo?(gbrown)
(Assignee)

Comment 6

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

Depends on D4142
(Assignee)

Comment 8

8 months ago
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
(Assignee)

Comment 10

8 months ago
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
(Assignee)

Comment 11

8 months ago
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
Attachment #9003597 - Attachment description: Bug 1475256 - Add documentation for robocop code coverage. r=nalexander → Bug 1475256 - Add documentation for robocop code coverage. r=nalexander,gbrown
(Assignee)

Comment 12

8 months ago
Fixed the issue by triggering the coverage dump before Robocop:Quit. See https://phabricator.services.mozilla.com/D4218
Flags: needinfo?(gbrown)
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.
Attachment #9003592 - Flags: review+
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.
Attachment #9003800 - Flags: review+
Comment on attachment 9003597 [details]
Bug 1475256 - Add documentation for robocop code coverage. r=nalexander,gbrown

Geoff Brown [:gbrown] has approved the revision.
Attachment #9003597 - Flags: review+
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.
Attachment #9003592 - Flags: review+
Comment on attachment 9003799 [details]
Bug 1475256 - Dump code coverage report file before quitting Fennec. r=gbrown

Geoff Brown [:gbrown] has approved the revision.
Attachment #9003799 - Flags: review+
Comment on attachment 9003595 [details]
Bug 1475256 - Add code coverage for android robocop UI tests. r=gbrown

Geoff Brown [:gbrown] has approved the revision.
Attachment #9003595 - Flags: review+
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.
Attachment #9003593 - Flags: review+
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.
Attachment #9003597 - Flags: review+
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.
Attachment #9003596 - Flags: review+
(Assignee)

Comment 22

8 months ago
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 on attachment 9006979 [details]
Bug 1475256 - Download and extract app classfiles, not only geckoview ones. r=marco

Marco Castelluccio [:marco] has approved the revision.
Attachment #9006979 - Flags: review+
Attachment #9003595 - Attachment description: Bug 1475256 - Add code coverage for android robocop UI tests. r=nalexander,gbrown → Bug 1475256 - Add code coverage for android robocop UI tests. r=gbrown
(Assignee)

Updated

8 months ago
Blocks: 1489236
(Assignee)

Comment 24

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

Comment 25

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aad7ddbe7a42
Refactor CodeCoverageMixin to pass --java-coverage-output-dir instead of a file path. r=gbrown,marco
Keywords: checkin-needed

Comment 26

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/468b56809947
Turn archive-geckoview-coverage-artifacts into archive-coverage-artifacts. r=nalexander

Comment 27

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e21fe645d43
Dump code coverage report file before quitting Fennec. r=gbrown

Comment 28

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/264b18cc70e5
Skip robocop tests that fail on debug and ccov builds. r=gbrown

Comment 29

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d860063c477
Add code coverage for android robocop UI tests. r=gbrown

Comment 30

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b602c278c16
Remove top-level 'apply plugin: jacoco' from android configurations. r=nalexander

Comment 31

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/65484445e453
Add documentation for robocop code coverage. r=nalexander,gbrown

Comment 32

8 months ago
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cd7581016f7a
Download and extract app classfiles, not only geckoview ones. r=marco
You need to log in before you can comment on or make changes to this bug.