On 12/09/2015 11:29 AM, Lenka Doudova wrote:



On 12/09/2015 10:13 AM, Martin Basti wrote:


On 09.12.2015 09:41, Lenka Doudova wrote:
Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to
https://fedorahosted.org/freeipa/ticket/5387.

Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:
On 11/19/2015 10:34 AM, Petr Viktorin wrote:
On 11/19/2015 09:30 AM, Lenka Doudova wrote:
On 11/18/2015 04:51 PM, Martin Babinsky wrote:
On 11/18/2015 02:16 PM, Lenka Doudova wrote:
Hi,

here's a patch that adds a few comments to stageuser tests in
order to
allow easier determining of a problem when tests fail.

Lenka



Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names
anyway.

I am also not sure if this patch is worth reviewing and pushing
as it
IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.

Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode,
you get
output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212]

PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done,
though,
the only way to find out which command failed is looking at the
code and
finding which parameters were put into the command, which was not
much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test
cases, one
for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I
don't know
of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.

Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would
make
the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at
https://pytest.org/latest/fixture.html#parametrizing-a-fixture



This looks like a better approach IMHO, you can then see which
attribute/value was being checked.

I would very much favor more descriptive test/fixture names in the
beginning.




Hello,

we use usually bottom posting on freeipa-devel please try to keep
reply in this way.

Patch:

I do not like the idea of separated lists, IMO it is hard to manage
and is easy to create mistakes.

How about this (untested, just from top of my head):
http://fpaste.org/298994/65184014/

Martin

Great idea, thanks. Fixed patches attached.

Lenka



Tests pass and code looks good, ACK.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to