Code Review Request: 7198073: (prefs) user prefs not saved [macosx]

[Kurchi Hazra]

Hi Alan,

    Please find an updated webrev with a test included: 
http://cr.openjdk.java.net/~khazra/7198073/webrev.01/

CheckUserPrefsStorage.sh is the main entry point - it runs 
CheckUserPrefFirst first (which creates and attempts to store a preference),
and then runs CheckUserPrefLater (which attempts to retrieve the 
preference stored by the former).

Thanks,
Kurchi


On 12.10.2012 07:57, Kurchi Subhra Hazra wrote:
> On 10/12/12 12:43 AM, Alan Bateman wrote:
>> On 12/10/2012 01:21, Kurchi Hazra wrote:
>>> Hi,
>>>
>>>    This is a regression introduced by the fix for 7160252[1] that I 
>>> pushed a few weeks ago. We should
>>> really be using the class member field isUser, rather than the 
>>> argument passed in the constructor as a parameter
>>> for cfFileForNode(). Due to this wrong parameter being passed, user 
>>> preferences were never getting stored into
>>> permanent storage.
>>>
>>> Webrev: http://cr.openjdk.java.net/~khazra/7198073/webrev.00/
>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7198073
>>>
>>> Thanks,
>>> Kurchi
>>>
>>> [1] http://cr.openjdk.java.net/~khazra/7160252/webrev.02/
>> Kurchi - thanks for tracking this one down, looking at it now it is 
>> obvious how this slipped through. I think the proposed change is okay 
>> although I think we need to do a bit of clean-up on this code at some 
>> point (not for this change, I realize this one needs to be fixed in 
>> 7u and I don't want to muddy the waters).
>  Right, I will have to spend more time cleaning up this after 
> committing this fix.
>
>> The other thin is tests. The preferences implementation that came via 
>> the Mac port is turning out to have a bit of a bug tail and one or 
>> two regressions have sneaked in along the way too. Do you think you 
>> could add a test for this issue? 
>  Let me get back to you on this.
>
>> Also at some point we need to look at the test coverage and quality 
>> of the tests for this area as most/all of these Mac specific issues 
>> should have been caught before it ever went in.
>  I agree. Again, this will be a longer time goal for me.
>
> - Kurchi
>

-- 
-Kurchi