Closed Bug 1182798 Opened 4 years ago Closed 4 years ago

Change the exit status of a Firefox UI update job if any of the update tests fails

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(firefox40 fixed, firefox41 fixed, firefox42 fixed)

RESOLVED FIXED
Tracking Status
firefox40 --- fixed
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: armenzg, Assigned: armenzg)

References

Details

Attachments

(1 file)

We now add a summary at the end of a run [1].
We should change the return code of the script if any locale fails.

This is a very simple fix.

[1] http://hg.mozilla.org/build/mozharness/file/default/scripts/firefox_ui_updates.py#l366
Assignee: nobody → armenzg
Attachment #8638040 - Flags: review?(jlund)
Comment on attachment 8638040 [details] [diff] [review]
[mozharness] status summary and set exit code

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

::: testing/mozharness/scripts/firefox_ui_updates.py
@@ +429,5 @@
>                          failed_locales.append(locale)
> +
> +                if failed_locales:
> +                    if exit_status == TBPL_SUCCESS:
> +                        self.info("\nSUMMARY - Firefox UI update tests failed locales:")

why the /n ? Does that work with mh loggging?

@@ +436,5 @@
> +
> +                    self.info(build_id)
> +                    self.info("  %s" % (', '.join(failed_locales)))
> +
> +            self.return_code = EXIT_STATUS_DICT[exit_status] 

question:

This will assign self.return_code with TBPL_SUCCESS or TBPL_WARNING regardless if you have already set that to something worse before hand.

Even if you don't touch self.return_code outside of this, would it be worth future proofing with something like: http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildbase.py#1365 ?
Attachment #8638040 - Flags: review?(jlund) → review+
I removed the \n and added a self.info("") before just in case Windows does something funky.

I'm happy to leave the setting of return codes as-is.
I just want to wrap this up and I don't foresee changes to this.
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.