1255875 - Make Android Unit test output TreeHerder friendly

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Nick Alexander :nalexander [he/him] [Back August 22, 2022]]

Right now, a failing Unit job (like 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca5a2cc0d3f4&selectedJob=17963488) requires clicking into the test, clicking on the right link to see the report (https://public-artifacts.taskcluster.net/CZRtlPO8QXCMUfFpT6pPMQ/0/public/android/lint/lint-results-automationDebug.html#UnusedResources), and then interpreting the report.  We need to surface failures and status more explicitly.  It should be somewhat easy, since we get a beautiful XML report, like (https://public-artifacts.taskcluster.net/CZRtlPO8QXCMUfFpT6pPMQ/0/public/android/lint/lint-results-automationDebug.xml).

But I don't know how to do this!  I'd like to add a "process lint logs" step after https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/tasks/builds/android_api_15_frontend.yml#46 or (better, I think) after https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/configs/builds/releng_sub_android_configs/64_api_15_frontend.py#9.

Comment 1

[Nick Alexander :nalexander [he/him] [Back August 22, 2022]]

ahal: can you tell me how to achieve this TC -> TH integration, or redirect me to somebody who can?

Comment 2

[Andrew Halberstadt [:ahal]]

Uploaded artifacts should show up directly in the "Job Details" pane, but for taskcluster they do not. This is bug 1218537. Is that what you mean?

If you want more information in the failure summary, your job will need to log errors to stdout. Treeherder then scans stdout for failures using a set of regexes. I'm a bit fuzzier on how this part works, maybe :camd could help you.

Comment 4

[Michael Comella (:mcomella) [needinfo or I won't see it]]

fwiw, I found out you can print errors to stdout with the following lint options:

lintOptions {
  ...
  explainIssues false // if this is true, it'll explain how to fix each issue
  textReport true
}

It just might be an (unideal but) easy win for Treeherder output.

One caveat is that all warnings are included in the text output (when we really just want the errors) but maybe this could be solved with regex post-processing on the output (e.g. sed, rather than parsing XML, which will likely take more development effort). You can also redirect the text output to a file for analysis with the `textOutput` [2] option.

See also some previous research (unrelated to these options) in bug 1253082.

[1]: https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.LintOptions.html#com.android.build.gradle.internal.dsl.LintOptions:explainIssues
[2]: https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.internal.dsl.LintOptions.html#com.android.build.gradle.internal.dsl.LintOptions:textOutput

Comment 5

[Michael Comella (:mcomella) [needinfo or I won't see it]]

I found https://github.com/lyft/linty_fresh which parses lint errors, creates a github pull request, and adds the appropriate comments – maybe we can re-use their parsers (apache v2 license).

Comment 6

[Phil Ringnalda (:philor)]

The good news is that because you really really really shouldn't ever have intermittent failures which need to be starred, only actual failures that need to be fixed or the cause backed out, you can use the same sort of method for skirting what's normal as the Hazard builds do: output a message which treeherder will recognize as a failure line which just says that the job found an error and where to see it, a la "TEST UNEXPECTED-FAILURE Checkstyle rule violations were found. See the report at: https://queue.taskcluster.net/v1/task/cmJb9OYhQzupSntuDjRPJA/runs/0/artifacts/public%2Fandroid%2Fcheckstyle%2Fcheckstyle.xml" (though, with that uploaded URL, not with the current file:/// URL).

The bad news is that since even for tier-2 https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Usable_job_logs requires that you have done that before making these suites visible, I'm hiding checkstyle/lint/test.

Comment 7

[Michael Comella (:mcomella) [needinfo or I won't see it]]

(In reply to Phil Ringnalda (:philor) from comment #6)
> The bad news is that since even for tier-2
> https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Usable_job_logs
> requires that you have done that before making these suites visible, I'm
> hiding checkstyle/lint/test.

Sebastian, NI for FYI (no action necessary if you don't deem it necessary). Since you're doing work on the browser as part of AS, it could be worth getting these tests to output properly so they're not hidden.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Argh. We added a bunch of unit tests lately and they are getting more and more valuable. I'd almost like to see them a tier-1 job... so yeah I think we should fix that in order to make them visible again.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/92d96352192c
Complain to treeherder if Android checkstyle, lint, or unittest fails. r=grisha

Comment 10

[Nick Alexander :nalexander [he/him] [Back August 22, 2022]]

(In reply to Pulsebot from comment #9)
> Pushed by [e-mail]:
> https://hg.mozilla.org/integration/fx-team/rev/92d96352192c
> Complain to treeherder if Android checkstyle, lint, or unittest fails.
> r=grisha

Reviewed over in Bug 1299015.

Comment 11

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/92d96352192c