/hg/icedtea-web: Fixed "could not clear cache" message and cache...
Jiri Vanek
jvanek at redhat.com
Fri Sep 6 00:11:03 PDT 2013
On 09/05/2013 03:10 PM, Andrew Azores wrote:
> On 09/05/13 05:28, Jacob Wisor wrote:
>> Jiri Vanek wrote:
>>> On 09/04/2013 07:49 PM, aazores at icedtea.classpath.org wrote:
>>>> changeset bb8132ebc241 in /hg/icedtea-web
>>>> details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=bb8132ebc241
>>>> author: Andrew Azores <aazores at redhat.com>
>>>> date: Wed Sep 04 13:48:42 2013 -0400
>>>>
>>>> Fixed "could not clear cache" message and cache location in CacheReproducer
>>>>
>>>>
>>>> diffstat:
>>>>
>>>> ChangeLog | 8 +
>>>> netx/net/sourceforge/jnlp/config/Defaults.java | 2 +-
>>>> tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java | 95 +++++----
>>>> 3 files changed, 64 insertions(+), 41 deletions(-)
>>>>
>>>> diffs (228 lines):
>>>>
>>>> diff -r 6c0e37585a5e -r bb8132ebc241 ChangeLog
>>>> --- a/ChangeLog Wed Sep 04 12:22:16 2013 -0400
>>>> +++ b/ChangeLog Wed Sep 04 13:48:42 2013 -0400
>>>> @@ -1,3 +1,11 @@
>>>> +2013-09-04 Andrew Azores <aazores at redhat.com>
>>>> +
>>>> + * netx/net/sourceforge/jnlp/config/Defaults.java: (USER_CACHE_HOME) made
>>>> + public for use in CacheReproducer
>>>> + * tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest:
>>>> + updated "could not clear cache" message and cache location. Other minor
>>>> + cleanup
>>>> +
>>>> 2013-09-04 Andrew Azores <aazores at redhat.com>
>>>>
>>>> * netx/net/sourceforge/jnlp/security/SecurityDialogs.java:
>>>> diff -r 6c0e37585a5e -r bb8132ebc241 netx/net/sourceforge/jnlp/config/Defaults.java
>>>> --- a/netx/net/sourceforge/jnlp/config/Defaults.java Wed Sep 04 12:22:16 2013 -0400
>>>> +++ b/netx/net/sourceforge/jnlp/config/Defaults.java Wed Sep 04 13:48:42 2013 -0400
>>>> @@ -55,7 +55,7 @@
>>>> final static String SYSTEM_HOME = System.getProperty("java.home");
>>>> final static String SYSTEM_SECURITY = SYSTEM_HOME + File.separator + "lib" +
>>>> File.separator + "security";
>>>> final static String USER_CONFIG_HOME;
>>>> - final static String USER_CACHE_HOME;
>>>> + public final static String USER_CACHE_HOME;
>>>> final static String USER_SECURITY;
>>>> final static String LOCKS_DIR = System.getProperty("java.io.tmpdir") + File.separator
>>>> + System.getProperty("user.name") + File.separator + "netx" + File.separator
>>>> diff -r 6c0e37585a5e -r bb8132ebc241
>>>> tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java
>>>> --- a/tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java Wed Sep 04
>>>> 12:22:16 2013 -0400
>>>> +++ b/tests/reproducers/signed/CacheReproducer/testcases/CacheReproducerTest.java Wed Sep 04
>>>> 13:48:42 2013 -0400
>>>> @@ -44,11 +44,13 @@
>>>> import java.io.UnsupportedEncodingException;
>>>> import java.util.Arrays;
>>>> import java.util.List;
>>>> +import java.util.PropertyResourceBundle;
>>>> import java.util.regex.Matcher;
>>>> import java.util.regex.Pattern;
>>>> import net.sourceforge.jnlp.ServerAccess;
>>>> -import net.sourceforge.jnlp.ServerAccess.ProcessResult;
>>>> +import net.sourceforge.jnlp.ProcessResult;
>>>> import net.sourceforge.jnlp.annotations.KnownToFail;
>>>> +import net.sourceforge.jnlp.config.Defaults;
>>>> import org.junit.AfterClass;
>>>> import org.junit.Assert;
>>>>
>>>> @@ -60,24 +62,37 @@
>>>> private static final List<String> clear = Arrays.asList(new
>>>> String[]{server.getJavawsLocation(), "-Xclearcache", ServerAccess.HEADLES_OPTION});
>>>> private static final List<String> trustedVerboses = Arrays.asList(new
>>>> String[]{"-Xtrustall", ServerAccess.HEADLES_OPTION,"-verbose"});
>>>> private static final List<String> verbosed = Arrays.asList(new String[]{"-verbose",
>>>> ServerAccess.HEADLES_OPTION});
>>>> - private static final String home = System.getProperty("user.home");
>>>> - private static final String name = System.getProperty("user.name");
>>>> - private static final String tmp = System.getProperty("java.io.tmpdir");
>>>> - private static final File icedteaDir = new File(home + "/" + ".icedtea");
>>>> - private static final File icedteaCache = new File(icedteaDir, "cache");
>>>> - private static final File icedteaCacheFile = new File(icedteaCache, "recently_used");
>>>> - private static final File netxLock = new File(tmp + "/" + name + "/netx/locks/netx_running");
>>>> +
>>>> private static final String lre = "LruCacheException";
>>>> private static final String ioobe = "IndexOutOfBoundsException";
>>>> private static final String corruptRegex = "\\d{13}";
>>>> private static final Pattern corruptPatern = Pattern.compile(corruptRegex);
>>>> private static final String corruptString = "156dsf1562kd5";
>>>>
>>>> - String testS = "#netx file\n"
>>>> - + "#Mon Dec 12 16:20:46 CET 2011\n"
>>>> - +
>>>> "1323703236508,0=/home/xp13/.icedtea/cache/0/http/localhost/ReadPropertiesBySignedHack.jnlp\n"
>>>> - +
>>>> "1323703243086,2=/home/xp14/.icedtea/cache/2/http/localhost/ReadProperties.jar\n"
>>>> - +
>>>> "1323703243082,1=/home/xp15/.icedtea/cache/1/http/localhost/ReadPropertiesBySignedHack.jar";
>>>> + private static final File icedteaCache = new File(Defaults.USER_CACHE_HOME, "cache");
>>>> + private static final File icedteaCacheFile = new File(icedteaCache, "recently_used");
>>>> + private static final File netxLock = new File(System.getProperty("java.io.tmpdir"),
>>>> + System.getProperty("user.name") + File.separator +
>>>> + "netx" + File.separator +
>>>> + "locks" + File.separator +
>>>> + "netx_running");
>>>> +
>>>> + private static final String messageResourcePath =
>>>> "net/sourceforge/jnlp/resources/Messages.properties";
>>>> + private static PropertyResourceBundle messageBundle = null;
>>>> +
>>>> + static {
>>>> + try {
>>>> + messageBundle =
>>>> + new
>>>> PropertyResourceBundle(CacheReproducerTest.class.getClassLoader().getResourceAsStream(messageResourcePath));
>>>>
>>>> + } catch (IOException e) {
>>>> + }
>>>> + }
>>>> +
>>>
>>> Pleae NEVER consume an exception.
>>>
>>> In this case the
>>>
>>> } catch (IOException ex) {
>>> throw new RuntimeException(ex);
>>> }
>>
>> Throwing a chained RuntimeException is going to disrupt the program's flow. So, I would rather advise
>>
>> } catch (IOException e) {
>> e.printStackTrace();
>> }
>>
>> This will make IOException at least visible on stderr and preserve what has been previously
>> intended, namely continuing program execution undisrupted.
>>
>>> Would be the best. You can push this without any other review (unless you think about different fix)
>>
>> Regards,
>> Jacob
>
> As Jacob says, I'd rather not throw a chained exception here because that would cause the entire
> reproducer to halt, would it not? But if the messageBundle is not properly instantiated that's only
> really a blocker for one test (clearCacheUnsucessfully). This exception being consumed would
> manifest as a NPE in that later test, which would just result in that single test failing. Printing
> the stack trace is agreeable enough but I don't really think the entire reproducer needs to stop if
> we're going to be unable to run one test.
>
imho nope. If messages.properties would become unavailable, then it is so crucial that I would be
happy even for whole test run failure.
Also not
>> } catch (IOException e) {
>> e.printStackTrace();
>> }
but
>> } catch (IOException e) {
ServerAccess.logException(ex);
>> }
(never use stdout/err in testcases, but always appropriate ServerAccess.log* function)
In this particular case do as you wish. My recommendation would lead to separate changeset, where
you will create static method getMessage(String key) somewhere in
test-extensions/net/sourceforge/jnlp/tools (probably new utility class) which will encapsulate the
>>>> + private static final String messageResourcePath =
>>>> "net/sourceforge/jnlp/resources/Messages.properties";
>>>> + private static PropertyResourceBundle messageBundle = null;
>>>> +
>>>> + static {
>>>> + try {
>>>> + messageBundle =
>>>> + new
>>>>
PropertyResourceBundle(CacheReproducerTest.class.getClassLoader().getResourceAsStream(messageResourcePath));
>>>>
>>>> + } catch (IOException e) {
with throw here :)
>>>> + }
>>>> + }
and
>>>> + String cacheClearError = messageBundle.getString("CCannotClearCache");
parts.
The reason for this is, that the hardocded "evaluate string" is wide spread issue, and I believe
some of the failures are because of small change in properties file (eg the "the" removal/addition
or something like that)
Also you can imprve it a bit more for localised messages too, and then make localisation
reproducers/unittests much thinner. But this is even another chapter.
Anyway I do not insists.
More information about the distro-pkg-dev
mailing list