[rfc][icedtea-web] fixing xdg reprodcuers

[Jie Kang]

----- Original Message -----
> On 03/12/2015 03:22 PM, Jie Kang wrote:
> >
> >
> > ----- Original Message -----
> >> hey were failing because of three issues
> >>    - first was affection by  [rfc][icedtea-web] fix itweb-settings set
> >>    command
> >>    to allow duplicate strings
> >>    - second was missing skip attributes check in faked
> >>    deployment.properties
> >>    - third were jars directories in .config/icedtea-web about which
> >>    transfer
> >>    from legacy to xdg wa not aware.
> >>
> >> All fixed on level of tests.
> >
> > Hello,
> >
> > Nice!
> >
> > A few nits below:
> >
> >       //intentionaly not using constants from itw to check itw
> >       private static final File oldRoot = new
> >       File(System.getProperty("user.home"), ".icedtea");
> > -    private static final File realCache;
> > -    private static final File realConfig;
> > -    private static final File homeCache = new
> > File(System.getProperty("user.home") + File.separator + ".cache" +
> > File.separator + "icedtea-web");
> > -    private static final File homeConfig = new
> > File(System.getProperty("user.home") + File.separator + ".config" +
> > File.separator + "icedtea-web");
> > +    private static final File realCache = new
> > File(PathsAndFiles.USER_CACHE_HOME);
> > +    private static final File realConfig = new File
> > (PathsAndFiles.USER_CONFIG_HOME);
> > +    private static final File homeCache = new File(realCache,
> > PathsAndFiles.DEPLOYMENT_SUBDIR_DIR);
> > +    private static final File homeConfig = new File(realConfig,
> > PathsAndFiles.DEPLOYMENT_SUBDIR_DIR);
> >
> > Are these changes necessary? The comment says it is intentional to not use
> > ITW constants which is sensible to me.
> 
> hmm.... Ok. I wil revert.
> 
> >
> > +        //we need fake security and manifests
> > +        File ff = new File(PathsAndFiles.USER_CONFIG_HOME);
> > +        try{
> >
> > Please fix the spacing and indentation here.
> >
> > +        fakeExtendedSecurity(ff);
> >           ProcessWrapper pw = new
> >           ProcessWrapper(server.getJavawsLocation(), null,
> >           server.getUrl("simpletest1.jnlp"), (ContentReaderListener) null,
> >           null, removeXdgVAlues());
> >           ProcessResult pr = pw.execute();
> >           Assert.assertTrue(simpletests1Run.toPassingString(),
> >           simpletests1Run.evaluate(pr.stdout));
> >           Assert.assertTrue(notMoving.toPassingString(),
> >           notMoving.evaluate(pr.stdout));
> >           assertMainFilesInHome(true, false, false);
> >           assertOldNotMainFilesInHome(true, true, true);
> > +        }finally{
> >
> > Spacing here as well
> >
> > +        try {
> >           assertNotMainFilesInHome(true, true, true);
> >
> > Please fix the indentation here.
> >
> > -                        "set", "blah", "blah"
> > +                        //one impl of new parser was unable to handle
> > duplicates
> > +                        "set", "blah", "differentBlah"
> 
> Sure I will fix it.
> >
> > Lukasz has a patch for this I think. I feel like you don't need these
> > changes if the patch goes through.
> 
> hmhm.  What seems to be better ? To test this weeknes, or prevent being
> affected by this weeknes?

I'd prefer keeping it the same just because the impl should be able to handle duplicates and it'd reduce the changes in the patch. But it's not a big deal so I leave it up to you.

Regards,

> >
> >
> > Regards,
> >
> >>
> Thanx!
> J.
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern