<html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 07/18/2013 01:35 PM, Petr Vobornik wrote: </div> <blockquote cite="mid:51E7D2EE.4050501@redhat.com" type="cite">On 07/18/2013 01:34 PM, Petr Vobornik wrote: <blockquote type="cite">[PATCH] 431 Web UI integration tests: Add trust tests [PATCH] 432 Web UI integration tests: Add ui_driver method descriptions [PATCH] 433 Web UI integration tests: Verify data after add and mod [PATCH] 434 Web UI integration tests: Compute range sizes to avoid overlaps Heavily inspired by code from xmlrpc tests. To obtain ranges, this patch also adds method to execute FreeIPA command through Web UI. It uses Web UI instead of ipalib so it doesn't need to care about authentication on a test-runner machine. All: <a class="moz-txt-link-freetext" href="https://fedorahosted.org/freeipa/ticket/3744">https://fedorahosted.org/freeipa/ticket/3744</a> </blockquote> And now the patches... <fieldset class="mimeAttachmentHeader"></fieldset> <pre wrap="">_______________________________________________ Freeipa-devel mailing list <a class="moz-txt-link-abbreviated" href="mailto:Freeipa-devel@redhat.com">Freeipa-devel@redhat.com</a> <a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/freeipa-devel">https://www.redhat.com/mailman/listinfo/freeipa-devel</a></pre> </blockquote> Patch 431: - In case 'ipa-adtrust-install' has not been run on the system, but 'has_trusts' is configured in 'ui_test.conf', trust tests fail. I suggest checking for 'ipa-adtrust-install' with 'adtrust_is_enabled' command and then skipping tests if trusts are not enabled. Patch 432: - Docstrings for 'has_ca()' and 'has_dns()' state that 'FreeIPA server was installed *without* CA/DNS' when they shoud state *with*. Patch 433: - Please also add docstrings for newly added methods. - The long 'if' statement in 'validate_fields()' can be shortened by using the 'in' keyword instead of individual string comparisons. For example: elif ftype in ('textbox', 'password', 'combobox'): actual = self.get_field_value(key, parent, 'input') - IIUC, the code in validate_fields() compares lists irrespective of element order. If this is the case, it can be replaced by simply 'sorted(expected) == sorted(actual)'. - In 'basic_crud()', shouldn't validate_fields with 'add_v' be called right after 'add_record'? The way it is now, data only gets validated in case of data modification, but not after initial addition. Patch 434: - The "if 'ipasecondarybaserid' in idrange" block should be nested under the "if 'ipasecondarybaserid' in idrange" block, since it assumes that the 'base_rid' variable is set. - Please remove trailing semicolons. General comments (these comments are by no means a requirement, and they are not a reason for a NACK, just a "nice to have"): - Some methods have unused parameters (e.g. validate_fields, basic_crud) - Some methods define variables which shadow Python built-ins of the same name (e.g. type, all). This can be a problem if you later want to use the built-in. - It would be nice to make at least newly added code PEP8 compliant. The 'pep8' utility can be used to easily check any python file for PEP8 compliance: $ sudo yum install python-pep8 $ pep8 /path/to/script.py <pre class="moz-signature" cols="80">-- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.</pre> </body> </html>