1238788 - (android-lint-automation) Run Android lint in automation

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect)

Description

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

This ticket tracks running Android lint in automation.

This is like Bug 1064004 and Bug 1238678 but for Android lint instead of various test suites.  On Android, running lint requires a (--disable-compile-environment) build to produce JAR files, so it's closer to a build than a standalone lint step.

Comment 1

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=737489921b81

Comment 2

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3994a64cae60

Comment 3

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=737a3d004387

Comment 4

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Attachment: MozReview Request: Bug 1238788 - Add job running Android lint in automation. r?dustin

This job looks like a frontend (--disable-compile-environment) build,
followed by some Java commands (that produce build artifacts).  If the
Java commands fail, we should fail the job (after copying artifacts,
etc).  I'll add some post-processor that interprets the outputs
eventually, but this pass/fail serves as a good starting point.

I'd like to extract some kind of android_api_11_base.yml and re-use
the configuration, but I don't really know if that's a good approach.
Guidance appreciated.

Review commit: https://reviewboard.mozilla.org/r/30463/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30463/

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8706718 [details]
MozReview Request: Bug 1238788 - Add job running Android lint in automation. r?dustin

https://reviewboard.mozilla.org/r/30463/#review27387

::: testing/taskcluster/tasks/builds/android_api_11_frontend.yml:61
(Diff revision 1)
> +          exit $err)

Yuck :)

Keep in mind we have a lot of layers:

 * docker; runs
 * build.sh; runs
 * mozharness; runs
 * mach; runs
 * make (or maybe gradle here? dunno)

And really it's best to keep the build scripting limited to the two tools that already contain most of that scripting -- mozharness and mach.

This is OK (even great!) for testing in try, but ultimately these commands need to be somewhere else.  Since they invoke mach, I'm guessing mozharness is the place.

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4422b622264

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

The treeherder push in comment 7 had the following error:


Errors received:
You are using pip version 7.1.2, however version 8.0.3 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Return code: 0
OSError: [Errno 2] No such file or directory

    566330 Linux64 Talos failure: OSError: [Errno 2] No such file or directory

IOError: [Errno 2] No such file or directory: 'dist/bin/platform.ini'
IOError: [Errno 2] No such file or directory: 'dist/bin/platform.ini'
IOError: [Errno 2] No such file or directory: 'dist/bin/platform.ini'
IOError: [Errno 2] No such file or directory: 'dist/bin/platform.ini'
IOError: [Errno 2] No such file or directory: 'dist/bin/platform.ini'
SyntaxError: unexpected EOF while parsing
Return code: 1
1 not in success codes: [0]
Halting on failure while running ['python2.7', 'mach', '--log-no-times', 'gradle', 'app:lintAutomationDebug']

However, the logs indicate this failed because of lint:

20:30:40     INFO -  Lint found 1 errors and 865 warnings
20:30:40     INFO -  :app:lintAutomationDebug FAILED
20:30:40     INFO -  FAILURE: Build failed with an exception.

So this patch works! Getting the proper output sounds like it's outside of my scope of expertise. Let's see if we can get someone else to fix that. For the sake of incremental improvements, I'm fine landing as is with an announcement email regarding the expected output for a lint error.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1238788 - Burn the tree on android lint failure. r=nalexander

Looking at [1], we halt_on_failure for the `postflight_build_mach_command`s and
the flag I flipped causes the lint command to return a non-zero exit code (i.e.
noting failure).

[1]: https://hg.mozilla.org/mozilla-central/rev/b7f1dac8306f#l2.29

Review commit: https://reviewboard.mozilla.org/r/37687/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37687/

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Some discussion on irc on how to provide a user-friendly error summary on treeherder:

14:28 <&garndt> mcomella: if I understand correctly, when someone sees that job fail on treeherder, once selecting that job, they should see only the error "Android lint failed" and none of the other errors.  Right now I don't think that's possible.  Treeherder will display *all* errors that are in the log, not just the one you want to display
14:29 <nalexander> garndt: mcomella: focus on printing links to the Gradle report and the Unit Test report.
14:30 <&garndt> mcomella: it's a series of regexes that are configured within the service https://github.com/mozilla/treeherder/blob/master/treeherder/log_parser/parsers.py#L383
14:30 <nalexander> garndt: mcomella: structured logging is possible but wasted effort in my opinion.
14:30 <&garndt> thanks nalexander 
14:35 <mcomella> nalexander: Do you have an example of how to do that?
14:35 <nalexander> mcomella: structured logging?
14:36 <mcomella> nalexander: print links to the gradle report & unit test report
14:36 <nalexander> mcomella: the Robocop and xpcshell test runners do it in Python.
14:36 <nalexander> mcomella: not really.  I don't know how to reference artifact uploads in logs.
14:36 <nalexander> mcomella: I kinda think it's not possible?

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

FWIW, this is an issue everywhere.  For example, if you have an error in your mozconfig, config.log is tail'd, and treeherder reports lots of compiler errors that were really just autoconf testing compiler features.

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Good to know.

Filed bug 1253082 to try to improve this situation.

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I added `explainIssues true` but I received the same output on tbpl so let's forgo that.

Comment 14

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

Comment on attachment 8725897 [details]
MozReview Request: Bug 1238788 - Burn the tree on android lint failure. r=nalexander

https://reviewboard.mozilla.org/r/37687/#review34737

Whenever you're ready, do it.  Be sure to mail the list!

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/02658c6e4adb4e42217319a3e08ca16e5aa57b30
Bug 1238788 - Burn the tree on android lint failure. r=nalexander

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/02658c6e4adb