[rfc][icedtea-web][policyeditor] CustomPolicyEditor method extractions and tests added

[Andrew Azores]

On 06/06/2014 02:51 PM, Jie Kang wrote:
>
> ----- Original Message -----
>> On 06/06/2014 11:22 AM, Lukasz Dracz wrote:
>>> ----- Original Message -----
>>>> From: "Andrew Azores" <aazores at redhat.com>
>>>> To: "IcedTea" <distro-pkg-dev at openjdk.java.net>
>>>> Sent: Friday, June 6, 2014 10:49:38 AM
>>>> Subject: Re: [rfc][icedtea-web][policyeditor] CustomPolicyEditor method
>>>> 	extractions and tests added
>>>>
>>>> On 06/06/2014 10:47 AM, Andrew Azores wrote:
>>>>> Hi,
>>>>>
>>>>> In the same vein as the previous patch to PolicyEditor which extracted
>>>>> some methods to allow unit testing, this attached patch does the same
>>>>> for CustomPolicyViewer. Additionally, CustomPolicyViewer no longer
>>>>> holds a reference of its own to the PolicyFileModel, instead using
>>>>> appropriate methods exposed through its parent PolicyEditor.
>>>>> CustomPolicyViewer now also uses CustomPolicy.fromString() rather than
>>>>> its own "parsing" solution, and also no longer allows duplicate
>>>>> permissions to be added to the user interface (which was a bug since
>>>>> the actual collection modelling this is a Set anyway, so the JList and
>>>>> Set could become out of sync).
>>>>>
> Hello,
>
> Currently testAddCustomPermissionDuplicates calls testAddCustomPermission in order to add a base permission. Though it is convenient I don't think this is good practice. Everything else looks fine.
>
>
>>>>> Thanks,
>>>>>
>>>> Sorry, in the subject line that should be CustomPolicy*Viewer*, not
>>>> CustomPolicyEditor.
>>>>
>>>> Thanks,
>>>>
>>>> --
>>>> Andrew A

Yea, I wasn't too happy with it either. The adding of a permission on 
its own is not enough to extract into a method, it needs to be done 
along with the asserts to be anything useful. I don't want to have the 
asserts in a non- at Test method really, so what's left is either calling a 
test from within other tests, or just having duplicate code. The 
attached patch lets the duplicate code be rather than calling another 
@Test method from within tests.

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: custompolicyviewer-refactor-tests-3.patch
Type: text/x-patch
Size: 16484 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140606/da6758ae/custompolicyviewer-refactor-tests-3-0001.patch>