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

RESOLVED FIXED in mozilla33

Status

Testing
Mochitest
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mayhemer, Assigned: vaibhav1994)

Tracking

Trunk
mozilla33
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

4 years ago
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.
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.
(Assignee)

Comment 2

3 years ago
The temporary profile stays even for browser-chrome tests. I will work on this bug.
(Assignee)

Updated

3 years ago
Assignee: nobody → vaibhavmagarwal
(Assignee)

Comment 3

3 years ago
Created attachment 8441391 [details] [diff] [review]
cleanup.patch

This patch fixes up the issue.
Attachment #8441391 - Flags: review?(jmaher)
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.
Attachment #8441391 - Flags: review?(jmaher) → review-
(Assignee)

Comment 5

3 years ago
Created attachment 8441593 [details] [diff] [review]
cleanup.patch

Made changes in mozprofile/profile.py rather than runtests.py (thanks jmaher for the insight!). Also did some general cleanup.
Attachment #8441391 - Attachment is obsolete: true
Attachment #8441593 - Flags: review?(jmaher)
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.
Attachment #8441593 - Flags: review?(jmaher) → review-
(Assignee)

Comment 7

3 years ago
Created attachment 8442846 [details] [diff] [review]
cleanup.patch

This patch fixes the problem. It passes on try: https://tbpl.mozilla.org/?tree=Try&rev=772e82c4a31e
Attachment #8441593 - Attachment is obsolete: true
Attachment #8442846 - Flags: review?(jmaher)
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!
Attachment #8442846 - Flags: review?(jmaher) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
This is bitrotted. Please rebase.
Keywords: checkin-needed
(Assignee)

Comment 10

3 years ago
Created attachment 8444598 [details] [diff] [review]
cleanup.patch (bitrot-fixed)
Attachment #8442846 - Attachment is obsolete: true
Attachment #8444598 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dea6aefe2f7
Keywords: checkin-needed
(Assignee)

Comment 12

3 years ago
Created attachment 8445187 [details] [diff] [review]
cleanup-continued.patch

Another patch in continuation with previous patch.
Attachment #8445187 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Attachment #8445187 - Flags: review+ → review?(jmaher)
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.
Attachment #8445187 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/9dea6aefe2f7
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
landed followup from bitrot cleanup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf816b147823
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/cf816b147823
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.