857956 - testrun_update.py needs to report a Failure if the "restore_binary" method fails

Status: RESOLVED FIXED

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

Description

[Andrei Eftimie]

On Windows the restore_binary method fails silently ont he following lines:
> shutil.rmtree(self._folder, True)
> shutil.move(self._backup_folder, self._folder)

"rmtree" fails to delete the whole folder, then
"move" moves the _backup_folder into _folder (instead of renaming itself to it).

We then silently fail to run the DirectUpdate test (we miss the firefox build in the correct location).
More investigation info can be found here: https://github.com/mozilla/mozmill-ci/issues/223

Such a failure should raise an exception and not yield to a successful report.

Comment 1

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

Attachment: Patch v1

Patch originally attached to bug 858686 and which fits better here.

Comment 2

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

Comment on attachment 734563 [details] [diff] [review]
Patch v1

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

A couple of nits, but r=me with these addressed.

::: libs/testrun.py
@@ +883,5 @@
>  
>      def restore_binary(self):
>          """ Restores the backup of the application binary. """
> +        timeout_rmtree = 15
> +        done = time.time() + timeout_rmtree

Can we call this timeout or max_time rather than done, which sounds more like a success boolean.

@@ +888,2 @@
>  
> +        print "*** Remove binary at '%s'" % self._folder

nit: Removing

@@ +898,5 @@
> +                    raise
> +                else:
> +                    time.sleep(1)
> +
> +        print "*** Restore backup from '%s'" % self._backup_folder

nit: Restoring

Comment 3

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

I addressed the nits and landed as this was blocking a release:
http://hg.mozilla.org/qa/mozmill-automation/rev/a3080c1a5297

Comment 4

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

Successful report: http://mozmill-ci.blargon7.com/#/update/report/cb9e33213cf3d2f37d7b0f8dcc34f8e7

Jenkins console included the following, indicating three failed attempts to remove the temporary folder, before eventually succeeding:

*** Removing binary at 'c:\users\mozauto\appdata\local\temp\tmpkiggp7.binary\'
[Error 145] The directory is not empty: 'c:\\users\\mozauto\\appdata\\local\\temp\\tmpkiggp7.binary\\'

[Error 5] Access is denied: 'c:\\users\\mozauto\\appdata\\local\\temp\\tmpkiggp7.binary\\maintenanceservice_installer.exe'

[Error 5] Access is denied: 'c:\\users\\mozauto\\appdata\\local\\temp\\tmpkiggp7.binary\\maintenanceservice_installer.exe'
*** Restoring backup from 'c:\users\mozauto\appdata\local\temp\tmpwt8omp.binary_backup'

Comment 5

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

Thanks for the landing Dave. Good to see that we make a little bit of progress on that stuff and that we correctly report now what's going wrong. It's a kinda mess what's going on with maintenanceservice_installer.exe here and I don't have a solution for it yet - see latest updates on bug 858686. 

By any means that was not really a blocker for the release tests given that we were affected by that for a longer time. The failure was simply not visible. Anyway, good to see that it has been landed.

I will forward port the patch to our 2.0 automation scripts today.