[rf][icedtea-web] fix xdgTests again

[Jie Kang]

----- Original Message -----
> Hi!
> 
> So the xdgtest startet to faila again.
> 
> Mainly because of change in ignorance of attributes from true/false to enum.
> To this the change in XDGspecificationTests is related (rest is small java7
> liek refactoring).
> 
> However,it appeared that this is not enough, that setupable
> InfrastrucureDescriptors did it small
> evil hit there.
> They caused, that DeploymentConfiguration singleton was created suddnely in
> the middle of
> files/directories move. That means, that it was created *before*
> deployment.properties were moved,
> co it actually caused lost of this data. The fix was obvious, move moving of
> deployment.properties
> to the begginig. So it moves, and then singleton can be created any time...
> later.
> 
> Thats  v1.patch.
> 
> Then I realized, that what if legacy deployment.properties contains some
> custom values, they are
> actually lost anyway. So I started to write v2.patch monstrosity
> It is based on (good idea) to load DeploymentConfiguration legacy  deployment
> configuration as first
> step, and then use that getFullPath(differentConfig) mechanism. All went fine
> and worked, except
> fact, that the custom files actually dont need to be handled like that!
> Because once they are custom-set  they do not need to be migrated.
> DeploymentConfiguration (loaded
> from copied deployment.configuration) will find them correctly.
> 
> So I reverted most the changes in  DeploymentConfiguration, but kept few
> nioce refactorings which
> were result of v2. So v3 is v1 + few refactorngs which I conisdered as ok.
> 
> So v3 is the one which I wont to push.
> 
> 
> But.... During this email, I started to feel like v2 approach really cold be
> the best one. But
> considering that no one should be running on itw1.4 now, I would like to
> touch this code as few as
> possible. Everybody should be already on XDG and I relly do not won to spam
> users by the warning
> that both legacy and new config exists together.
> 
> 
> Thanx for bringing me out of this maze!

Hello Jiri,


Thanks for the work here!

When comparing v2 and v3, I prefer v3 since it's less invasive. However v2 was easier to understand for me. For you, what are the benefits for v2 over v3?


Below is patch review nits for v3:

+    /** source of true location */

Can this comment be expanded? I can't tell what you mean by 'true' location. 

+    private final InfrastructureFileDescriptor userPropertiesFileDescriptor;

I think the name could be changed to not be confused with PropertiesFile objects.

s/userPropertiesFileDescriptor/userDeploymentFileDescriptor ?

+     public DeploymentConfiguration(InfrastructureFileDescriptor usedConfigFile) {

Here as well, the argument name 'usedConfigFile' is a bit weird. s/usedConfigFile/configFile ?

+            //move this first, the cration of config singleton may happen anytime...(but hsould not in this block)

s/cration/creation
s/hsould/should

One question I have is; for the creation of config singleton, can't we force it to happen when we want by making a deliberate call beforehand? That way it won't be 'anytime' but, before this block is executed. Or did you want the config singleton to be created afterwards? Can you explain this part a little more for me? Sorry;;


diff -r dd4f7ccae35e tests/reproducers/simple/simpletest1/testcases/XDGspecificationTests.java
--- a/tests/reproducers/simple/simpletest1/testcases/XDGspecificationTests.java	Mon Apr 20 14:13:08 2015 -0400
+++ b/tests/reproducers/simple/simpletest1/testcases/XDGspecificationTests.java	Tue Apr 21 17:21:58 2015 +0200
@@ -90,7 +90,7 @@

For this set of tests, when I run them they fail because Permissions attribute isn't set, which launches a dialog causing applet not to launch. Shouldn't there be a before/after class to set the 'deployment.manifest.attributes.check' attribute to 'NONE' using the new DeploymentPropertiesModifier class?


Regards,


>    J.
> 
> 
> 

-- 

Jie Kang

OpenJDK Team - Software Engineering Intern