[Freeipa-devel] [PATCHES] 0094-0099 Make Fuzzy objects available for the whole ipatests module

[Tomas Babej]

On 09/13/2013 07:53 PM, Petr Viktorin wrote:
> On 09/13/2013 07:04 PM, Rob Crittenden wrote:
>> Petr Viktorin wrote:
>>> On 09/13/2013 04:12 PM, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> The following patches move and extend the functionality of Fuzzy
>>>> objects. This is necessary to use them from within integration tests.
>>>
>>> -1 to the idea.
>>> I'm not a fan of the Fuzzy objects; they basically exist so that you 
>>> can
>>> use regex matching in the Declarative tests.
>>> As you've probably noticed Fuzzy is quite specific to the framework and
>>> its test suite -- see the strict bytes/unicode separation and the 
>>> amount
>>> of changes it takes to tear them out.
>>>
>>> I'm also not a fan of having a generic "Match anything using some 
>>> rules"
>>> object where everybody adds behavior specific to their use case. Each
>>> addition increases the complexity and number of corner cases, and the
>>> complete intended functionality can never be achieved.
>>>
>>> Using regular expressions directly should actually be easier and less
>>> error-prone in most cases.
>>> If you disagree, I'd like to see your use case.
>>
>> I'm not sure what the objection is, Fuzzy is just an abstraction. It
>> lets one do in-line regex which is the reason it was introduced
>> (initially for uid and gid because they are non-deterministic).
>
> Yes. "In-line" is the key here. It lets you do regex matching via the 
> == operator, which is what Declarative tests need, but elsewhere the 
> abstraction is completely unnecessary.
>
>> Replacing it would either a) replicate its functionality almost
>> completely or b) spread duplicate regex code all over the place.
>
> I'd go for b; spreading this code:
>     import re
>     some_regex = re.compile('some.*regex$')
>     ...
>     assert some_regex.search(x)
> instead of:
>     from wherever import Fuzzy
>     warm_fuzzy = Fuzzy('some.*regex$')
>     ...
>     assert x == warm_fuzzy
> all over the place is fine in my book. And you can even, say, add 
> custom flags to the regex without complicating shared code.
>
> The rest of Fuzzy functionality consists of strict type checking 
> (which isn't really necessary in integration tests), and the ability 
> to call arbitrary callables (which is just the scope creep I was 
> talking about).
> By the way, in current tests these features are hardly ever used in 
> combination.
> Even if this extra functionality is necessary, it's orthogonal to 
> regex matching and can be more cleanly done with several separate 
> asserts. Unless you need a single declarative object to compare 
> against, of course.
>
>> That isn't to say that Fuzzy isn't being abused, but that is also the
>> responsibility of the reviewers to be strict about.
>
> Then perhaps I'm too strict, but I say that using it outside of the 
> declarative tests is abuse.
> Especially if it takes six patches with hundreds of changed lines to 
> repurpose Fuzzy for integration tests (but that's not part of "-1 to 
> the idea").
>

While I have no strong opinions on using Fuzzy or regex directly in the 
integration tests, I wouldn't deprecate the idea of moving the Fuzzy 
object and its instances to a separate module.

We can make this part of test_xmlrpc module, and that would drive the 
message that Fuzzy should be used only within declarative tests home 
much more than the current state, when we have the functionality 
implemented at several levels (instances in 
ipatests.test_xmlrpc.xmlrpc_test, class in ipatests.util).

Patches also add some neat functionality, as enabling stacking regex 
Fuzzy objects together, or removing the counter-intuitive restriction 
for unicode type with regex (replaced by the explicit usage of the 'u' 
property, which is better than the implicit requirement).

Attached patches move the Fuzzy functionality to the 
ipatests.test_xmlrpc.fuzzy module.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0094-2-ipatests-Make-basestring-default-type-for-Fuzzy-when.patch
Type: text/x-patch
Size: 1215 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0095-2-ipatests-Add-line_unbound_regex-property-to-Fuzzy-ob.patch
Type: text/x-patch
Size: 968 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0096-2-ipatests-Move-generic-fuzzy-expressions-to-ipatests-.patch
Type: text/x-patch
Size: 201952 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0097-2-ipatests-Add-u-property-to-the-Fuzzy-object.patch
Type: text/x-patch
Size: 1745 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0098-2-ipatests-Add-unit-tests-for-u-and-line_unbound_regex.patch
Type: text/x-patch
Size: 3572 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0099-2-ipatests-Restrict-fuzzy-objects-in-unit-tests-to-uni.patch
Type: text/x-patch
Size: 94578 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130916/e63826a6/attachment-0005.bin>