478096 - leak test machines report red when they should be orange

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

In at least some cases of test failure (crash/hang), the leak test boxes report a red build rather than an orange one.

For an example, see http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234378261.1234382132.15587.gz

This should be reporting orange for test failure rather than red for build failure.

Comment 1

[[e-mail] (:bhearsum)]

It looks like flunkOnFailure is defaulted to True for ShellCommands, which the AliveTest steps are a subclass of. Let's try flipping that and see if that helps. Patch incoming.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

What happens if we don't stop when these steps fail ? Later steps rely on the logs they produce.

Comment 3

[[e-mail] (:bhearsum)]

Attachment: don't flunkOnFailure in AliveTest

Comment 4

[[e-mail] (:bhearsum)]

(In reply to comment #2)
> What happens if we don't stop when these steps fail ? Later steps rely on the
> logs they produce.

We won't be flipping haltOnFailure - we still want to stop. flunkOnFailure controls whether or not a BuildStep can turn the overall build red

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 361849 [details] [diff] [review]
don't flunkOnFailure in AliveTest

changeset:   190:4617dc0f2fe8

Comment 6

[[e-mail] (:bhearsum)]

I just landed & kicked a new build - let's see what happens.

Comment 7

[[e-mail] (:bhearsum)]

Yargh, this patch didn't fix it. I'll have a closer look tomorrow.

Comment 8

[[e-mail] (:bhearsum)]

Attachment: explicitly pass flunkOnFailure=False and flunkOnWarnings=False

I'm not sure why the last patch didn't resolve the issue, the step certainly ends up with FAILURE as its' status, according to the log, and the warnOnFailure/flunkOnFailure settings should turn it orange....In any case, let's try explicitly passing flunkOnFailure and flunkOnWarnings=False to AliveTest, which will end up getting passed down to ShellCommand.

Do class level attributes not override in Python? I think they have to since that's how ShellCommand overrides BuildStep's flunkOnFailure.....

Comment 9

[Chris AtLee [:catlee]]

Comment on attachment 362008 [details] [diff] [review]
explicitly pass flunkOnFailure=False and flunkOnWarnings=False

if haltOnFailure==True, then the step will be marked as FAILED in buildbot/process/base.py:507

Comment 10

[[e-mail] (:bhearsum)]

Attachment: haltOnFailure should not imply flunkOnFailure

Just landed this upstream. We can back out my second patch after landing this.

I *think* we can get this picked up right away by a reconfig.

Comment 11

[Chris AtLee [:catlee]]

Comment on attachment 362219 [details] [diff] [review]
haltOnFailure should not imply flunkOnFailure

Looks good, but I think we need a full restart to pick up this change since we don't reload the buildbot core on reconfig.

Comment 12

[[e-mail] (:bhearsum)]

You're right again Catlee. For some reason I thought by changes were in buildstep.py, not base.py.

I don't want to do a downtime just for this, but it will ride along in the next one.

Comment 13

[[e-mail] (:bhearsum)]

For posterity, the upstream bug is http://buildbot.net/trac/ticket/425

Comment 14

[[e-mail] (:bhearsum)]

Comment on attachment 362219 [details] [diff] [review]
haltOnFailure should not imply flunkOnFailure

changeset:   10:a2845f74d55d

Comment 15

[[e-mail] (:bhearsum)]

This landed in production.