1347177 - Custom mach command should cause job to fail when it fails

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[William Lachance (:wlach)]

Noticed when trying to custom retrigger for bug 1318389 that we're not actually failing the job when the custom test fails:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f2ad538f1c9939d57591e6f06a7a6e7750bd118&selectedJob=83496495

We really should. I think I just need to modify the bash script a bit we use to run mochitests.

Comment 1

[William Lachance (:wlach)]

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

Comment 2

[William Lachance (:wlach)]

Attachment: Bug 1347177 - When doing custom retrigger, job should fail when custom mach command fails

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

Comment 3

[William Lachance (:wlach)]

Comment on attachment 8847242 [details]
Bug 1347177 - When doing custom retrigger, job should fail when custom mach command fails

Hey Dustin, I know this isn't necessarily your area but the usual person I'd ask to review this (:ahal) seems to be away, and this seems to be a trivial change to me.

Comment 4

[William Lachance (:wlach)]

It seems like in some cases mach returns an exit code of 1 even if there was a failure (in this case a leak):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9f1ae807d384f42912547621443e46db90b00f7&selectedJob=83758884

I'll file another bug to look into that.

Comment 5

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

Comment on attachment 8847242 [details]
Bug 1347177 - When doing custom retrigger, job should fail when custom mach command fails

https://reviewboard.mozilla.org/r/120246/#review122162

::: taskcluster/scripts/tester/test-ubuntu.sh:193
(Diff revision 1)
>  fi
>  
>  # Run a custom mach command (this is typically used by action tasks to run
>  # harnesses in a particular way)
>  if [ "$CUSTOM_MACH_COMMAND" ]; then
>      eval "/home/worker/workspace/build/tests/mach ${CUSTOM_MACH_COMMAND}"

Do you think this was supposed to be "exec"?  That would have roughly the same effect..

Comment 6

[William Lachance (:wlach)]

Comment on attachment 8847242 [details]
Bug 1347177 - When doing custom retrigger, job should fail when custom mach command fails

https://reviewboard.mozilla.org/r/120246/#review122162

> Do you think this was supposed to be "exec"?  That would have roughly the same effect..

Would the cleanup part of this script still get called if we called exec?

Comment 7

[William Lachance (:wlach)]

Comment on attachment 8847242 [details]
Bug 1347177 - When doing custom retrigger, job should fail when custom mach command fails

https://reviewboard.mozilla.org/r/120246/#review122642

::: taskcluster/scripts/tester/test-ubuntu.sh:193
(Diff revision 1)
>  fi
>  
>  # Run a custom mach command (this is typically used by action tasks to run
>  # harnesses in a particular way)
>  if [ "$CUSTOM_MACH_COMMAND" ]; then
>      eval "/home/worker/workspace/build/tests/mach ${CUSTOM_MACH_COMMAND}"

So the simple answer to this question is "no", the cleanup part would not be called. There are hacks around this, but using eval and returning $? is the right thing here, I think. I'm going to go ahead and land this as-is.

http://stackoverflow.com/questions/24111981/how-can-i-achieve-bash-exit-trap-when-exec-ing-another-binary#24112832

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e500058bdd0
When doing custom retrigger, job should fail when custom mach command fails r=dustin

Comment 9

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

Huh, I could swear I replied to comment 6 to say "oh, good point, go ahead and land" sorry!

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8e500058bdd0