793678 - Add more "Remote Device Error:" prefixed error messages to sut_tools scripts

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Ed Morley [:emorley]]

Carries on the work in bug 790613.

Adds "Automation Error:" / "Remote Device Error:" prefixes to error strings where missing & outputs messages in places where we were up until now only using setFlag().

Covers quite a few of the cases where TBPL's annotated summary currently says "Summary is empty." and will hopefully give a bit more debugging info for some of our open Tegra oranges/purples.

Comment 1

[Ed Morley [:emorley]]

Attachment: Patch v1

(Likely bitrots some of bug 781341, but I'd rather not wait any longer, since improving our 'manual starring' story is a Q3 goal)

Comment 2

[Justin Wood (:Callek)]

Comment on attachment 664046 [details] [diff] [review]
Patch v1

In code-is-correct wise, this is good, in "do I want to take this" r-.

First, we have log.info specified for any setFlag calls, http://mxr.mozilla.org/build/source/tools/sut_tools/sut_lib.py#385 which is (should in theory) printing the error to the log.

However the error.flg can kill buildbot and abort the step/job when we are having a buffered stdout from python, so we never flush before it exits, which we can probably fix by adding PYTHONDONTWRITEBYTECODE to the env.

That said, if this is necessary for Sheriff Sanity, I won't block too strongly on it, but I hate meaningless duplication. Feel free to debate me :-)

Comment 3

[Phil Ringnalda (:philor)]

We don't know whether it's necessary, because we've never seen them. Do they happen 30 times a day, and get eaten? Do they never happen? Do they happen once every two weeks, and get printed, and get lost in the incredible spew of meaningless garbage that's the full logs that we have to look at 50 times a day?

Comment 4

[Ed Morley [:emorley]]

(In reply to Justin Wood (:Callek) from comment #2)
> First, we have log.info specified for any setFlag calls,
> http://mxr.mozilla.org/build/source/tools/sut_tools/sut_lib.py#385 which is
> (should in theory) printing the error to the log.

Ah, think I've found why this doesn't work.

Unlike {check.py,clientproxy.py,stop.py}, we don't log.setLevel() in sut_lib.py; so are on the default of warnings and above only.

setFlag() in sut_lib.py uses log.info, so we don't see the messages.

Either way, I agree the duplication is yucky, so worst case, we just stick a print inside setFlag(), or else hopefully s/log.info/log.warning/ will do the trick so print won't be necessary.

Comment 5

[Ed Morley [:emorley]]

Attachment: Patch v2

* No longer adds redundant print statements, instead adds prefix to the setFlag call where missing. May need tweaking once tested on staging, if some of the "Remote Device Error:" cases are psuedo-expected and thus occur on "good" runs and so would cause unnecessary RETRYs.
* Removes pre-existing cases where we had doubled up on setFlag() and print.

Relies on setFlag being fixed, will file a bug for that now.

Comment 6

[Justin Wood (:Callek)]

Comment on attachment 664418 [details] [diff] [review]
Patch v2

Review of attachment 664418 [details] [diff] [review]:
-----------------------------------------------------------------

All changes look good to me, land away. f? to joel, for the mere fact that this bitrots him (I think). I'll be able to assist in rebasing one of your existing patches if need be.

(do not block landing on joel's feedback)

::: sut_tools/clientproxy.py
@@ +373,5 @@
>                          log.warning(i)
>                      log.warning('verify.py returned with errors')
>                      if not os.path.isfile(errorFile):
>                          log.warning('verify.py did not create \'%s\' as expected, bailing.' % errorFile)
> +                        setFlag(errorFile, "Remote Device Error: Verify.py returned with an unexpected error.")

buildbot will never see this message -- and in theory neither should we (if we coded verify.py correctly) however the change doesn't hurt, so fine to leave in.

Comment 7

[Ed Morley [:emorley]]

Comment on attachment 664418 [details] [diff] [review]
Patch v2

http://hg.mozilla.org/build/tools/rev/544a6ba8eca4

Comment 8

[Justin Wood (:Callek)]

Deployed to production

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 664418 [details] [diff] [review]
Patch v2

Review of attachment 664418 [details] [diff] [review]:
-----------------------------------------------------------------

ed morley and the monster cleanup!