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 13:39:28 GMT
@anshul1886 I agree that we disagree.

Folks I do not know what to do in this case, I will not looking for
references to support what I said and did. Whatever you guys decided I will
do (remove or let the test cases there).

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

> Let me summarise the tests
> class A {
> x(){
> return “d”;  a constant
> }
> }
>
> test
>
> p=“d”
> Assert( A.x() = p)
>
> Which can be reduced to
>
> class A {
> q=“d";
> }
>
> Here q is replacement for x method as it is only returning a constant
>
> now test is
> p=“d"
> assert (p=q)
>
> To me this basically proves that java assignment works and nothing more
> than that.
>
> Here A can be any class. But in this context it is subclass of something.
>
> I can’t even figure out how super class coming into picture in tests which
> you are trying to say.
>
> Please explain in context of above example how it is testing more than
> java assignment.
>
> Regards,
> Anshul
>
> On 19-Aug-2015, at 5:16 pm, Anshul Gangwar <anshul.gangwar@citrix.com>
> wrote:
>
> Can you point to any reference/blog which justifies writing tests for this
> kind of scenario?
>
> What I can infer from these tests is that that there are two scenarios
> 1) Method will not change
>    In that case it doesn’t make sense to put test for never changing
> method.
> 2) Method will change
>    In that case you have to change test to make it pass and then also it
> doesn’t make sense as you have to change test to make it actually pass.
>
> Regards,
> Anshul
>
>
> On 19-Aug-2015, at 4:18 pm, Rafael Weingärtner <
> rafaelweingartner@gmail.com<mailto:rafaelweingartner@gmail.com
> <rafaelweingartner@gmail.com>>> wrote:
>
> @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<
> mailto:daan.hoogland@gmail.com <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
> <mailto:anshul.gangwar@citrix.com <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<
> mailto:daan.hoogland@gmail.com <daan.hoogland@gmail.com>>>
> wrote:
>
>
> On Wed, Aug 19, 2015 at 5:56 AM, anshul1886 <git@git.apache.org<
> mailto:git@git.apache.org <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
>
>
>


-- 
Rafael Weingärtner

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