NYCPHP Meetup

[Gary A. Mort]

Thanks Robert...  I may be misunderstanding something here:

On 11/15/2013 12:57 PM, Robert Stoll wrote:
> I am not sure if we talk about the same. Just to avoid misunderstands I am going to outline a little bit more what I
> meant. I did not mean that each method of a class has to have its one test class. But each method of a class A should
> have an own test method in the test class T. And if the method of class A has branches, let's say one if-statement, then
> the ideal case would be that you create two test methods in C which covers both cases. Once for the case that the
> if-condition evaluates to true and once to false.
> For example:
>
> class A{
>      private $_isActive=false;
>      function isActive(){
>          return $this->_isActive;
>      }
>      function foo(){
>          $this->_isActive=true;
>      }
>
>      function bar(){
>          if($isActive){
>              doesThis();
>          } else{
>              doesThat();
>          }
>      }
> }
>
> class T extends SomeTestFramework{
>      public function testFoo_Standard_IsActiveSetToTrue (){
>          // arrange
>          // act
>          // assert
>      }
>      public function testBar_IsActiveIsTrue_DoesThis(){}

I am assuming that there is no unit testing magic which is setting 
things, so this method would actually be:

public function testBar_IsActiveIsTrue_DoesThis(){

// create an object $testObject of class A
// call $testObject->foo() to make it active
// Test that $testObject->isActive returns true
// Test that $testObject->bar executes doesThis()

}


public function testBar_IsActiveIsFalse_DoesThat(){

// create an object $testObject of class A
// Test that $testObject->isActive returns false
// Test that $testObject->bar executes doesThat()

}


It's with the above commented steps that I have an issue.  Primarily 
because in practice, if someone creates:

Class APrime extend A{}

Then they also create

class TPrime extends SomeTestFramework{

     function bar(){
         if($isActive){
             doesThis();
         } else{
             doesNOTDOThat();
         }

}

In in TPrime will be:


public function testBar_IsActiveIsTrue_DoesThis(){

// create an object $testObject of class APrime
// call $testObject->foo() to make it active
// Test that $testObject->isActive returns true
// Test that $testObject->bar executes doesThis()

}


public function testBar_IsActiveIsFalse_DoesNOTDOThat(){

// create an object $testObject of class APrime
// Test that $testObject->isActive returns false
// Test that $testObject->bar executes doesNOTDOThat()

}


So everything has been cut and pasted from one to the other.  The only 
difference is that APrime calls doesNOTDOThat instead of doesThat.  
Testing items where taken from one to the other, with minor editing 
changes to half of the new tests to change doesThat to doesNOTDOThat

Now to extend this further, let's say a year later someone goes in and 
modifies class A:

class A{
     private $_isActive=false;

      public function __construct($active = false) {
          $this->$_isActive = $active;
     }


Everything works pretty well for a while because no one is actually 
USING the new parameter added to object construction.  Months pass:


Then the class needs to have THREE states, and is active is easily 
modified for that:

  function bar(){
         if($isActive === true){
             doesThis();
         } else if ($isActive === false) {
             doesThat();
         } else {
	    doesSomethingEntirelyDifferent();	
	}
  }


The testing class is updated with the appropriate:

public function testBar_IsActiveThirdState_DoesSomethingEntirelyDeffierentThat(){

// create an object $testObject of class A using $testObject = new A(3);
// Test that $testObject->isActive returns 3
// Test that $testObject->bar executes doesSomethingEntirelyDifferent()

}




And here is where the maintenance fun begins!

Class APrime was forgotten about - so it wasn't updated to use the new 
tristate logic.  APrime still passes all the tests associated with it, 
so from the Unit Tests all looks good.

The very reason we wrote unit tests, to discover when changes break 
compatibility, and because we just copy and paste tests from one test 
class to the next, the tests don't tell us all that they should and could.


The more I think about it, the more I think that inheritance is not the 
answer because we just would have overwritten some of the tests anyway 
and would still end up failing.

Ideally, what we should be doing is:
When testing a subclass, after running the tests for the subclass, we 
should also load the tests for the parent class and run them. Somehow we 
need to add a blacklist of tests not to run, ie:
For APrime we want to run:
TPrime::testBar_IsActiveIsTrue_DoesThis, 
TPrime::testBar_IsActiveIsTrue_DoesNOTDOTHAT
and run
all tests from class T EXCEPT FOR TPrime::testBar_IsActiveIsTrue_DoesThat

 From a test report perspective, I want to know that:
Class APrime is sane
Class APrime is also a sane subclass of class A

If tests are run this way, then when the modifications I've seen happen 
such as the above are done, our unit tests will come back and say:
Class APrime is sane
Class APrime is not a sane subclass of class A

Because it will fail the newly added and not excluded test that 
testBar_IsActiveThirdState_DoesSomethingEntirelyDeffierentThat

That then immediately tells the project maintainers that the new changes 
are affecting some other areas that they had not considered.

Or, a more likely occurrence, when some downstream project team imports 
the new library and then uses their own custom classes, they will know 
that the API for class A was changed in the new API and they can decide 
how they should handle it in their own project.

Sorry for the really length explanation and if I'm just missing 
something "obvious" about how PHPUnit should be configured that would 
have caught this sort of thing.