/hg/icedtea-web: Fixed "could not clear cache" message and cache...

[Andrew Azores]

On 09/09/2013 03:51 AM, Jiri Vanek wrote:
> On 09/06/2013 04:40 PM, Andrew Azores wrote:
>> On 09/06/2013 03:11 AM, Jiri Vanek wrote:
>>> On 09/05/2013 03:10 PM, Andrew Azores wrote:
>>>> (snip)
>>>> As Jacob says, I'd rather not throw a chained exception here 
>>>> because that would cause the entire
>>>> reproducer to halt, would it not? But if the messageBundle is not 
>>>> properly instantiated that's only
>>>> really a blocker for one test (clearCacheUnsucessfully). This 
>>>> exception being consumed would
>>>> manifest as a NPE in that later test, which would just result in 
>>>> that single test failing. Printing
>>>> the stack trace is agreeable enough but I don't really think the 
>>>> entire reproducer needs to stop if
>>>> we're going to be unable to run one test.
>>>>
>>>
>>>
>>> imho nope. If messages.properties would become unavailable, then it 
>>> is so crucial that I would be happy even for whole test run failure.
>>
>> Hm yea, I suppose that's true. However, see below -
>>
>>>
>>> Also not
>>> >> } catch (IOException e) {
>>> >> e.printStackTrace();
>>> >> }
>>>
>>> but
>>>
>>> >> } catch (IOException e) {
>>> ServerAccess.logException(ex);
>>> >> }
>>>
>>> (never use stdout/err in testcases, but always appropriate 
>>> ServerAccess.log* function)
>>>
>>>
>>>
>>> In this particular case do as you wish. My recommendation would lead 
>>> to separate changeset, where you will create static method 
>>> getMessage(String key) somewhere in 
>>> test-extensions/net/sourceforge/jnlp/tools (probably new utility 
>>> class) which will encapsulate the
>>> >>>> + private static final String messageResourcePath =
>>> >>>> "net/sourceforge/jnlp/resources/Messages.properties";
>>> >>>> + private static PropertyResourceBundle messageBundle = null;
>>> >>>> +
>>> >>>> + static {
>>> >>>> + try {
>>> >>>> + messageBundle =
>>> >>>> + new
>>> >>>> 
>>> PropertyResourceBundle(CacheReproducerTest.class.getClassLoader().getResourceAsStream(messageResourcePath));
>>> >>>>
>>> >>>> + } catch (IOException e) {
>>>
>>> with throw here :)
>>>
>>> >>>> + }
>>> >>>> + }
>>> and
>>> >>>> + String cacheClearError = 
>>> messageBundle.getString("CCannotClearCache");
>>>
>>>
>>> parts.
>>>
>>> The reason for this is, that the hardocded "evaluate string" is wide 
>>> spread issue, and I believe some of the failures are because of 
>>> small change in properties file (eg the "the" removal/addition or 
>>> something like that)
>>>
>>> Also you can imprve it a bit more for localised messages too, and 
>>> then make localisation reproducers/unittests much thinner. But this 
>>> is even another chapter.
>>>
>>> Anyway I do not insists.
>>>
>>>
>>>
>>>
>>
>> I do like this idea, and I whipped up a quick utility class for it 
>> just now. But a side effect of extracting this logic out into a 
>> helper class is that the IOException thrown if the 
>> Messages.properties file(s) are not available is now only thrown when 
>> the new utility class is used. So in the case of this 
>> CacheReproducer, it only happens in the clearCacheUnsuccessfully test 
>> method. If something goes wrong in the utility class, eg 
>> Messages.properties is not available, then only this test fails. If 
>> we want the "stop the whole reproducer" behaviour we'll need to use 
>> static blocks or @BeforeClass annotated methods or something to force 
>> checking if the resources are available.
>>
>> Thanks,
>>
>
> I would consider it as ok :)
>
> if you implement properties  singleton like this:
>
>
> getSingleton(){
> if (singleton=null) then loadSingleton()
> }
>
> getMessage(key){
> return getSingleton().getMessage(key)
> }
>
>
> Then relevant exception will be thrown during each use. So I guess the 
> behaviour will be most correct.
>
> Thanx for stitching with it!
>
> J.
>

Changelog:
* 
tests/test-extensions/net/sourceforge/jnlp/tools/MessageProperties.java: 
new helper class for retrieving values from Messages.properties
* 
tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java: 
refactor to use new MessageProperties helper class

This'll throw an exception whenever you call #getMessage, so individual 
test methods using this will fail if the resource is not available, but 
the whole reproducer won't halt.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: CacheReproducer2.patch
Type: text/x-patch
Size: 4898 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130909/60f6fd2b/CacheReproducer2.patch