[rfc][icedteaweb] Add comments about permission attributes not being checked in reproducers

[Lukasz Dracz]

Hello,

----- Original Message -----
> From: "Jiri Vanek" <jvanek at redhat.com>
> To: "Lukasz Dracz" <ldracz at redhat.com>, "IcedTea Distro List" <distro-pkg-dev at openjdk.java.net>
> Sent: Thursday, March 5, 2015 2:48:25 AM
> Subject: Re: [rfc][icedteaweb] Add comments about permission attributes not being checked in reproducers
> 
> On 03/04/2015 10:00 PM, Lukasz Dracz wrote:
> > Hello,
> >
> > I have been looking into why certain reproducers are failing. For the
> > following tests
> >
> > testPartiallySignedAppletWithSandboxPermissionsInManifestLaunchWithSignedHTMLApp
> > testPartiallySignedJNLPAppletWithSandboxPermissionsInManifestLaunchWithSignedApp
> > testSignedAppletWithSandboxPermissionsInManifestHtml
> > testSignedAppletWithSandboxPermissionsInManifestHtmlJnlpHref
> > testSignedAppletWithSandboxPermissionsInManifestJnlpApplet
> > testSignedAppletWithSandboxPermissionsInManifestJnlpApplication
> >
> > I found that changeset 1129:0284eb954ebc is reason.
> >
> > The reason why is that the permissions attributes check has been moved from
> > always being checked into an if statement
> > where they are only checked when deployment.manifest.attributes.check is
> > set to true. I have tested and found the tests to pass when this
> > deployment.manifest.attributes.check is set to true.
> >
> > This patch just adds comments to the tests for reference to help explain to
> > anyone in the future looking at the tests. At the moment I don't have a
> > solution, does anyone have an idea on how to properly fix this or handle
> > this ?
> > I had tried to set the property to true for the tests and revert it back
> > after to no avail. I will try to think more on this problem.
> >
> 
> Hi!
> 
> So yes - the testsuite is running with
> deployment.manifest.attributes.check=false.
> 
> IMHO best would be to tune it to run without any
> deployment.manifest.attributes.check.
> 
> 
> For your group of tests - I think the best way to go is :
> @beforeclass ensure that no deployment.manifest.attributes.check is present
> in
> deploymnet.properties. If it is, remove it.
> @afterclass return it back, if it was here.
> 
> 
> What do you think?  Note - tehre is plenty util methods which loads file to
> string or save string to
> file. Just do not add more :)

I have tested the @beforeclass @afterclass method of editing the deployment.properties file and well it works somewhat, some of the tests pass now
but the other ones will prompt a pop-up and if you click to proceed it passes but obviously failing to click proceed it fails.

This defeats the purpose of the manifest attributes check being set to false.
I can't seem to think of a solution I would like.
I suppose we could split the deployment.manifest.attributes.check into two properties, by adding a new one that decides whether to check the Permissions Attribute and the default deployment.manifest.attributes.check would check all the other attributes.

For the test we would edit the manifest like you mentioned with @beforeclass to change the deployment.manifest.permissions.attributes.check to true and keep deployment.manifest.attributes.check false.
What do you think about this solution ?
Do you have any other ideas ?

I suppose we could also try to make it possible to edit the property directly and having that change updated when done. I would need to look into how feasible this is.

Thoughts ? 

Thank you,
Lukasz Dracz




> Thank you!
> 
> J.
> 
> 
> 
> 
> 
> 
>