[rfc][icedtea-web] fixing CacheReproducerTest and improving VersionedJarTest

[Jie Kang]

----- Original Message -----
> snip
> > However. Later I noted that recently_used file is not handled via
> > PahsAndFiles.
> >
> > I moved it here, which needed some more changes - like make it properly
> > testable and so get rid of
> > this untestable ENUM and replace it by getter singleton with possibility to
> > make testable instance.
> >
> > I agree that this patch was written in rush, and needs  proper review.
> > J.

Hello,

Nits below:

+    public final PropertiesFile cacheOrder;
+    public final File cacheOrderParent;
+    public final File cacheOrderFile;

The name 'cacheOrder' isn't very descriptive. Can you change it to recentlyUsedFile or something similar?

Can you use getters instead of public variables? Then you can reduce it to just one variable.

e.g.

private final PropertiesFile recentlyUsed;

public PropertiesFile getProperties() { ... }
public File getParentFile() { ... }
public File getFile() { ... }

Names of function are up to you.

+    private static final CacheLRUWrapper clw = new CacheLRUWrapper(new File(System.getProperty("java.io.tmpdir"),cacheIndexFileName));

Space after the comma: ("java.io.tmpdir"),cacheIndexFileName

Also, this will set the cache to the tmp directory. Can you create a tempcache directory inside tmp and then use that instead?

It's a little bit cleaner. And please make sure the tests clean themselves up afterwards using file.deleteOnExit(), etc.

+    private static final int PERNAMENT_FILES = 1;

s/PERNAMENT/PERMANENT


Side note:

I don't see how this is more testable than before. enum singleton and class singleton are equally testable to me. Can you provide any examples?


Regards,

> 
> ping
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern