cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rafael Weingärtner <rafaelweingart...@gmail.com>
Subject Re: [GitHub] cloudstack pull request: Removed duplicate code in CitrixResourceB...
Date Wed, 19 Aug 2015 10:48:54 GMT
@anshul1886 I totally agree with you that tests are meant to test
individual, and as you pointed the individual code that we want to test is
“getPatchFilePath”. However, that method is abstract, and its
“implementation” that is as simple as returning a constant, changes in few
subclasses of CitrixResourceBase. I am not testing the constant per se; I
am testing if each one of the implementation of that method is returning
what I expect.

 In one hand, I agree with you that the method could have documentation. I
just have not done that because I really do not know what that String that
the method is returning is. On the other hand, documentation will not save
us from future bugs, as a consequence of some change in those methods.
Those tests can do that automatically.

If that method had a conditional statement and it was coded into
CitrixResourceBase would you think different? The point here is that object
orientation removed those ifs.

On Wed, Aug 19, 2015 at 7:26 AM, Daan Hoogland <daan.hoogland@gmail.com>
wrote:

> 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
>



-- 
Rafael Weingärtner

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