[Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky