I already did that!

[Erlis Vidal-2]

Hi guys,

 

I'm having an issue and I would like to know if some of you have some
advice, comments, or whatever that can help me. Because I feel that I'm
doing something wrong.

 

Here is my problem:

 

I'm creating a function, and during that process, a bunch of test are
created as well. Then while creating another functionality I realize
that I need some refactoring in order to keep the class DRY. And I
extract some of the methods to a new class that will  contains the
common logic. Then I move some tests also. But here is my problem. Some
of the tests seem to be ok for both classes. On the other hand I feel
I'm testing the same thing twice.

 

Example:

 

Before the refactoring:

~~~~~~~~~~~~~~~

Function FindRunningReleases() {

...

var result = new List<Release>();

foreach ( var eachRelease in FindActiveReleases() ) {

              var pendingSteps =
releaseStepService.FindPendingStepByRelease( eachRelease );

              if ( pendingSteps.Count == 0 ) continue;

             

              bool isPaused = PrivateLocalFunctionWithTheLogicHere(
eachRelease, pendingSteps.First() );

             

              if ( ! isPaused ) {

                    result.Add( eachRelease );

              }

}

...

}

 

 

After the refactoring

~~~~~~~~~~~~~~~

Function FindRunningReleases() {

...

       var result = new List<Release>();

foreach ( var eachRelease in FindActiveReleases() ) {

              var pendingSteps =
releaseStepService.FindPendingStepByRelease( eachRelease );

              if ( pendingSteps.Count == 0 ) continue;

             

              if ( !
releaseFailedService.IsReleasePausedForStep(eachRelease,
pendingSteps.First() ) ) {

                    result.Add( eachRelease );

              }

}

...

}

 

My Original tests for FindRunningReleases where basically testing that
when the release was not in pause the method return that release
(testing the logic for: When is a release in pause?).

 

After the refactoring, for the new method I have to test the same logic,
When is a release in pause?.

 

I feel that the latest tests are more "in place" that the original ones.
But I don't want to remove the tests for FindRunningReleases. At the
end, must be something to test in this method!

 

I was thinking to use Mock to validate that the FindRunningReleases was
relaying in the releaseFailedService.IsReleasePausedForStep but that
look awkward. That is why write you guys.  

 

What is your advice? How do you do this?

 

Thanks in advance.

Erlis



***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


[Non-text portions of this message have been removed]



------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/testdrivendevelopment/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/testdrivendevelopment/join
    (Yahoo! ID required)

<*> To change settings via email:
    mailto:testdrivendevelopment-digest@...
    mailto:testdrivendevelopment-fullfeatured@...

<*> To unsubscribe from this group, send an email to:
    testdrivendevelopment-unsubscribe@...

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/

[Olof Bjarnason]

2009/11/3 Erlis Vidal <Erlis.Vidal@...>

> Hi guys,
>
>
>
> I'm having an issue and I would like to know if some of you have some
> advice, comments, or whatever that can help me. Because I feel that I'm
> doing something wrong.
>
>
>
> Here is my problem:
>
>
>
> I'm creating a function, and during that process, a bunch of test are
> created as well. Then while creating another functionality I realize
> that I need some refactoring in order to keep the class DRY. And I
> extract some of the methods to a new class that will  contains the
> common logic. Then I move some tests also. But here is my problem. Some
> of the tests seem to be ok for both classes. On the other hand I feel
> I'm testing the same thing twice.
>
>
>
> Example:
>
>
>
> Before the refactoring:
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
>              var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
>              if ( pendingSteps.Count == 0 ) continue;
>
>
>
>              bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> eachRelease, pendingSteps.First() );
>
>
>
>              if ( ! isPaused ) {
>
>                    result.Add( eachRelease );
>
>              }
>
> }
>
> ...
>
> }
>
>
>
>
>
> After the refactoring
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
>       var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
>              var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
>              if ( pendingSteps.Count == 0 ) continue;
>
>
>
>              if ( !
> releaseFailedService.IsReleasePausedForStep(eachRelease,
> pendingSteps.First() ) ) {
>
>                    result.Add( eachRelease );
>
>              }
>
> }
>
> ...
>
> }
>
>
>
> My Original tests for FindRunningReleases where basically testing that
> when the release was not in pause the method return that release
> (testing the logic for: When is a release in pause?).
>
>
>
> After the refactoring, for the new method I have to test the same logic,
> When is a release in pause?.
>
>
>
> I feel that the latest tests are more "in place" that the original ones.
> But I don't want to remove the tests for FindRunningReleases. At the
> end, must be something to test in this method!
>

The single responsibility principle states that each unit should have a
single purpose/responsibility.

If you compare your two units, could you describe, in words, what the
different responsibilities are?

Think of the tests as "responsibility descriptors", and let that guide you
which tests to remove, move, rename or whatever. the DRY principle is
applicable to tests too.

>
>
>
> I was thinking to use Mock to validate that the FindRunningReleases was
> relaying in the releaseFailedService.IsReleasePausedForStep but that
> look awkward. That is why write you guys.
>
>
>
> What is your advice? How do you do this?
>
>
>
> Thanks in advance.
>
> Erlis
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>
>
>
> ------------------------------------
>
> Yahoo! Groups Links
>
>
>
>

--
twitter.com/olofb
olofb.wordpress.com
olofb.wordpress.com/tag/english


[Non-text portions of this message have been removed]

[Erlis Vidal-2]

Hi Olof,

 

Thanks for your response. I like your point of view, and I'm really trying to follow that idea, that's why I'm confused.

 

