cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [GitHub] cloudstack pull request: Removed duplicate code in CitrixResourceB...
Date Wed, 19 Aug 2015 10:26:49 GMT
a unit can be defined at more then just the method level and in this case
those paths have changed from under us in the past. I am not justifying
testing any constant this way. I am justifying just this work.

On Wed, Aug 19, 2015 at 9:03 AM, Anshul Gangwar <anshul.gangwar@citrix.com>
wrote:

> What I mean to say is that unit test are meant to test individual unit
> which here is getPatchFilePath and not meant to test hierarchy as you are
> pointing out here. By individual unit I mean it doesn’t matter for test
> that it is in class A or class B. This way you are kind of justifying that
> we should write test for any constant which you have defined has the value
> which you have given. Because constant under different classes can have
> different values.
>
> Can you point to any reference which justifies writing tests for this kind
> of scenario?
>
>
> Regards,
> Anshul
>
> On 19-Aug-2015, at 11:34 am, Daan Hoogland <daan.hoogland@gmail.com>
> wrote:
>
>
> On Wed, Aug 19, 2015 at 5:56 AM, anshul1886 <git@git.apache.org> wrote:
>
>> If the purpose is to make sure that path is not modified by other
>> developer then adding note/comment on top of that line makes more sense.
>> Even adding note is kind of implicit as paths are kind of constants which
>> any developer would think before changing. Tests are not meant for that
>> purpose.
>>
>
> ​Anshul, I hope I don't understand you when you say, 'tests are not meant
> for that pupose'. When the hierarchy is changed and this leads to the
> constants to be used in a different way in different classes, an error
> occurs due to this that will be caught by these tests. This is exactly what
> unit tests are for.​
>
> class A has constant pth="/root".
> class B:A has constant pth="/root/bla".
> ​class C:B has no constant hence pth="/root/bla".
>
> now C is changed to C:A and its pth is there fore changed to "/root".​
> This is uninteded and a mistake that will be caught by such tests.
>
>
>
> --
> Daan
>
>
>


-- 
Daan

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message