925699 - pymake/mach mochitest-plain don't delete the temporary profile

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Honza Bambas (:mayhemer)]

This applies to xpcshell tests as well.  

I was recently running a single test in a loop.  Suddenly there were 0 bytes free on my hard drive!  So I started to investigate and found out that temp profiles under c:\Users\<user>\AppData\Local\Temp\ are all there!  And take 30 GB!

So, I have to delete them manually, all the time.  No idea when this started to regress.  I didn't take a look at what was the date of the oldest tmp dir.. sorry.

Comment 1

[(not currently active) Ted Mielczarek]

I think this is supposed to do the cleanup:
http://hg.mozilla.org/mozilla-central/annotate/7a2edc5171e6/testing/mochitest/runtests.py#l681

but apparently that doesn't always get called:
http://hg.mozilla.org/mozilla-central/annotate/7a2edc5171e6/testing/mochitest/runtests.py#l1077

This is probably a regression from bug 746243.

Comment 2

[Vaibhav (:vaibhav1994)]

The temporary profile stays even for browser-chrome tests. I will work on this bug.

Comment 3

[Vaibhav (:vaibhav1994)]

Attachment: cleanup.patch

This patch fixes up the issue.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8441391 [details] [diff] [review]
cleanup.patch

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

::: testing/mochitest/runtests.py
@@ +1053,5 @@
>      """ remove temporary files and profile """
>      if self.manifest is not None:
>        os.remove(self.manifest)
> +    if options.profilePath:
> +      shutil.rmtree(options.profilePath)

my understanding is self.profile (a mozprofile object) would clean this up, especially when we do a 'del self.profile'.  Could you double check in the mozprofile api to see if there is a cleanup there.  Otherwise this is a decent and simple fix.

Comment 5

[Vaibhav (:vaibhav1994)]

Attachment: cleanup.patch

Made changes in mozprofile/profile.py rather than runtests.py (thanks jmaher for the insight!). Also did some general cleanup.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8441593 [details] [diff] [review]
cleanup.patch

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

I think you have really dug in and tackled this, but a more obscure use case is lost where we would want to preserve the profile.  Is there a way we could adjust our calls to mozprofile instead of modifying mozprofile?

::: testing/mozbase/mozprofile/mozprofile/profile.py
@@ -64,5 @@
>          if profile:
>              # Ensure we have a full path to the profile
>              self.profile = os.path.abspath(os.path.expanduser(profile))
> -        else:
> -            self.profile = tempfile.mkdtemp(suffix='.mozrunner')

here we do not create a new profile.

@@ +121,5 @@
>              if getattr(self, 'webapps', None) is not None:
>                  self.webapps.clean()
>  
>              # If it's a temporary profile we have to remove it
> +            if self.profile:

This could yield a potential problem.  There are cases where we might want to keep the profile.  With the changes here, we always delete the profile.

Comment 7

[Vaibhav (:vaibhav1994)]

Attachment: cleanup.patch

This patch fixes the problem. It passes on try: https://tbpl.mozilla.org/?tree=Try&rev=772e82c4a31e

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8442846 [details] [diff] [review]
cleanup.patch

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

can you link to your try server push, this looks great!

::: testing/mochitest/runtests.py
@@ +1358,5 @@
>        print "testpath: %s" % options.testPath
>  
> +      # If we are using --run-by-dir, we should not use the profile path (if) provided
> +      # by the user, since we need to create a new directory for each run. We would face problems
> +      # if we use the directory provided by the user.

thanks for the great comment here.

::: testing/mozbase/mozprofile/mozprofile/profile.py
@@ +105,5 @@
>          self.webapps = WebappCollection(profile=self.profile, apps=self._apps)
>          self.webapps.update_manifests()
>  
>      def __del__(self):
> +        self.cleanup()

yay for fixing spacing issues!

Comment 9

[Ryan VanderMeulen [:RyanVM]]

This is bitrotted. Please rebase.

Comment 10

[Vaibhav (:vaibhav1994)]

Attachment: cleanup.patch (bitrot-fixed)

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9dea6aefe2f7

Comment 12

[Vaibhav (:vaibhav1994)]

Attachment: cleanup-continued.patch

Another patch in continuation with previous patch.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8445187 [details] [diff] [review]
cleanup-continued.patch

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

easy to miss stuff while cleaning up bitrot.

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9dea6aefe2f7

Comment 15

[Joel Maher ( :jmaher ) (UTC -8)]

landed followup from bitrot cleanup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf816b147823

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/cf816b147823