I created a  method, with the unit tests. While refactoring I create a collaborator object and when trying to identify the edges cases seems to be a lot of collision here. So my question is: Should I test that edge cases again? Where? Just once? In both? I feel that using the same edges cases for a class and their collaborator violates DRY (maybe I'm wrong) and I'm wondering if there is a better way to do this? In the past I had resolved some similar problems mocking, but here, in this specific example I don't see the point of mocking.

 

Maybe the nature of the problem which is simple (in this moment) is the root of my confusion. "A loop, and adding to a list by testing a condition". The tests of this method and the tests of "Testing a condition" seems to be using the same edges cases. If in the future the method can became complex or even change the implementation (changing collaborator) the tests in place will be very helpful.

 

The responsibility here are different (and this makes me think that using same edge cases for both unit tests is ok), one add to a list, and the other test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that fine?  

 

As you can see, I need guidance, that's why I write you guys.

 

Thanks for your help.

Erlis

 

2009/11/3 Erlis Vidal <Erlis.Vidal@... <mailto:Erlis.Vidal%40radialpoint.com> >

> Hi guys,
>
>
>
> I'm having an issue and I would like to know if some of you have some
> advice, comments, or whatever that can help me. Because I feel that I'm
> doing something wrong.
>
>
>
> Here is my problem:
>
>
>
> I'm creating a function, and during that process, a bunch of test are
> created as well. Then while creating another functionality I realize
> that I need some refactoring in order to keep the class DRY. And I
> extract some of the methods to a new class that will contains the
> common logic. Then I move some tests also. But here is my problem. Some
> of the tests seem to be ok for both classes. On the other hand I feel
> I'm testing the same thing twice.
>
>
>
> Example:
>
>
>
> Before the refactoring:
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> eachRelease, pendingSteps.First() );
>
>
>
> if ( ! isPaused ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
>
>
> After the refactoring
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> if ( !
> releaseFailedService.IsReleasePausedForStep(eachRelease,
> pendingSteps.First() ) ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
> My Original tests for FindRunningReleases where basically testing that
> when the release was not in pause the method return that release
> (testing the logic for: When is a release in pause?).
>
>
>
> After the refactoring, for the new method I have to test the same logic,
> When is a release in pause?.
>
>
>
> I feel that the latest tests are more "in place" that the original ones.
> But I don't want to remove the tests for FindRunningReleases. At the
> end, must be something to test in this method!
>

The single responsibility principle states that each unit should have a
single purpose/responsibility.

If you compare your two units, could you describe, in words, what the
different responsibilities are?

Think of the tests as "responsibility descriptors", and let that guide you
which tests to remove, move, rename or whatever. the DRY principle is
applicable to tests too.

>
>
>
> I was thinking to use Mock to validate that the FindRunningReleases was
> relaying in the releaseFailedService.IsReleasePausedForStep but that
> look awkward. That is why write you guys.
>
>
>
> What is your advice? How do you do this?
>
>
>
> Thanks in advance.
>
> Erlis
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>
>
>
> ------------------------------------
>
> Yahoo! Groups Links
>
>
>
>

--
twitter.com/olofb
olofb.wordpress.com
olofb.wordpress.com/tag/english

[Non-text portions of this message have been removed]



 
***********************************************************************
 
This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
 
Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

 



***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


[Non-text portions of this message have been removed]

[gutzofter]

@Erlis,

I use a rule of thumb when using extract class: Extract behavior not state.

What  it looks like is you are extracted state to a new class.

So now, not only do you have tests for your original object, but you need tests that verify you get the correct state(s) from your new object.

What your trying to do is iterate over a collection and run some logic on the items in the collection, then if the item passes the logic test, add to item to a new collection. I would recommend pushing all the logic into the tester object and let it create the new collection. See example.

Function FindRunningReleases() {

...

var result = new List<Release>();

foreach ( var eachRelease in FindActiveReleases() ) {

runningReleases.Add(eachRelease, releaseStepService, releaseFailedService);

}

result = runningReleases.active();
}
...
}


// Running realease service class
...


Function Add(release, stepService, failedService) {

var active = new List<Release>();

var pendingSteps = stepService.FindPendingStepByRelease( release );

if ( pendingSteps.Count == 0 ) continue;



if ( !failedService.IsReleasePausedForStep(release, pendingSteps.First() ) ) {

active.Add( release);

}

}


Final note:

State extract method.
Behavior extract class.



--- In testdrivendevelopment@..., "Erlis Vidal" <Erlis.Vidal@...> wrote:

>
> Hi Olof,
>
>  
>
> Thanks for your response. I like your point of view, and I'm really trying to follow that idea, that's why I'm confused.
>
>  
>
> I created a  method, with the unit tests. While refactoring I create a collaborator object and when trying to identify the edges cases seems to be a lot of collision here. So my question is: Should I test that edge cases again? Where? Just once? In both? I feel that using the same edges cases for a class and their collaborator violates DRY (maybe I'm wrong) and I'm wondering if there is a better way to do this? In the past I had resolved some similar problems mocking, but here, in this specific example I don't see the point of mocking.
>
>  
>
> Maybe the nature of the problem which is simple (in this moment) is the root of my confusion. "A loop, and adding to a list by testing a condition". The tests of this method and the tests of "Testing a condition" seems to be using the same edges cases. If in the future the method can became complex or even change the implementation (changing collaborator) the tests in place will be very helpful.
>
>  
>
> The responsibility here are different (and this makes me think that using same edge cases for both unit tests is ok), one add to a list, and the other test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that fine?  
>
>  
>
> As you can see, I need guidance, that's why I write you guys.
>
>  
>
> Thanks for your help.
>
> Erlis
>
>  
>
> 2009/11/3 Erlis Vidal <Erlis.Vidal@... <mailto:Erlis.Vidal%40radialpoint.com> >
>
> > Hi guys,
> >
> >
> >
> > I'm having an issue and I would like to know if some of you have some
> > advice, comments, or whatever that can help me. Because I feel that I'm
> > doing something wrong.
> >
> >
> >
> > Here is my problem:
> >
> >
> >
> > I'm creating a function, and during that process, a bunch of test are
> > created as well. Then while creating another functionality I realize
> > that I need some refactoring in order to keep the class DRY. And I
> > extract some of the methods to a new class that will contains the
> > common logic. Then I move some tests also. But here is my problem. Some
> > of the tests seem to be ok for both classes. On the other hand I feel
> > I'm testing the same thing twice.
> >
> >
> >
> > Example:
> >
> >
> >
> > Before the refactoring:
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> > eachRelease, pendingSteps.First() );
> >
> >
> >
> > if ( ! isPaused ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> >
> >
> > After the refactoring
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > if ( !
> > releaseFailedService.IsReleasePausedForStep(eachRelease,
> > pendingSteps.First() ) ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> > My Original tests for FindRunningReleases where basically testing that
> > when the release was not in pause the method return that release
> > (testing the logic for: When is a release in pause?).
> >
> >
> >
> > After the refactoring, for the new method I have to test the same logic,
> > When is a release in pause?.
> >
> >
> >
> > I feel that the latest tests are more "in place" that the original ones.
> > But I don't want to remove the tests for FindRunningReleases. At the
> > end, must be something to test in this method!
> >
>
> The single responsibility principle states that each unit should have a
> single purpose/responsibility.
>
> If you compare your two units, could you describe, in words, what the
> different responsibilities are?
>
> Think of the tests as "responsibility descriptors", and let that guide you
> which tests to remove, move, rename or whatever. the DRY principle is
> applicable to tests too.
>
> >
> >
> >
> > I was thinking to use Mock to validate that the FindRunningReleases was
> > relaying in the releaseFailedService.IsReleasePausedForStep but that
> > look awkward. That is why write you guys.
> >
> >
> >
> > What is your advice? How do you do this?
> >
> >
> >
> > Thanks in advance.
> >
> > Erlis
> >
> >
> >
> > ***********************************************************************
> >
> > This e-mail and its attachments are confidential, legally privileged, may
> > be subject to copyright and sent solely for the attention of the
> > addressee(s).
> > Any unauthorized use or disclosure is prohibited. Statements and opinions
> > expressed in this e-mail may not represent those of Radialpoint.
> >
> > Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> > à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> > ses
> > destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> > autorisation. Les opinions exprimées dans le présent courriel peuvent
> > diverger de celles de Radialpoint.
> >
> >
> > [Non-text portions of this message have been removed]
> >
> >
> >
> > ------------------------------------
> >
> > Yahoo! Groups Links
> >
> >
> >
> >
>
> --
> twitter.com/olofb
> olofb.wordpress.com
> olofb.wordpress.com/tag/english
>
> [Non-text portions of this message have been removed]
>
>
>
>  
> ***********************************************************************
>  
> This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
>  
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.
>
>  
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>

[Donaldson, John (GEO)]

Erlis,

I'm a bit confused.
Here's the basic position: duplicated code is bad - remove it.
It seems as though you have something duplicated, so I'm wondering why you don't want to tidy it up.

Perhaps your case is like this:
- some tests which test something - let's call it X
- now you refactor and the functionality of X moves to another class
- you add some new tests which are testing X in its new place
- still your initial tests keep working, but the new tests are duplicating some of the old ones

Is this the situation?

If yes, then I would want to remove from the old X tests anything which is now being done in the new X tests.
But I don't think it's a serious problem.

Excuse me if I misunderstood the problem.

John D.

-----Original Message-----
From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Erlis Vidal
Sent: 04 November 2009 15:09
To: testdrivendevelopment@...
Subject: RE: [TDD] I already did that!

Hi Olof,

 

Thanks for your response. I like your point of view, and I'm really trying to follow that idea, that's why I'm confused.

 

I created a  method, with the unit tests. While refactoring I create a collaborator object and when trying to identify the edges cases seems to be a lot of collision here. So my question is: Should I test that edge cases again? Where? Just once? In both? I feel that using the same edges cases for a class and their collaborator violates DRY (maybe I'm wrong) and I'm wondering if there is a better way to do this? In the past I had resolved some similar problems mocking, but here, in this specific example I don't see the point of mocking.

 

Maybe the nature of the problem which is simple (in this moment) is the root of my confusion. "A loop, and adding to a list by testing a condition". The tests of this method and the tests of "Testing a condition" seems to be using the same edges cases. If in the future the method can became complex or even change the implementation (changing collaborator) the tests in place will be very helpful.

 

The responsibility here are different (and this makes me think that using same edge cases for both unit tests is ok), one add to a list, and the other test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that fine?  

 

As you can see, I need guidance, that's why I write you guys.

 

Thanks for your help.

Erlis

 

2009/11/3 Erlis Vidal <Erlis.Vidal@... <mailto:Erlis.Vidal%40radialpoint.com> >

> Hi guys,
>
>
>
> I'm having an issue and I would like to know if some of you have some
> advice, comments, or whatever that can help me. Because I feel that I'm
> doing something wrong.
>
>
>
> Here is my problem:
>
>
>
> I'm creating a function, and during that process, a bunch of test are
> created as well. Then while creating another functionality I realize
> that I need some refactoring in order to keep the class DRY. And I
> extract some of the methods to a new class that will contains the
> common logic. Then I move some tests also. But here is my problem. Some
> of the tests seem to be ok for both classes. On the other hand I feel
> I'm testing the same thing twice.
>
>
>
> Example:
>
>
>
> Before the refactoring:
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> eachRelease, pendingSteps.First() );
>
>
>
> if ( ! isPaused ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
>
>
> After the refactoring
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> if ( !
> releaseFailedService.IsReleasePausedForStep(eachRelease,
> pendingSteps.First() ) ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
> My Original tests for FindRunningReleases where basically testing that
> when the release was not in pause the method return that release
> (testing the logic for: When is a release in pause?).
>
>
>
> After the refactoring, for the new method I have to test the same logic,
> When is a release in pause?.
>
>
>
> I feel that the latest tests are more "in place" that the original ones.
> But I don't want to remove the tests for FindRunningReleases. At the
> end, must be something to test in this method!
>

The single responsibility principle states that each unit should have a
single purpose/responsibility.

If you compare your two units, could you describe, in words, what the
different responsibilities are?

Think of the tests as "responsibility descriptors", and let that guide you
which tests to remove, move, rename or whatever. the DRY principle is
applicable to tests too.

>
>
>
> I was thinking to use Mock to validate that the FindRunningReleases was
> relaying in the releaseFailedService.IsReleasePausedForStep but that
> look awkward. That is why write you guys.
>
>
>
> What is your advice? How do you do this?
>
>
>
> Thanks in advance.
>
> Erlis
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>
>
>
> ------------------------------------
>
> Yahoo! Groups Links
>
>
>
>

--
twitter.com/olofb
olofb.wordpress.com
olofb.wordpress.com/tag/english

[Non-text portions of this message have been removed]



 
***********************************************************************
 
This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
 
Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

 



***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


[Non-text portions of this message have been removed]



------------------------------------

Yahoo! Groups Links

[Erlis Vidal-2]

Hi John,

 

You are right when referred to remove the old X tests, that is what I usually do. But in this case what seems to happen is that all X appear to be moved to X' (lets use some notation) That drive me to no X, at this point I see something is not good.

 

I don't know if I described the example clearly. I'll try once again

 

I have a method that return only the Releases that aren't in pause. That gives me the set X of unit test that basically test this:

   FindRunningReleases_should_return_releases_NOT_in_pause (keep in mind that this is not a single test, here I have like 5 tests in order to test edge cases when releases are not in pause)

 

Then I move the logic out from the original class and I create a collaborator object which will encapsulate the InPause() logic, then the tests should prove:

  IsReleasePaused()  

 

Here I see that the same edge cases to test when a release is in paused or not should be used, the same past 5 tests.

 

In few words: How can I tests that the returning elements from FindRunningReleases are returning releases not in pause without testing that condition again in this class. A release in pause logic is tested in the collaborator class.

 

When I refer to mock this I was thinking on validate that the IsReleasePaused() was called during FindRunningReleases, so the returning elements must be find because I'm relying in the implementation of IsReleasePause() so I don't need to test the edges cases again. This makes sense ?

 

I hope you got the point now.

Thanks,

Erlis

 

 

From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Donaldson, John (GEO)
Sent: November-04-09 1:41 PM
To: testdrivendevelopment@...
Subject: RE: [TDD] I already did that!

 

 

Erlis,

I'm a bit confused.
Here's the basic position: duplicated code is bad - remove it.
It seems as though you have something duplicated, so I'm wondering why you don't want to tidy it up.

Perhaps your case is like this:
- some tests which test something - let's call it X
- now you refactor and the functionality of X moves to another class
- you add some new tests which are testing X in its new place
- still your initial tests keep working, but the new tests are duplicating some of the old ones

Is this the situation?

If yes, then I would want to remove from the old X tests anything which is now being done in the new X tests.
But I don't think it's a serious problem.

Excuse me if I misunderstood the problem.

John D.

-----Original Message-----
From: testdrivendevelopment@... <mailto:testdrivendevelopment%40yahoogroups.com>  [mailto:testdrivendevelopment@... <mailto:testdrivendevelopment%40yahoogroups.com> ] On Behalf Of Erlis Vidal
Sent: 04 November 2009 15:09
To: testdrivendevelopment@... <mailto:testdrivendevelopment%40yahoogroups.com>
Subject: RE: [TDD] I already did that!

Hi Olof,

Thanks for your response. I like your point of view, and I'm really trying to follow that idea, that's why I'm confused.

I created a method, with the unit tests. While refactoring I create a collaborator object and when trying to identify the edges cases seems to be a lot of collision here. So my question is: Should I test that edge cases again? Where? Just once? In both? I feel that using the same edges cases for a class and their collaborator violates DRY (maybe I'm wrong) and I'm wondering if there is a better way to do this? In the past I had resolved some similar problems mocking, but here, in this specific example I don't see the point of mocking.

Maybe the nature of the problem which is simple (in this moment) is the root of my confusion. "A loop, and adding to a list by testing a condition". The tests of this method and the tests of "Testing a condition" seems to be using the same edges cases. If in the future the method can became complex or even change the implementation (changing collaborator) the tests in place will be very helpful.

The responsibility here are different (and this makes me think that using same edge cases for both unit tests is ok), one add to a list, and the other test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that fine?

As you can see, I need guidance, that's why I write you guys.

Thanks for your help.

Erlis

2009/11/3 Erlis Vidal <Erlis.Vidal@... <mailto:Erlis.Vidal%40radialpoint.com>  <mailto:Erlis.Vidal%40radialpoint.com> >

> Hi guys,
>
>
>
> I'm having an issue and I would like to know if some of you have some
> advice, comments, or whatever that can help me. Because I feel that I'm
> doing something wrong.
>
>
>
> Here is my problem:
>
>
>
> I'm creating a function, and during that process, a bunch of test are
> created as well. Then while creating another functionality I realize
> that I need some refactoring in order to keep the class DRY. And I
> extract some of the methods to a new class that will contains the
> common logic. Then I move some tests also. But here is my problem. Some
> of the tests seem to be ok for both classes. On the other hand I feel
> I'm testing the same thing twice.
>
>
>
> Example:
>
>
>
> Before the refactoring:
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> eachRelease, pendingSteps.First() );
>
>
>
> if ( ! isPaused ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
>
>
> After the refactoring
>
> ~~~~~~~~~~~~~~~
>
> Function FindRunningReleases() {
>
> ...
>
> var result = new List<Release>();
>
> foreach ( var eachRelease in FindActiveReleases() ) {
>
> var pendingSteps =
> releaseStepService.FindPendingStepByRelease( eachRelease );
>
> if ( pendingSteps.Count == 0 ) continue;
>
>
>
> if ( !
> releaseFailedService.IsReleasePausedForStep(eachRelease,
> pendingSteps.First() ) ) {
>
> result.Add( eachRelease );
>
> }
>
> }
>
> ...
>
> }
>
>
>
> My Original tests for FindRunningReleases where basically testing that
> when the release was not in pause the method return that release
> (testing the logic for: When is a release in pause?).
>
>
>
> After the refactoring, for the new method I have to test the same logic,
> When is a release in pause?.
>
>
>
> I feel that the latest tests are more "in place" that the original ones.
> But I don't want to remove the tests for FindRunningReleases. At the
> end, must be something to test in this method!
>

The single responsibility principle states that each unit should have a
single purpose/responsibility.

If you compare your two units, could you describe, in words, what the
different responsibilities are?

Think of the tests as "responsibility descriptors", and let that guide you
which tests to remove, move, rename or whatever. the DRY principle is
applicable to tests too.

>
>
>
> I was thinking to use Mock to validate that the FindRunningReleases was
> relaying in the releaseFailedService.IsReleasePausedForStep but that
> look awkward. That is why write you guys.
>
>
>
> What is your advice? How do you do this?
>
>
>
> Thanks in advance.
>
> Erlis
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>
>
>
> ------------------------------------
>
> Yahoo! Groups Links
>
>
>
>

--
twitter.com/olofb
olofb.wordpress.com
olofb.wordpress.com/tag/english

[Non-text portions of this message have been removed]

***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

[Non-text portions of this message have been removed]

------------------------------------

Yahoo! Groups Links



 
***********************************************************************
 
This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
 
Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

 



***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


[Non-text portions of this message have been removed]

[Erlis Vidal-2]

Hi gutzofter,

 

Thanks for your answer. I have to admit that I didn't understand well what is the difference between behaviour vs state. Can you send me some references? Look interesting.

 

 

I don't see how the example you provide me helps with the tests needed to prove that the elements returned from the method actual meet the logic that was tested. The logic is not what you put on the Add() method, is what is inside the if condition:

         if ( !failedService.IsReleasePausedForStep(release, pendingSteps.First() ) )

 

You still have to test the FindRunningReleases. How can I tests that the returning elements are the expected without providing the entire set of edge conditions again?

 

Sincerely

Erlis



 

 

From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of gutzofter
Sent: November-04-09 12:02 PM
To: testdrivendevelopment@...
Subject: Re: [TDD] I already did that!

 

 

@Erlis,

I use a rule of thumb when using extract class: Extract behavior not state.

What it looks like is you are extracted state to a new class.

So now, not only do you have tests for your original object, but you need tests that verify you get the correct state(s) from your new object.

What your trying to do is iterate over a collection and run some logic on the items in the collection, then if the item passes the logic test, add to item to a new collection. I would recommend pushing all the logic into the tester object and let it create the new collection. See example.

Function FindRunningReleases() {

...

var result = new List<Release>();

foreach ( var eachRelease in FindActiveReleases() ) {

runningReleases.Add(eachRelease, releaseStepService, releaseFailedService);

}

result = runningReleases.active();
}
...
}

// Running realease service class
...

Function Add(release, stepService, failedService) {

var active = new List<Release>();

var pendingSteps = stepService.FindPendingStepByRelease( release );

if ( pendingSteps.Count == 0 ) continue;

if ( !failedService.IsReleasePausedForStep(release, pendingSteps.First() ) ) {

active.Add( release);

}

}

Final note:

State extract method.
Behavior extract class.

--- In testdrivendevelopment@... <mailto:testdrivendevelopment%40yahoogroups.com> , "Erlis Vidal" <Erlis.Vidal@...> wrote:

>
> Hi Olof,
>
>
>
> Thanks for your response. I like your point of view, and I'm really trying to follow that idea, that's why I'm confused.
>
>
>
> I created a method, with the unit tests. While refactoring I create a collaborator object and when trying to identify the edges cases seems to be a lot of collision here. So my question is: Should I test that edge cases again? Where? Just once? In both? I feel that using the same edges cases for a class and their collaborator violates DRY (maybe I'm wrong) and I'm wondering if there is a better way to do this? In the past I had resolved some similar problems mocking, but here, in this specific example I don't see the point of mocking.
>
>
>
> Maybe the nature of the problem which is simple (in this moment) is the root of my confusion. "A loop, and adding to a list by testing a condition". The tests of this method and the tests of "Testing a condition" seems to be using the same edges cases. If in the future the method can became complex or even change the implementation (changing collaborator) the tests in place will be very helpful.
>
>
>
> The responsibility here are different (and this makes me think that using same edge cases for both unit tests is ok), one add to a list, and the other test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that fine?
>
>
>
> As you can see, I need guidance, that's why I write you guys.
>
>
>
> Thanks for your help.
>
> Erlis
>
>
>
> 2009/11/3 Erlis Vidal <Erlis.Vidal@... <mailto:Erlis.Vidal%40radialpoint.com> >
>
> > Hi guys,
> >
> >
> >
> > I'm having an issue and I would like to know if some of you have some
> > advice, comments, or whatever that can help me. Because I feel that I'm
> > doing something wrong.
> >
> >
> >
> > Here is my problem:
> >
> >
> >
> > I'm creating a function, and during that process, a bunch of test are
> > created as well. Then while creating another functionality I realize
> > that I need some refactoring in order to keep the class DRY. And I
> > extract some of the methods to a new class that will contains the
> > common logic. Then I move some tests also. But here is my problem. Some
> > of the tests seem to be ok for both classes. On the other hand I feel
> > I'm testing the same thing twice.
> >
> >
> >
> > Example:
> >
> >
> >
> > Before the refactoring:
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> > eachRelease, pendingSteps.First() );
> >
> >
> >
> > if ( ! isPaused ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> >
> >
> > After the refactoring
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > if ( !
> > releaseFailedService.IsReleasePausedForStep(eachRelease,
> > pendingSteps.First() ) ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> > My Original tests for FindRunningReleases where basically testing that
> > when the release was not in pause the method return that release
> > (testing the logic for: When is a release in pause?).
> >
> >
> >
> > After the refactoring, for the new method I have to test the same logic,
> > When is a release in pause?.
> >
> >
> >
> > I feel that the latest tests are more "in place" that the original ones.
> > But I don't want to remove the tests for FindRunningReleases. At the
> > end, must be something to test in this method!
> >
>
> The single responsibility principle states that each unit should have a
> single purpose/responsibility.
>
> If you compare your two units, could you describe, in words, what the
> different responsibilities are?
>
> Think of the tests as "responsibility descriptors", and let that guide you
> which tests to remove, move, rename or whatever. the DRY principle is
> applicable to tests too.
>
> >
> >
> >
> > I was thinking to use Mock to validate that the FindRunningReleases was
> > relaying in the releaseFailedService.IsReleasePausedForStep but that
> > look awkward. That is why write you guys.
> >
> >
> >
> > What is your advice? How do you do this?
> >
> >
> >
> > Thanks in advance.
> >
> > Erlis
> >
> >
> >
> > ***********************************************************************
> >
> > This e-mail and its attachments are confidential, legally privileged, may
> > be subject to copyright and sent solely for the attention of the
> > addressee(s).
> > Any unauthorized use or disclosure is prohibited. Statements and opinions
> > expressed in this e-mail may not represent those of Radialpoint.
> >
> > Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> > à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> > ses
> > destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> > autorisation. Les opinions exprimées dans le présent courriel peuvent
> > diverger de celles de Radialpoint.
> >
> >
> > [Non-text portions of this message have been removed]
> >
> >
> >
> > ------------------------------------
> >
> > Yahoo! Groups Links
> >
> >
> >
> >
>
> --
> twitter.com/olofb
> olofb.wordpress.com
> olofb.wordpress.com/tag/english
>
> [Non-text portions of this message have been removed]
>
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.
>
>
>
>
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.
>
>
> [Non-text portions of this message have been removed]
>

 
***********************************************************************
 
This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.
 
Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.

 



***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut être soumis à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


[Non-text portions of this message have been removed]

[Donaldson, John (GEO)]

Erlis,

You have separated the InPause logic, and you can test that separately. This seems to be the right direction. I don't see that you need to test again the InPause logic. In the tests of the Release finder (FindRunningReleases etc) you should just be focusing on whether it does indeed return the releases you want. Not that it knows how to distinguish paused releases from non-paused ones.

So, somehow these tests might be clearer if you could set them up like this:
- Create a paused release, add it to the set of releases
- Create a non-paused release, add it to the set
- Assert: FindRunningReleases that are not paused should return one release.

In this way you separate the detecting of "in pause" and the returning of the set from the set of releases.

It might still be true that you call out and detect "in pause" at the moment you need it. Or you might set some Boolean attribute "Paused", or there might be sub-types of Release - PausedRelease, UnpausedRelease. I don't know your app well enough.

But like that you don't need to test all the "in pause" detection twice.

Maybe you need to consider injecting a mock version of the InPause collaborator - then you would have no temptation of testing its logic! :-)

John D.

-----Original Message-----
From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Erlis Vidal
Sent: 04 November 2009 21:10
To: testdrivendevelopment@...
Subject: RE: [TDD] I already did that!

Hi John,

 

You are right when referred to remove the old X tests, that is what I usually do. But in this case what seems to happen is that all X appear to be moved to X' (lets use some notation) That drive me to no X, at this point I see something is not good.

 

I don't know if I described the example clearly. I'll try once again

 

I have a method that return only the Releases that aren't in pause. That gives me the set X of unit test that basically test this:

   FindRunningReleases_should_return_releases_NOT_in_pause (keep in mind that this is not a single test, here I have like 5 tests in order to test edge cases when releases are not in pause)

 

Then I move the logic out from the original class and I create a collaborator object which will encapsulate the InPause() logic, then the tests should prove:

  IsReleasePaused()  

 

Here I see that the same edge cases to test when a release is in paused or not should be used, the same past 5 tests.

 

In few words: How can I tests that the returning elements from FindRunningReleases are returning releases not in pause without testing that condition again in this class. A release in pause logic is tested in the collaborator class.

 

When I refer to mock this I was thinking on validate that the IsReleasePaused() was called during FindRunningReleases, so the returning elements must be find because I'm relying in the implementation of IsReleasePause() so I don't need to test the edges cases again. This makes sense ?

 

I hope you got the point now.

Thanks,

Erlis

 

 

[franz see]

If I understood your problem correctly, initially, you have tests for
FindRunningReleases that are something like these

FindRunningReleases_should_<logic#1-related-to-pause>
> FindRunningReleases_should_<logic#2-related-to-pause>
> FindRunningReleases_should_<logic#3-related-to-pause>
> ...
> FindRunningReleases_should_<logic#n-related-to-pause>
>

And after the refactoring, you now have these

IsReleasePausedForStep_should_<logic#1-related-to-pause>
> IsReleasePausedForStep_should_<logic#2-related-to-pause>
> IsReleasePausedForStep_should_<logic#3-related-to-pause>
> ...
> IsReleasePausedForStep_should_<logic#n-related-to-pause>
>

But you still kept

FindRunningReleases_should_<logic#1-related-to-pause>
> FindRunningReleases_should_<logic#2-related-to-pause>
> FindRunningReleases_should_<logic#3-related-to-pause>
> ...
> FindRunningReleases_should_<logic#n-related-to-pause>
>

Now, your problem is you want to remove those FindRunningReleases_should_XXX
because they're duplicates of IsReleasePausedForStep_should_XXX, but
removing those would be removing all tests that you have for
FindRunningReleases.

If I understood it correctly, here's my reply:

Before refactoring, FindRunningReleases is all about
a.) the pending-steps-count logic,
b.) handling the pause-logic, and
c.) the collection of these running releases.

But after you moved the pause-logic to IsReleasePausedForStep, your
FindRunningReleases is now just concerned about (a.) & (c.).

So I think the problem is not that FindRunningReleases will be left without
tests. Instead, the problem is FindRunningReleases is not testing (a.) and
(c.).

(..and btw, you can still refactor (a.) out of FindRunningReleases so that
it's only concerned with (c.)).

Cheers,

--
Franz Allan Valencia See | Java Software Engineer
franz.see@...
LinkedIn: http://www.linkedin.com/in/franzsee
Twitter: http://www.twitter.com/franz_see

On Thu, Nov 5, 2009 at 4:09 AM, Erlis Vidal <Erlis.Vidal@...>wrote:

>
>
> Hi John,
>
> You are right when referred to remove the old X tests, that is what I
> usually do. But in this case what seems to happen is that all X appear to be
> moved to X' (lets use some notation) That drive me to no X, at this point I
> see something is not good.
>
> I don't know if I described the example clearly. I'll try once again
>
> I have a method that return only the Releases that aren't in pause. That
> gives me the set X of unit test that basically test this:
>
> FindRunningReleases_should_return_releases_NOT_in_pause (keep in mind that
> this is not a single test, here I have like 5 tests in order to test edge
> cases when releases are not in pause)
>
> Then I move the logic out from the original class and I create a
> collaborator object which will encapsulate the InPause() logic, then the
> tests should prove:
>
> IsReleasePaused()
>
> Here I see that the same edge cases to test when a release is in paused or
> not should be used, the same past 5 tests.
>
> In few words: How can I tests that the returning elements from
> FindRunningReleases are returning releases not in pause without testing that
> condition again in this class. A release in pause logic is tested in the
> collaborator class.
>
> When I refer to mock this I was thinking on validate that the
> IsReleasePaused() was called during FindRunningReleases, so the returning
> elements must be find because I'm relying in the implementation of
> IsReleasePause() so I don't need to test the edges cases again. This makes
> sense ?
>
> I hope you got the point now.
>
> Thanks,
>
> Erlis
>
> From: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>[mailto:
> testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>]
> On Behalf Of Donaldson, John (GEO)
> Sent: November-04-09 1:41 PM
>
> To: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>
> Subject: RE: [TDD] I already did that!
>
> Erlis,
>
> I'm a bit confused.
> Here's the basic position: duplicated code is bad - remove it.
> It seems as though you have something duplicated, so I'm wondering why you
> don't want to tidy it up.
>
> Perhaps your case is like this:
> - some tests which test something - let's call it X
> - now you refactor and the functionality of X moves to another class
> - you add some new tests which are testing X in its new place
> - still your initial tests keep working, but the new tests are duplicating
> some of the old ones
>
> Is this the situation?
>
> If yes, then I would want to remove from the old X tests anything which is
> now being done in the new X tests.
> But I don't think it's a serious problem.
>
> Excuse me if I misunderstood the problem.
>
> John D.
>
> -----Original Message-----
> From: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
> [mailto:testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
> ] On Behalf Of Erlis Vidal
> Sent: 04 November 2009 15:09
> To: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
>
> Subject: RE: [TDD] I already did that!
>
> Hi Olof,
>
> Thanks for your response. I like your point of view, and I'm really trying
> to follow that idea, that's why I'm confused.
>
> I created a method, with the unit tests. While refactoring I create a
> collaborator object and when trying to identify the edges cases seems to be
> a lot of collision here. So my question is: Should I test that edge cases
> again? Where? Just once? In both? I feel that using the same edges cases for
> a class and their collaborator violates DRY (maybe I'm wrong) and I'm
> wondering if there is a better way to do this? In the past I had resolved
> some similar problems mocking, but here, in this specific example I don't
> see the point of mocking.
>
> Maybe the nature of the problem which is simple (in this moment) is the
> root of my confusion. "A loop, and adding to a list by testing a condition".
> The tests of this method and the tests of "Testing a condition" seems to be
> using the same edges cases. If in the future the method can became complex
> or even change the implementation (changing collaborator) the tests in place
> will be very helpful.
>
> The responsibility here are different (and this makes me think that using
> same edge cases for both unit tests is ok), one add to a list, and the other
> test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that
> fine?
>
> As you can see, I need guidance, that's why I write you guys.
>
> Thanks for your help.
>
> Erlis
>
> 2009/11/3 Erlis Vidal <Erlis.Vidal@...<Erlis.Vidal%40radialpoint.com><mailto:
> Erlis.Vidal%40radialpoint.com <Erlis.Vidal%2540radialpoint.com>> <mailto:
> Erlis.Vidal%40radialpoint.com <Erlis.Vidal%2540radialpoint.com>> >
>
>
> > Hi guys,
> >
> >
> >
> > I'm having an issue and I would like to know if some of you have some
> > advice, comments, or whatever that can help me. Because I feel that I'm
> > doing something wrong.
> >
> >
> >
> > Here is my problem:
> >
> >
> >
> > I'm creating a function, and during that process, a bunch of test are
> > created as well. Then while creating another functionality I realize
> > that I need some refactoring in order to keep the class DRY. And I
> > extract some of the methods to a new class that will contains the
> > common logic. Then I move some tests also. But here is my problem. Some
> > of the tests seem to be ok for both classes. On the other hand I feel
> > I'm testing the same thing twice.
> >
> >
> >
> > Example:
> >
> >
> >
> > Before the refactoring:
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> > eachRelease, pendingSteps.First() );
> >
> >
> >
> > if ( ! isPaused ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> >
> >
> > After the refactoring
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > if ( !
> > releaseFailedService.IsReleasePausedForStep(eachRelease,
> > pendingSteps.First() ) ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> > My Original tests for FindRunningReleases where basically testing that
> > when the release was not in pause the method return that release
> > (testing the logic for: When is a release in pause?).
> >
> >
> >
> > After the refactoring, for the new method I have to test the same logic,
> > When is a release in pause?.
> >
> >
> >
> > I feel that the latest tests are more "in place" that the original ones.
> > But I don't want to remove the tests for FindRunningReleases. At the
> > end, must be something to test in this method!
> >
>
> The single responsibility principle states that each unit should have a
> single purpose/responsibility.
>
> If you compare your two units, could you describe, in words, what the
> different responsibilities are?
>
> Think of the tests as "responsibility descriptors", and let that guide you
> which tests to remove, move, rename or whatever. the DRY principle is
> applicable to tests too.
>
> >
> >
> >
> > I was thinking to use Mock to validate that the FindRunningReleases was
> > relaying in the releaseFailedService.IsReleasePausedForStep but that
> > look awkward. That is why write you guys.
> >
> >
> >
> > What is your advice? How do you do this?
> >
> >
> >
> > Thanks in advance.
> >
> > Erlis
> >
> >
> >
> > ***********************************************************************
> >
> > This e-mail and its attachments are confidential, legally privileged, may
> > be subject to copyright and sent solely for the attention of the
> > addressee(s).
> > Any unauthorized use or disclosure is prohibited. Statements and opinions
> > expressed in this e-mail may not represent those of Radialpoint.
> >
> > Le contenu de ce courriel est confidentiel, privilégié et peut être
> soumis
> > à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> > ses
> > destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> > autorisation. Les opinions exprimées dans le présent courriel peuvent
> > diverger de celles de Radialpoint.
> >
> >
> > [Non-text portions of this message have been removed]
> >
> >
> >
> > ------------------------------------
> >
> > Yahoo! Groups Links
> >
> >
> >
> >
>
> --
> twitter.com/olofb
> olofb.wordpress.com
> olofb.wordpress.com/tag/english
>
> [Non-text portions of this message have been removed]
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> [Non-text portions of this message have been removed]
>
> ------------------------------------
>
> Yahoo! Groups Links
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut être soumis
> à des droits d'auteur. Il est envoyé à l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> [Non-text portions of this message have been removed]
>
>  
>

[Non-text portions of this message have been removed]



------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/testdrivendevelopment/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/testdrivendevelopment/join
    (Yahoo! ID required)

<*> To change settings via email:
    testdrivendevelopment-digest@...
    testdrivendevelopment-fullfeatured@...

<*> To unsubscribe from this group, send an email to:
    testdrivendevelopment-unsubscribe@...

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/

[gutzofter]

--- In testdrivendevelopment@..., "Erlis Vidal" <Erlis.Vidal@...> wrote:
>
> Hi gutzofter,
>
>  
>
> Thanks for your answer. I have to admit that I didn't understand well what is the difference between behaviour vs state. Can you send me some references? Look interesting.
>

It's really simple state is just data. When I say 'behavior', I mean the algorithm for transforming data.

>  
>
>  
>
> I don't see how the example you provide me helps with the tests needed to prove that the elements returned from the method actual meet the logic that was tested. The logic is not what you put on the Add() method, is what is inside the if condition:
>
>          if ( !failedService.IsReleasePausedForStep(release, pendingSteps.First() ) )
>
>  
>
> You still have to test the FindRunningReleases. How can I tests that the returning elements are the expected without providing the entire set of edge conditions again?
>
>

FindRunningRealeases is now just an iterator. You only need to test that you iterate over your releases and that you return a result in a new list. It really doesn't matter if the list returned is null or some other list. The real key to this is:

runningReleases.Add(eachRelease, releaseStepService, releaseFailedService);

The real tests now are pushed into running release service method 'add'. Your tests only need to pass a release (running and paused), two tests.


PS: you need to be careful not to prematurely refactor code if there is no duplication and if there is no external contexts pushing you to create these collaborators.

PSS: The test should be refactored to this:

if (( pendingSteps.Count == 0 )&& (!failedService.IsReleasePausedForStep(release, pendingSteps.First() )) {
...
}

Hope this helps

>
> Sincerely
>
> Erlis
>
>
>
>  
>
>  
>

[Erlis Vidal-2]

Hi guys,

Thanks for all your responses. Putting together John, gutzofter and Franz ideas I could do a better solution.

When I was reading all your responses, every time you said "create a pause/non-pause release" I was thinking, here is the problem, is not a Boolean, a release is paused/non-pause for 5 different reasons, so I still have to create 5 different unit tests. I was wrong. This is what I did:

- All the unit tests related with the pause logic was moved from the original tests to the collaborator tests.  
- The refactored class only contains one test that is focused in the returned elements.
    * In this test I have two lists one for paused and another for non-paused releases and use them combined as the input list for the method under test.
      (all previous _should_when_XXX are now elements of this list combining non-paused and paused releases)
    * then the test validate that the returned elements are non-paused releases. Now I have my precious name FindRunningReleases_should_return_NON_paused_releases

In this way this test is not focused in HOW but in WHAT is detected. And if in the future the FindRunningReleases use another filter than the paused, I can create a different test FindRunningReleases_should_return_ANOTHER_LOGIC_releases.

I still have an small problem with this solution:
if a new "case" appear for a paused/non-paused release, I have changes in two different places: the IsPaused logic tests, and update the lists for this unit test...

Even when I can smell something here I can say that I "like" more this smell than the previous one.  

BTW. Franz good observation about the pending refactoring, I missed that! That's the magic of pair programming, thanks!

Once again, thanks a lot for all your help.
Erlis


-----Original Message-----
From: testdrivendevelopment@... [mailto:testdrivendevelopment@...] On Behalf Of Franz Allan Valencia See
Sent: November-05-09 3:02 AM
To: testdrivendevelopment@...
Subject: Re: [TDD] I already did that!

If I understood your problem correctly, initially, you have tests for
FindRunningReleases that are something like these

FindRunningReleases_should_<logic#1-related-to-pause>
> FindRunningReleases_should_<logic#2-related-to-pause>
> FindRunningReleases_should_<logic#3-related-to-pause>
> ...
> FindRunningReleases_should_<logic#n-related-to-pause>
>

And after the refactoring, you now have these

IsReleasePausedForStep_should_<logic#1-related-to-pause>
> IsReleasePausedForStep_should_<logic#2-related-to-pause>
> IsReleasePausedForStep_should_<logic#3-related-to-pause>
> ...
> IsReleasePausedForStep_should_<logic#n-related-to-pause>
>

But you still kept

FindRunningReleases_should_<logic#1-related-to-pause>
> FindRunningReleases_should_<logic#2-related-to-pause>
> FindRunningReleases_should_<logic#3-related-to-pause>
> ...
> FindRunningReleases_should_<logic#n-related-to-pause>
>

Now, your problem is you want to remove those FindRunningReleases_should_XXX
because they're duplicates of IsReleasePausedForStep_should_XXX, but
removing those would be removing all tests that you have for
FindRunningReleases.

If I understood it correctly, here's my reply:

Before refactoring, FindRunningReleases is all about
a.) the pending-steps-count logic,
b.) handling the pause-logic, and
c.) the collection of these running releases.

But after you moved the pause-logic to IsReleasePausedForStep, your
FindRunningReleases is now just concerned about (a.) & (c.).

So I think the problem is not that FindRunningReleases will be left without
tests. Instead, the problem is FindRunningReleases is not testing (a.) and
(c.).

(..and btw, you can still refactor (a.) out of FindRunningReleases so that
it's only concerned with (c.)).

Cheers,

--
Franz Allan Valencia See | Java Software Engineer
franz.see@...
LinkedIn: http://www.linkedin.com/in/franzsee
Twitter: http://www.twitter.com/franz_see

On Thu, Nov 5, 2009 at 4:09 AM, Erlis Vidal <Erlis.Vidal@...>wrote:

>
>
> Hi John,
>
> You are right when referred to remove the old X tests, that is what I
> usually do. But in this case what seems to happen is that all X appear to be
> moved to X' (lets use some notation) That drive me to no X, at this point I
> see something is not good.
>
> I don't know if I described the example clearly. I'll try once again
>
> I have a method that return only the Releases that aren't in pause. That
> gives me the set X of unit test that basically test this:
>
> FindRunningReleases_should_return_releases_NOT_in_pause (keep in mind that
> this is not a single test, here I have like 5 tests in order to test edge
> cases when releases are not in pause)
>
> Then I move the logic out from the original class and I create a
> collaborator object which will encapsulate the InPause() logic, then the
> tests should prove:
>
> IsReleasePaused()
>
> Here I see that the same edge cases to test when a release is in paused or
> not should be used, the same past 5 tests.
>
> In few words: How can I tests that the returning elements from
> FindRunningReleases are returning releases not in pause without testing that
> condition again in this class. A release in pause logic is tested in the
> collaborator class.
>
> When I refer to mock this I was thinking on validate that the
> IsReleasePaused() was called during FindRunningReleases, so the returning
> elements must be find because I'm relying in the implementation of
> IsReleasePause() so I don't need to test the edges cases again. This makes
> sense ?
>
> I hope you got the point now.
>
> Thanks,
>
> Erlis
>
> From: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>[mailto:
> testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>]
> On Behalf Of Donaldson, John (GEO)
> Sent: November-04-09 1:41 PM
>
> To: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com>
> Subject: RE: [TDD] I already did that!
>
> Erlis,
>
> I'm a bit confused.
> Here's the basic position: duplicated code is bad - remove it.
> It seems as though you have something duplicated, so I'm wondering why you
> don't want to tidy it up.
>
> Perhaps your case is like this:
> - some tests which test something - let's call it X
> - now you refactor and the functionality of X moves to another class
> - you add some new tests which are testing X in its new place
> - still your initial tests keep working, but the new tests are duplicating
> some of the old ones
>
> Is this the situation?
>
> If yes, then I would want to remove from the old X tests anything which is
> now being done in the new X tests.
> But I don't think it's a serious problem.
>
> Excuse me if I misunderstood the problem.
>
> John D.
>
> -----Original Message-----
> From: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
> [mailto:testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
> ] On Behalf Of Erlis Vidal
> Sent: 04 November 2009 15:09
> To: testdrivendevelopment@...<testdrivendevelopment%40yahoogroups.com><mailto:
> testdrivendevelopment%40yahoogroups.com<testdrivendevelopment%2540yahoogroups.com>>
>
> Subject: RE: [TDD] I already did that!
>
> Hi Olof,
>
> Thanks for your response. I like your point of view, and I'm really trying
> to follow that idea, that's why I'm confused.
>
> I created a method, with the unit tests. While refactoring I create a
> collaborator object and when trying to identify the edges cases seems to be
> a lot of collision here. So my question is: Should I test that edge cases
> again? Where? Just once? In both? I feel that using the same edges cases for
> a class and their collaborator violates DRY (maybe I'm wrong) and I'm
> wondering if there is a better way to do this? In the past I had resolved
> some similar problems mocking, but here, in this specific example I don't
> see the point of mocking.
>
> Maybe the nature of the problem which is simple (in this moment) is the
> root of my confusion. "A loop, and adding to a list by testing a condition".
> The tests of this method and the tests of "Testing a condition" seems to be
> using the same edges cases. If in the future the method can became complex
> or even change the implementation (changing collaborator) the tests in place
> will be very helpful.
>
> The responsibility here are different (and this makes me think that using
> same edge cases for both unit tests is ok), one add to a list, and the other
> test a condition... Maybe I'm violating DRY (Am I?) but not SRP? Is that
> fine?
>
> As you can see, I need guidance, that's why I write you guys.
>
> Thanks for your help.
>
> Erlis
>
> 2009/11/3 Erlis Vidal <Erlis.Vidal@...<Erlis.Vidal%40radialpoint.com><mailto:
> Erlis.Vidal%40radialpoint.com <Erlis.Vidal%2540radialpoint.com>> <mailto:
> Erlis.Vidal%40radialpoint.com <Erlis.Vidal%2540radialpoint.com>> >
>
>
> > Hi guys,
> >
> >
> >
> > I'm having an issue and I would like to know if some of you have some
> > advice, comments, or whatever that can help me. Because I feel that I'm
> > doing something wrong.
> >
> >
> >
> > Here is my problem:
> >
> >
> >
> > I'm creating a function, and during that process, a bunch of test are
> > created as well. Then while creating another functionality I realize
> > that I need some refactoring in order to keep the class DRY. And I
> > extract some of the methods to a new class that will contains the
> > common logic. Then I move some tests also. But here is my problem. Some
> > of the tests seem to be ok for both classes. On the other hand I feel
> > I'm testing the same thing twice.
> >
> >
> >
> > Example:
> >
> >
> >
> > Before the refactoring:
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > bool isPaused = PrivateLocalFunctionWithTheLogicHere(
> > eachRelease, pendingSteps.First() );
> >
> >
> >
> > if ( ! isPaused ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> >
> >
> > After the refactoring
> >
> > ~~~~~~~~~~~~~~~
> >
> > Function FindRunningReleases() {
> >
> > ...
> >
> > var result = new List<Release>();
> >
> > foreach ( var eachRelease in FindActiveReleases() ) {
> >
> > var pendingSteps =
> > releaseStepService.FindPendingStepByRelease( eachRelease );
> >
> > if ( pendingSteps.Count == 0 ) continue;
> >
> >
> >
> > if ( !
> > releaseFailedService.IsReleasePausedForStep(eachRelease,
> > pendingSteps.First() ) ) {
> >
> > result.Add( eachRelease );
> >
> > }
> >
> > }
> >
> > ...
> >
> > }
> >
> >
> >
> > My Original tests for FindRunningReleases where basically testing that
> > when the release was not in pause the method return that release
> > (testing the logic for: When is a release in pause?).
> >
> >
> >
> > After the refactoring, for the new method I have to test the same logic,
> > When is a release in pause?.
> >
> >
> >
> > I feel that the latest tests are more "in place" that the original ones.
> > But I don't want to remove the tests for FindRunningReleases. At the
> > end, must be something to test in this method!
> >
>
> The single responsibility principle states that each unit should have a
> single purpose/responsibility.
>
> If you compare your two units, could you describe, in words, what the
> different responsibilities are?
>
> Think of the tests as "responsibility descriptors", and let that guide you
> which tests to remove, move, rename or whatever. the DRY principle is
> applicable to tests too.
>
> >
> >
> >
> > I was thinking to use Mock to validate that the FindRunningReleases was
> > relaying in the releaseFailedService.IsReleasePausedForStep but that
> > look awkward. That is why write you guys.
> >
> >
> >
> > What is your advice? How do you do this?
> >
> >
> >
> > Thanks in advance.
> >
> > Erlis
> >
> >
> >
> > ***********************************************************************
> >
> > This e-mail and its attachments are confidential, legally privileged, may
> > be subject to copyright and sent solely for the attention of the
> > addressee(s).
> > Any unauthorized use or disclosure is prohibited. Statements and opinions
> > expressed in this e-mail may not represent those of Radialpoint.
> >
> > Le contenu de ce courriel est confidentiel, privilégié et peut ętre
> soumis
> > ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de
> > ses
> > destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> > autorisation. Les opinions exprimées dans le présent courriel peuvent
> > diverger de celles de Radialpoint.
> >
> >
> > [Non-text portions of this message have been removed]
> >
> >
> >
> > ------------------------------------
> >
> > Yahoo! Groups Links
> >
> >
> >
> >
>
> --
> twitter.com/olofb
> olofb.wordpress.com
> olofb.wordpress.com/tag/english
>
> [Non-text portions of this message have been removed]
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis
> ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis
> ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> [Non-text portions of this message have been removed]
>
> ------------------------------------
>
> Yahoo! Groups Links
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis
> ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> ***********************************************************************
>
> This e-mail and its attachments are confidential, legally privileged, may
> be subject to copyright and sent solely for the attention of the
> addressee(s).
> Any unauthorized use or disclosure is prohibited. Statements and opinions
> expressed in this e-mail may not represent those of Radialpoint.
>
> Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis
> ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de
> ses
> destinataires. Il est interdit de l'utiliser ou de le divulguer sans
> autorisation. Les opinions exprimées dans le présent courriel peuvent
> diverger de celles de Radialpoint.
>
> [Non-text portions of this message have been removed]
>
>  
>

[Non-text portions of this message have been removed]



------------------------------------

Yahoo! Groups Links





***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


***********************************************************************

This e-mail and its attachments are confidential, legally privileged, may be subject to copyright and sent solely for the attention of the addressee(s).
Any unauthorized use or disclosure is prohibited. Statements and opinions expressed in this e-mail may not represent those of Radialpoint.

Le contenu de ce courriel est confidentiel, privilégié et peut ętre soumis ŕ des droits d'auteur. Il est envoyé ŕ l'intention exclusive de son ou de ses
destinataires. Il est interdit de l'utiliser ou de le divulguer sans autorisation. Les opinions exprimées dans le présent courriel peuvent diverger de celles de Radialpoint.


------------------------------------

Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/testdrivendevelopment/

<*> Your email settings:
    Individual Email | Traditional

<*> To change settings online go to:
    http://groups.yahoo.com/group/testdrivendevelopment/join
    (Yahoo! ID required)

<*> To change settings via email:
    testdrivendevelopment-digest@...
    testdrivendevelopment-fullfeatured@...

<*> To unsubscribe from this group, send an email to:
    testdrivendevelopment-unsubscribe@...

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/

[Sean Corfield]

2009/11/5 Erlis Vidal <Erlis.Vidal@...>:
> I still have an small problem with this solution:
> if a new "case" appear for a paused/non-paused release, I have changes in two different places: the IsPaused logic tests, and update the lists for this unit test...

If I'm understanding you correctly, no, you don't really have two
places. You have a method that filters on whether something is paused
or not - but it doesn't matter to that method WHY it is paused.
Therefore if you add a new case, you only need to adds tests around
the IsPaused() test. As long as that returns true/false correctly for
all cases, the FindRunning() method is unaffected.
--
Sean A Corfield -- (904) 302-SEAN
Railo Technologies US -- http://getrailo.com/
An Architect's View -- http://corfield.org/

"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood