1032255 - TPS has to exit with code != 0 in case of failures

Status: RESOLVED FIXED

(Testing Graveyard :: TPS, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Right now TPS always exits with code 0 even if failures happened. This should be changed so that it exists appropriately to the testrun status. This is necessary so that jenkins can detect if the tests were successful or not.

Comment 1

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v1.0

Henrik requested that I would have a look over this while hes gone and that Dave can review after an internal review:
https://etherpad.mozilla.org/automation-ask-an-expert

So here is the fix-patch for this, I added the exit code in the main class so we don't have to handle all different situation of single test run, group tests run etc.

Comment 2

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8463355 [details] [diff] [review]
patch v1.0

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

::: testing/tps/tps/cli.py
@@ +116,4 @@
>                        )
>      TPS.run_tests()
>  
> +    if TPS.numfailed > 0 or TPS.numpassed == 0:

Is it possible to skip tests? If so, we may have a situation where zero tests are run, and that might not actually indicate a failure. I think it might be best to just check if numfailed is > 0, but I'm not familiar with these tests so please convince if you think this is the best approach.

Comment 3

[Cosmin Malutan, [:cosmin-malutan]]

Here is the code where it switches the email address to send the notification email:
https://github.com/mozilla/gecko-dev/blob/RELEASE_BASE_20140602/testing/tps/tps/testrunner.py#L385

So far I haven't seen any skipped test but I think we would remove them from this list when they will fail, I'm not aware of any other skip mechanism.
I thought we might check for passed builds in case we have no test in https://github.com/mozilla/gecko-dev/blob/RELEASE_BASE_20140602/services/sync/tests/tps/all_tests.json and to be in sync with code that sends the emails. It wouldn't be ok to mark the build as PASS if we send a failure notification email. 
From my point of view we could either check only for failures and change the check for emails to or live it as it is. 
What do you think Dave?

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 8463355 [details] [diff] [review]
patch v1.0

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

Okay, that's enough to convince me. Thanks.

Comment 5

[Andrei Eftimie]

Comment on attachment 8463355 [details] [diff] [review]
patch v1.0

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

Given the way we handle things for email, and how this is structured it looks good to me.

Comment 6

[Cosmin Malutan, [:cosmin-malutan]]

This needs to be checked in, it doesn't need a try run as the TPS tests are ran on pulse message.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7cce2d4f5055

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7cce2d4f5055

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We also need to get this backported to aurora and beta.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/924e2d5187e3
https://hg.mozilla.org/releases/mozilla-beta/rev/1b581df81c9b