[rfc][icedtea-web] fix CodeBaseManifestEntrySignedMatching Tests by enabling Codebase check

[Jiri Vanek]

On 03/30/2015 07:53 PM, Lukasz Dracz wrote:
> Hello,
>
> I have been looking into why these tests fail:
>
> - ApplicationJNLPLocalTest
> - AppletJNLPRLocalTest
> - ApplicationJNLPRemoteTest
> - BrowserAppletLocalTest
> - BrowserJNLPHrefRemoteTest
> - BrowserJNLPHrefLocalTest
> - AppletJNLPRemoteTest
> - BrowserAppletRemoteTest
> - ApplicationJNLPLocalTestWithRemoteCodebase
>
> I have found that the changeset 984 where manifest attributes check property was introduced to be the problem. The tests pass regularly if the check is enabled to ALL or contains CODEBASE in the combination of attributes check.

Thanx for investigations! Why they need the attributes at all?

> But since when the reproducers are typically run the attributes check flag is set to NONE, I have made this patch that adds a BeforeClass and AfterClass that changes the manifest attributes check property to CODEBASE and back to its previous value after the tests. This ensures the tests pass no matter what the deployment.manifest.attributes.check is set to.
>
> Thank you,
> Lukasz Dracz
>


The code seem to be.. pretty similar to one changeset ahead .. hmm?-)

  Do you mind to encapsulate his logic to new class in textextensions? Those two testcases can then use this class.

In ideal world of APIs:

public class DeploymentPropetiesModifier {

public DeploymentPropetiesModifier(){
this (PathsAndFiles.USER_DEPLOYMENT_FILE)
}
DeploymentPropetiesModifier(InfrastructureDescriptor src){
	this.src=src;
}

public void setProperties(ReasonableInput i){
   if (setPropertiesCalled) throw new IllegalStateException
  ReasonableReturn originalState  setPropertiesToDeploymentConfig(i, src.file){
  saveState(originalState)
}


public void restoreProperties(){
   if (!setPropertiesCalled) throw new IllegalStateException
   setPropertiesToDeploymentConfig(savedFile, src.getFile){

}





public static ReasonableReturn setPropertiesToDeploymentConfig(ReasonableInput i, File config){
     the actual logic :)
}



public static ReasonableReturn setPropertiesToDeploymentConfig(ReasonableInput i){
    return setPropertiesToDeploymentConfig(i, PathsAndFiles.USER_DEPLOYMENT_FILE.getFile());
}
}

maybe you will need to have separate methods for set and restore.. but I hope not.


Also please try to avoid hardcoded string like deployment.manifest.attributes.check=CODEBASE. Use the variables rather both. Not in impl, but in calls to impl :)



Then the calls will be


in testextensions-tests -
DeploymentPropetiesModifierTest{

test  ReasonableReturn setPropertiesToDeploymentConfig(ReasonableInput i, File config)s
test  DeploymentPropetiesModifier(tmpSource)s

}


in calling class:

  @BeforeClass
    public static void setupDeploymentProperties() throws IOException {
deploymentPropetiesModifier = new DeploymentPropetiesModifier();
deploymentPropetiesModifier.setProperties({KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK,MANIFEST_ATTRIBUTES_CHECK.whatever}) [*]
}

[*]if that will occure to many times, feel free to intorduce shortcyt method in DeploymentPropetiesModifier
...
public void setMAnifestAttributesToWhatever(){
    setProperties({KEY_ENABLE_MANIFEST_ATTRIBUTES_CHECK,MANIFEST_ATTRIBUTES_CHECK.whatever)
}
...

    @AfterClass
     public static void setbackDeploymentProperties() throws IOException {
deploymentPropetiesModifier.restoreProperties()
}


Whole thos eis maybe overkill, but right thing to do.


IF you will follow my ideas,t ry to come with better names and not so complex ReasonableInput/Output

J.