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

Kurchi Hazra kurchi.subhra.hazra at oracle.com
Tue Oct 16 00:53:56 UTC 2012

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/viewbug.do?bugid=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

More information about the core-libs-dev mailing list