[rfc][icedtea-web] Fix CacheReproducerTest

[Jiri Vanek]

On 08/30/2013 01:06 PM, Jacob Wisor wrote:
> "Andrew Azores"<aazores at xxxxxxxxxx> wrote:
>> On 08/29/2013 09:59 AM, Jiri Vanek wrote:
>>> On 08/29/2013 03:13 PM, Andrew Azores wrote:
>>>> Changelog:
>>>> *
>>>> tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java:
>>>> (icedteaDir) field
>>>> updated to reflect new ITW cache location from XDG spec.
>>>> (clearCacheUnsucessfully) checks for
>>>> correct "can not clear the cache" error message
>>>>
>>>> Not much going on here, really :). This is the start of my effort to
>>>> stabilize reproducers. Looks
>>>> like I got lucky with my first set to fix, because this turned out to
>>>> be very very easy. There is
>>>> one "KnownToFail" test in here that occasionally passes, so I will
>>>> also look into that, but these
>>>> two small changes stand on their own to fix the rest of the tests at
>>>> least.
>>>>
>>>> Thanks,
>>>>
>>>
>>> Nice catch. My wrong I overlooked the hard coded path.
>>>
>>> However - the test case can see the libraries, and the real
>>> cache(/config) path is now in Defaults. So I think it would be better
>>> to get rid of this hard coded path completely and replace by
>>> variable[1] (there was no variable in time of this test writing). This
>>> variable should be filled correctly no meter if custom XDG variable i
>>> set.
>>>
>>>
>>> J.
>>>
>>> [1]http://icedtea.classpath.org/hg/icedtea-web/file/420d72e5cee7/netx/net/sourceforge/jnlp/config/Defaults.java#l58
>>>
>>>
>>> If you do not wont to use the full path (but why not), then maybe just
>>> http://icedtea.classpath.org/hg/icedtea-web/file/420d72e5cee7/netx/net/sourceforge/jnlp/config/DeploymentConfiguration.java#l53
>>>
>>
>> Like this you mean? I had to add the test to the same package as the
>> Defaults class since that variable is set to default visibility, not public.


Nope, rather make it public.

Anyaway, as others suggested, Can you tray to use DeploymentConfiguration singleton to get real
location of cache. If you think it is good idea, then go with it.
>>
> 
> Even at the risk this may be perceived as nagging, but you may want to consider replacing those "/" String literals with File.separator, especially in places where the string concatinations are derived from system properties. It is probably no problem to pass slashes ( / ), backslashes ( \ ), or any other system specific file separator to File's constuctors or objects because File converts any paramters into URLs internally anyway. But when manipulating or comparing raw strings one may get undefined or different results. Or, one should make sure to use URLs only and stick to those when manipulating or comparing strings.
> 
> Another thing that perhaps should be considered are systems that do not - for what ever reason - implement the XDG specification. What should be IcedTea-Web's expected behavior in this case? Should it simply force XDG onto that system or should it rather fall back to the previously employed simple scheme (System.getProperty("user.home") + File.separator + ".icedtea")?
> 
> Regards,
> Jacob
>