853478 - Junit report generation fails if Mozmill is not able to send the report to the dashboard

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Automation, defect, P1)

Description

[Daniela Petrovici]

There was a failure today without a link to Mozmill dashboard. The Jenkins console shows:
INFO Passed: 39
INFO Failed: 0
INFO Skipped: 3
timed out
*** Removing old installation at /tmp/tmp2OLIc7.binary/
*** Removing repository '/tmp/tmpsEyHav.mozmill-tests'
Recording test results
No test report files were found. Configuration error?
Build step 'Publish JUnit test result report' changed build result to FAILURE

It seems that there was a timeout when submitting the report to Mozmill dashboard.

The error message should be improved or in case it is possible:
- to delay until the JUnit report was written
- automatically retrying the sending of the report

Comment 1

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

The situation here is that Mozmill itself returns with a non zero exit code when the report cannot be sent. It's a good question if someone should consider this a fail or not. I for my self wouldn't necessarily do that.

Dave, what do you think? Shall we update Mozmill to return with 0 on exit even if the report cannot be sent?

Comment 2

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

I'm not sure. Strictly speaking I think Mozmill should exit with a non-zero code if it fails to submit the report. Perhaps we could catch this in the automation scripts and re-raise it at the end (so we can write the JUnit report, etc). I'm fine with the build failing so long as it's easy to determine that the only issue was the report could not be submitted.

We could also enhance Mozmill to retry on failure, which might give us a little more resilience.

Comment 3

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

I wouldn't change Mozmill itself so it retries sending the report. If we fail in that something is broken on the other side which is unlikely to be fixed in the next couple of minutes. I don't want to hold off the finish status of the job.

So lets fetch the exception, run the junit report code, and re-throw the exception afterward. Or which might be better, run the junit report code in finally.

Comment 4

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

Just checked our code and we should simply switch the order of both lines so we call _generate_custom_report() first before invoking send_report().

Comment 5

[Andreea Matei [:AndreeaMatei]]

I have tried to reproduce this (or force it somehow to reproduce) in order to check the patch I made with Henrik's suggestion in comment 4. I wasn't able to get that error on my local jenkins and as far as I know, this only happened once on the ci?
I used smaller delay time for endurance, to make it faster, tried to block mozmill-crowd through /etc/hosts or use another invalid page, tried with smaller abort timeout.
Should I upload the patch anyway as it is an improvement?

Comment 6

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

Please check our mozmill-automation scripts and how the different reports are getting created. There is no need to do excessive testing. Please see my comment 4. A simple code move is enough here.

Comment 7

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1

Moved those lines between them as requested and tested on our local jenkins the change.

Comment 8

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

Comment on attachment 736818 [details] [diff] [review]
patch v1

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

::: libs/testrun.py
@@ +395,2 @@
>          if self.options.report_url:
>              self.send_report(self.options.report_url)

You will also have to move up the increment of the testrun_index. Otherwise we will overwrite the junit log we formerly created.

Comment 9

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2

Moved both lines up before calling send_report()

Comment 10

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

Comment on attachment 737849 [details] [diff] [review]
patch v2

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

Looks good to me. If Dave is happy too we can get this landed.

Comment 11

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

Looks great, I will go ahead and land this.

Comment 12

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

Comment on attachment 737849 [details] [diff] [review]
patch v2

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

Landed as:
http://hg.mozilla.org/qa/mozmill-automation/rev/3a22993b9996