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

[Lukasz Dracz]

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.

sure

> 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

I have attached a patch with the new class. I have also changed the tests to use this
new class in the patch.

I was not sure where to put the new class in test-extensions, so I put it in the jnlp.tools package,
where would be a good place to put it ?

Thank you,
Lukasz Dracz





-------------- next part --------------
A non-text attachment was scrubbed...
Name: deploymentPropertiesModifier-2.patch
Type: text/x-patch
Size: 12774 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20150402/cb839fb8/deploymentPropertiesModifier-2-0001.patch>