[SSSD] [PATCH][ding_libs] INI: run also negative_test

Michal Židek mzidek at redhat.com
Thu Jan 16 15:29:51 UTC 2014


On 01/15/2014 08:59 PM, Dmitri Pal wrote:
> On 01/15/2014 10:30 AM, Lukas Slebodnik wrote:
>> On (13/01/14 10:41), Dmitri Pal wrote:
>>> On 01/13/2014 09:32 AM, Lukas Slebodnik wrote:
>>>> On (13/01/14 08:55), Dmitri Pal wrote:
>>>>> On 01/13/2014 05:46 AM, Lukas Slebodnik wrote:
>>>>>> ehlo,
>>>>>>
>>>>>> Test negative_test was defined, but it has never been executed as part of
>>>>>> ini_config test suite.
>>>>>>
>>>>>> Simple patch is attached.
>>>>>>
>>>>>> LS
>>>>>>
>>>>> This unit test tests old interfaces that is being phased out.
>>>>> I do not think we need to improve UNIT test for the API that is
>>>>> deprecated. IMO sends a wrong message so I would rather drop this.
>>>>>
>>>> Yes,
>>>> it is testing old interface, but old interface is still part of library.
>>>> Unit test passes without any problem. Do we really need to remove this unit
>>>> test? Personally, I don't have problem with it.
>>>>
>>>> LS
>>> We need to remove unit test when we remove the old interface.
>>> I do not think it is time yet to remove the interface.
>>> But fixing unit test also does not seem like time well spent.
>> Unit test is not broken. So, I cannot see any problem.
>> Can I consider reply as ACK?
>>
>> LS
> IMO it is busy work to enhance unit test for the interface being
> deprecated.
> So you can check it in but I have not tested or reviewed whether this
> test does something meaningful or not. It is just a waste of time and
> this is my point.
>

True, since it is deprecated interface, I do not see much value in 
enhancing the test suite for it as well. But on the other hand, these 
test are already there and are simply ignored, which just makes them 
dead code and that does not make much sense either. I would ACK the 
patch for now and later when this API is removed we will remove the 
tests for it as well.

So ACK from me.

Michal



More information about the sssd-devel mailing list