[SSSD] [PATCH v4] Add integration tests

Lukas Slebodnik lslebodn at redhat.com
Tue May 26 16:56:40 UTC 2015


On (26/05/15 13:03), Nikolai Kondrashov wrote:
>On 05/26/2015 09:25 AM, Lukas Slebodnik wrote:
>>It's better to do not mix utility functions.
>>So feel free to split file to subfiles if it will be used among many files
>>or include helper function directly to module. You can later extract helper
>>function to another utility module when there will be use case.
>>(run_shell is not used atm)
>>
>>And here are proposed names. string_utils | utils_string | utils.string,
>>shell_utils (python already has shutil) ...
>
>Hmm, you asked me to change the "misc.py" name, but now you're suggesting we
>should split it. Is it really worth it for three tiny functions? What if we
>just rename it to "util.py", or some other name which you find more suitable?
>
OK, it does not worth to block patches

>>It might be better idea to do nat catch excetion (or rethrow exception)
>>to see full stack trace with failure.
>
>Hmm, I'm doing neither I think, but perhaps I'm missing or misunderstanding
>something. Care to elaborate?
>
I had just an idea that sometimes it might be usefull to see failed test
due to exception. Maybe it can help to find a probelm.
We will what will be the best way in future.

>>>         AM_INTGCHECK_REQ([HAVE_PYTEST],         [pytest])
>>>         AM_INTGCHECK_REQ([HAVE_PY2MOD_LDAP],    [python-ldap])
>>                                               ^^^^
>>                 Such might look good but if there is a longer 1st argument
>>                 you will need to reindent it.
>
>Alright, I used your version.
>
>>                 BTW it causes many pep8 warnings in python code.
>
>You mean similar practice in Python code causes PEP8 warnings? In Python code
>in general, or in the Python code I'm currently submitting? Should we be
>checking our code for these? If yes, how do I do that?
>
sh$  pep8 src/tests/intg/ds.py
src/tests/intg/ds.py:22:1: E302 expected 2 blank lines, found 1
src/tests/intg/ds.py:37:17: E221 multiple spaces before operator
src/tests/intg/ds.py:38:18: E221 multiple spaces before operator
src/tests/intg/ds.py:39:22: E221 multiple spaces before operator
src/tests/intg/ds.py:40:21: E221 multiple spaces before operator
src/tests/intg/ds.py:41:23: E221 multiple spaces before operator
src/tests/intg/ds.py:42:22: E221 multiple spaces before operator
src/tests/intg/ds.py:43:22: E221 multiple spaces before operator

We needn't fix all warnings atm, we can discuss it with FreeIPA
what is the best practise. I know they suppress some warnings.

BTW.
Have you found a reason of failed test?

LS



More information about the sssd-devel mailing list