[rfc][icedtea-web] DeploymentPropertiesAreExposed reproducer fix

[Andrew Azores]

On 10/04/13 16:19, Omair Majid wrote:
> On 10/04/2013 04:10 PM, Andrew Azores wrote:
>> On 10/04/2013 03:38 PM, Omair Majid wrote:
>>> On 09/26/2013 02:58 PM, Andrew Azores wrote:
>>>> Actually, since the test is about checking that the
>>>> DeploymentConfiguration data is available through
>>>> System.getProperty()... is there any reason I can't just "abuse" the
>>>> DeploymentConfiguration singleton from JNLPRuntime again? Should work
>>>> just fine, I think still captures the "spirit" of this test, and also
>>>> means we can leave that constant in Defaults as default visibility.
>>> Sounds fine to me.
>>>
>> Here it is then.
>> +        DeploymentConfiguration config = JNLPRuntime.getConfiguration();
>> +        config.load();
> If you have to invoke load() manually, why not skip JNLPRuntime
> altogether and just use:
>
> DeploymentConfiguration config = new DeploymentConfiguration();
> config.load();
>
> Disclaimer: I did not try and compile this. It just seems a little
> awkward that you are using config through JNLPRuntime, but JNLPRuntime
> has not done its one job: calling config.load(). Of course, calling
> JNLPRuntime.initialize() is a big no-no.

Using DeploymentConfiguration directly is what I did last time I needed 
to read something for a reproducer like this, but Jiri was more 
comfortable with using the existing singleton instance, so that 
reproducer was changed and I stuck to the same pattern this time. What 
you said does work as well and does the exact same job as far as the 
reproducer is concerned.

>
>> +        String userLogDir = config.getProperty(DeploymentConfiguration.KEY_USER_LOG_DIR);
>> +        String expectedRegex = userLogDir + "/?";
> I am a little surprised here. Is this file named with a single character?
>
> Anyway, these are just nits. If this works correctly, feel free to push
> this.
>
> Thanks,
> Omair
>

Thanks,

-- 
Andrew A