[icedtea-web][discussion] Regression in unit test ResourceTrackerTest.downloadResource()

[Jie Kang]

Hello,

jvanek noticed a newly failing unit test, ResourceTrackerTest. after the changeset:

changeset:   1135:ed2781d76012
user:        Jiri Vanek <jvanek at redhat.com>
date:        Thu Dec 18 20:05:41 2014 +0100
summary:     Added support for generating shortcuts also for applets.


After some digging I found out the cause:

1. Changes to PluginBridge now uses JNLPFile.openURL(...). This function has a side effect of attempting to download the given URL.

2. Previous tests in PluginBridgeTest use fake URLs for JNLP files that are needed to initialize the PluginBridge object.

3. This leads to those tests calling JNLPFile.openURL(..) and attempting to download the fake urls. This has 2 side-effects that cause ResourceTrackerTest.downloadResource() to fail.

 Side-effect 1:
     In ResourceTracker.initializeResource(...):
           JNLPRuntime.detectOnline(...) is called on the fake URL. This obviously can't be connected to and so JNLPRuntime.isOnline() now returns false.
           
           When ResourceTrackerTest.downloadResource() is run, the call for JNLPRuntime.isOnline() inside ResourceTracker.initializeResource(...) returns false and this causes nothing to be downloaded, so the test fails
          
           This is addressed in the patch: Check online detection even if checked before [1]. 

  Side-effect 2:
      In CacheUtil and CacheLRUWrapper, both classes have a separate static, first-use initialized variable cacheDir. Since PluginBridge now uses JNLPFile.openURL(), which calls ResourceTracker functions, which now call CacheUtil and CacheLRUWrapper functions, these classes get initialized to their default value, the user's cache directory. Now when ResourceTrackerTest.downloadResource(...) is run, the cachedir is already initialized and calls to JNLPRuntime.getConfiguration.setProperty("cache-dir-key") fail to change the value used by CacheUtil and CacheLRUWrapper. The test expects the cache-dir to actually be changed to the temp directory, and so once again, the test fails. As well, there are now entries in the user's cache directory from the tests in PluginBridgeTest.

      This is addressed in the two patches: Allow cache directory to be changed during runtime [2] and Use temporary cache in PluginBridge tests [3].


[1] http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-January/030290.html
[2] http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-January/030291.html
[3] http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2015-January/030292.html

I think there are various solutions to these problems, especially for Side-effect 1, and any thoughts/opinions are appreciated.  Patch [1] does fix the problem but there could be a better solution. I do believe that patch [2] and [3] are good though.

Hopefully this sheds some light on why the 3 patches above were sent for review.



Regards,

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern