Bug 96543 – [GTK] Clear application cache between tests in DumpRenderTree

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[GTK] Clear application cache between tests in DumpRenderTree

[Zan Dobersek]

Created attachment 163671 [details]
Patch

[Philippe Normand]

Comment on attachment 163671 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=163671&action=review

> Tools/Scripts/webkitpy/layout_tests/port/gtk.py:-70
> +        environment['XDG_CACHE_HOME'] = self._filesystem.join(self.results_directory(), 'appcache')
>          self._copy_value_from_environ_if_set(environment, 'WEBKITOUTPUTDIR')
> -        if self.get_option('webkit_test_runner'):
> -            # FIXME: This is a workaround to ensure that testing with WebKitTestRunner is started with
> -            # a non-existing cache. This should be removed when (and if) it will be possible to properly
> -            # set the cache directory path through a WebKitWebContext.
> -            environment['XDG_CACHE_HOME'] = self._filesystem.join(self.results_directory(), 'appcache')

Not sure I understand this change. If the appcache is cleared properly why do we still need to set XDG_CACHE_HOME to point to the results directory?

[Zan Dobersek]

(In reply to comment #2)
> 
> Not sure I understand this change. If the appcache is cleared properly why do we still need to set XDG_CACHE_HOME to point to the results directory?

One reason would be to avoid manipulating the default user's cache (~/.cache/) which is used by Epiphany as well.
Another reason is to start every test run (at least on the builders, with each build) with a clean, non-existent cache.

I can put a comment above this change, explaining why this is done.

[Philippe Normand]

(In reply to comment #3)
> (In reply to comment #2)
> > 
> > Not sure I understand this change. If the appcache is cleared properly why do we still need to set XDG_CACHE_HOME to point to the results directory?
> 
> One reason would be to avoid manipulating the default user's cache (~/.cache/) which is used by Epiphany as well.

Well Epiphany uses a custom directory I hope?

> Another reason is to start every test run (at least on the builders, with each build) with a clean, non-existent cache.
> 

But that's exactly the purpose of the method added in DRT support no?

[Zan Dobersek]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > 
> > > Not sure I understand this change. If the appcache is cleared properly why do we still need to set XDG_CACHE_HOME to point to the results directory?
> > 
> > One reason would be to avoid manipulating the default user's cache (~/.cache/) which is used by Epiphany as well.
> 
> Well Epiphany uses a custom directory I hope?

I can't tell for sure, the cache directory is by default set to $XDG_CACHE_HOME/webkitgtk/applications:
http://trac.webkit.org/browser/trunk/Source/WebKit/gtk/webkit/webkitglobals.cpp#L585

> > Another reason is to start every test run (at least on the builders, with each build) with a clean, non-existent cache.
> > 
> 
> But that's exactly the purpose of the method added in DRT support no?

The main use of that method is to clean the cache between each test. Setting XDG_CACHE_HOME env just assures we're not poking around user's actual WebKitGTK+ cache and that the testing is started with an empty cache directory.

[Zan Dobersek]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Another reason is to start every test run (at least on the builders, with each build) with a clean, non-existent cache.
> > > 
> > 
> > But that's exactly the purpose of the method added in DRT support no?
> 
> The main use of that method is to clean the cache between each test. Setting XDG_CACHE_HOME env just assures we're not poking around user's actual WebKitGTK+ cache and that the testing is started with an empty cache directory.

Another option is to point XDG_CACHE_HOME to a temporary directory (if it's decided overriding XDG_CACHE_HOME is acceptable).

[Philippe Normand]

Ok I think it's fine to use the results directory then. Thanks for the clarification!

[Zan Dobersek]

(In reply to comment #7)
> Ok I think it's fine to use the results directory then. Thanks for the clarification!

So r+ maybe? :>

[Philippe Normand]

Comment on attachment 163671 [details]
Patch

r=me

[Zan Dobersek]

(In reply to comment #9)
> (From update of attachment 163671 [details])
> r=me

Thanks!

[Zan Dobersek]

Comment on attachment 163671 [details]
Patch

Clearing flags on attachment: 163671

Committed r128453: <http://trac.webkit.org/changeset/128453>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by 96681

[Zan Dobersek]

Created attachment 164115 [details]
Patch

[Zan Dobersek]

Comment on attachment 164115 [details]
Patch

Clearing flags on attachment: 164115

Committed r128596: <http://trac.webkit.org/changeset/128596>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.