1139911 - make-check test-failures not turning builds orange

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Andrew Halberstadt [:ahal]]

I pushed a change to try that broke a mozbase test, but it only turned about half of the builds orange, though the failure shows up in the logs for all of the builds. I suspect we're doing the right thing in certain build scripts, but not others.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60a457e81c05&filter-searchStr=build

Note that all the green builds have the test failure in the logs. I guessed this was a mozharness issue, but I could be wrong. Please move if it's not!

Comment 2

[Phil Ringnalda (:philor)]

It was mozharness - for some unclear reason we're still doing non-mozharness builds on mozilla-beta, so a permanent test failure on mozilla-aurora in mozharness builds is completely green, and suddenly becomes orange on the merge to mozilla-beta.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Lack of test coverage is basically a development blocker....  We shouldn't be checking in things if we don't know whether they're correct.

Comment 4

[Jonathan Griffin (:jgriffin)]

This is a bug in the mozharness script that runs the builds, not mozbase, so I'm putting this back in releng. Ahal, can you take a quick look? Are we just not parsing results like we should be here?

Comment 5

[Andrew Halberstadt [:ahal]]

Sorry, I filed and forgot. No excuse for not looking into this right away.

My theory is that the parsing is working fine, but the build script needs to set TBPL_FAILURE as the worst level instead of TBPL_WARNING. There are other steps that also return TBPL_WARNING on fail with comments like "turn the job orange". So maybe this was regressed in a treeherder change that stopped turning jobs with a TBPL_WARNING status orange.

Here's a try run to test that theory out:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6ce3352ff175

Comment 6

[Phil Ringnalda (:philor)]

What part of http://mxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/building/buildbase.py#160 intends to do anything about the job status? I'd expect it to be doing something like http://mxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/testing/unittest.py#169

Comment 7

[Andrew Halberstadt [:ahal]]

I thought that was happening in the parent OutputParser, but evidently I was wrong. Yes, it looks like that's the problem. Maybe they should be using the same output parser as the other desktop unittests, but that's a can of warms I'm not too keen on opening just yet.

Comment 8

[Phil Ringnalda (:philor)]

Yeah, leave that can closed, since the other desktop unittests have the flow "harnesses summarize pass/fail/todo themselves -> parser looks for the predigested summary and acts accordingly, setting the job status to nonsuccess either if tests failed or if the summary doesn't exist" so opening it up still requires that someone write a parser that will decide whether or not make check failed tests :)

Comment 10

[Andrew Halberstadt [:ahal]]

See also bug 1215755 comment 3

Comment 11

[Andrew Halberstadt [:ahal]]

Hooray, the builds fail:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c2df0cf0235c

I'm quite confused about how this worked before, or what regressed this.

Now I guess we actually have to fix everything before I can land this. I'll file new bugs for individual issues and make them block this. Here's a new try run with every platform to see if there are any platform specific issues:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=98902d03a742

Comment 12

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Comment 13

[Boris Zbarsky [:bzbarsky]]

> Hooray, the builds fail:

Yes, that's what happens when test coverage gets turned off: test failures creep in.  :(

Comment 14

[Andrew Halberstadt [:ahal]]

I actually wasn't being sarcastic :). I knew they were expected to fail, so was happy that my patch turned the jobs orange like it was supposed to.

But yes, not happy that test failures crept in. Luckily it looks like bug 1215755 (which you already fixed) and bug 1215756 (which we're currently investigating and will disable if the fix isn't trivial) are the only failures that crept in, on Linux at least. So hopefully I'll be able to get this landed ASAP.

Comment 15

[Andrew Halberstadt [:ahal]]

Comment on attachment 8676313 [details]
MozReview Request: Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Comment 16

[Boris Zbarsky [:bzbarsky]]

> so was happy that my patch turned the jobs orange like it was supposed to.

Ah, makes sense.  Thank you very much for dealing with this, by the way!

Comment 17

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8676313 [details]
MozReview Request: Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

https://reviewboard.mozilla.org/r/22653/#review20155

I'm not familiar with this particular file, but this looks like the right fix to me.

::: testing/mozharness/mozharness/mozilla/building/buildbase.py:197
(Diff revision 1)
>              else:
>                  self.fail_count += 1

I think self.fail_count += 1 should be outside of this else.

Comment 18

[Andrew Halberstadt [:ahal]]

Here's a try run with the mozprocess tests fixed:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f96eede62f8

Still might be a problem on osx, though one of the failures is due to the log being too big.. I don't think that would have been hidden by the output parser before :/

Comment 19

[Andrew Halberstadt [:ahal]]

Actually here's a try run with all the mozprocess issues fixed, and showing green despite the too large log files. Just waiting for bug 1215756 to land, and this should be good to go.

(The log file issue is likely bug 1217051 btw, but it doesn't appear to block this).

Comment 20

[Andrew Halberstadt [:ahal]]

Comment on attachment 8676313 [details]
MozReview Request: Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Bug 1139911 - Set proper buildbot status on make-check errors and test failures, r=chmanchester

Comment 21

[Andrew Halberstadt [:ahal]]

Green! The log size issue was also fixed by ted.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f0073d35f0b

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b859d76b0451

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b859d76b0451