Is this practice good on phpunit?

Question

when I'm coding I see often stuff like this:

     testMyMethod() {
        ....
        $mockMyServiceB
            ->expects($this->once())
            ->method('myMethodA')
            ->with(myvalue1, myvalue2, myvalue3)
            ->willReturn($someMockResult);

        $myServiceA = new ServiceA($mockMyServiceB)
        $results = $myServiceA->something();

        $this->assertEquals(['resultA', 'resultB'], results);            
     }

Not sure, but I think that as you write more and more tests, this make them to loose readability quickly and so easy. You just put too much info (expects mixed with returns). You need to understand for each test the order of the arguments, the expectations, order of execution, and return values....too much I think.

I was thinking to move the code to something where you just test the correct use of myMethodA, and later another test where you can focus only on results, like this:

  testMyMethodUseServiceBCorrectly() {
      ....
      // this time no WillReturn, just focus on how is used
      $mockMyServiceB
          ->expects($this->once())
          ->method('myMethodA')
          ->with(myvalue1, myvalue2, myvalue3);

      $myServiceA = new ServiceA($mockMyServiceB)
      $myServiceA->something(); 
 }


  testMyMethodUseServiceResults() {
      ....
      // this time no Expects() or With(), just focus on results
      $mockMyServiceB
          ->method('myMethodA') 
          ->willReturn(myvalue1, myvalue2, myvalue3);

      $myServiceA = new ServiceA($mockMyServiceB)
      $this->assertEquals(['resultA', 'resultB'], $myServiceA->something());
 }

I think this makes clear what you are testing, and also produce smaller tests. But not sure if is also usual.....is a recommended practice?

Short answer as a comment: It depends! I would suggest to write *complete* tests for each execution path. Your example suggests you are writing incomplete tests. The problem is that if you do that, you may have shorter tests methods, but they don't reveal the full picture. You might be able to make individual tests pass, but in reality the corresponding production code doesn't do what is expected of it. It's always easier and better to actually do TDD. If your tests become too big, they might indicate an opportunity for refactoring. These indicators are more important than short tests. — localheinz, Aug 27 '17 at 13:57
Hi, thanks for your answer. It gave me an interesting point of view and make sense. I can see that I can separate expectations from assertations as they test different behaviours, but still as you said I should try to test the whole execution path, thanks for the idea. — itaka, Aug 28 '17 at 22:01

score 0 · Answer 1 · answered Aug 24 '17 at 15:33

You can still make protected or private methods to configure mocks. For exemple, We have a project that has a class BaseTestCase extends TestCase with a basic createMock($classname) method from which are extending all unit tests in order to have some code for most Mock building uses.

Just take in mind to write readable tests code with clear names for vars and methods and to re-use code when possible.

Also, take in mind that if your test is getting bigger and bigger, maybe you need to refactor your class and get some simplified services.

score -1 · Answer 2 · answered Aug 27 '17 at 13:42

-1

There are few things to improve here:

Move creating instances of tested services into setUp() method
Use data providers to separate tests from data https://phpunit.de/manual/current/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.data-providers

Then tests would look like a conventional PHPunit tests. Also data providers would add a lot of readability and transparency when you name test cases properly inside a data provider (Example 2.6: Using a data provider with named datasets).

answered Aug 27 '17 at 13:42

Maksym Moskvychev

1,471
8
11

1

I would recommend against setting up the system under test in `setUp()`. It might have the advantage to reduce a bit of duplication, but the disadvantage is that you can't understand what a test does without looking at both the test and `setUp()`. Eventually you have the challenge that you want to inject and mock dependencies of the SUT differently in individual tests, and then things become less and less easy to understand. I recommend taking a look at https://stackoverflow.com/questions/6453235/what-does-damp-not-dry-mean-when-talking-about-unit-tests. – localheinz Aug 27 '17 at 13:52
1

Thank you, a very interesting discussion about DAMP and DRY. I'm for both concepts actually. In case of setUp, I usually put there "new Service($this->dependencyOne, $this->dependencyTwo)". Because if you write a unit test for Service - you would need this line anyway. And then inside of test methods - you define what exactly to mock in $this->dependencyOne, $this->dependencyTwo – Maksym Moskvychev Aug 27 '17 at 14:00
Apologies for down-voting, I'm reverting that! – localheinz Aug 27 '17 at 14:01

Is this practice good on phpunit?

2 Answers2