3.0 - Refactoring tests

[Frank de Graaf (Phally)]

Currently CakePHP's core tests are a bit messy. Most of them are integration tests instead of actual unit tests. We all have seen the problems caused by that: Random fails on the PaginatorComponent due to database specific issues or fails because the time is one second off (and many more).

Also finding and fixing bugs is made a lot more difficult because of that. For example this test for the PaginatorHelper: https://github.com/cakephp/cakephp/pull/1435/files

The above example would've been a lot easier with a mocked Router. We wouldn't have to build a CakeRequest and didn't need to check the tags. Just a single expectation to check if the array passed to the Router wasn't empty would have been enough for this test. Now this is just one example, but things like this are all over the place.

My proposal is to refactor every test case into separated unit tests and integration tests. This means:

• In unit tests use mocks where possible to ensure isolation.
  
• Make integration tests sensible.
• Refactor static classes (like the Router class) to make mocking possible.
• Have CakeTime implement wrappers for time(), strtotime(), etc. (so we can get rid of the time fails, by mocking the methods and have them return consistent values.)

The benefits when all this is done will be:

• Less dependencies within tests; easier to write a test case.
• Consistent test results; less chance of random errors popping up.
• Cleaner test code; tests are easier to maintain.
• Faster test execution; less tests will require a database.

Why start in 3.0 and not in 2.4? Well mocking static classes is only possible with 5.3+ because it uses late static bindings. Reference: http://sebastian-bergmann.de/archives/883-Stubbing-and-Mocking-Static-Methods.html

I like to start on this as soon as possible if everyone agrees with this. Thoughts, ideas and suggestions are welcome.

[José Lorenzo]

I think that is a very good goal to aim for. Nevertheless, integration tests have a very important role to play and we cannot neglect them. In my experience those kind of tests are the ones that catch the most of the bugs and also serve as a guide for other to understand how a feature is supposed to work in relation to the rest of the system.

Unit tests suffer from a problem that is very hard to solve, you cannot reliably mock every dependency specific behavior, developers often make assumption about how a dependencies is expected to respond under certain circumstances and fail to model every aspect of it. Databases are the prime example of this.

Stuff like CakeTime, Router and other classes should indeed be interchangeable by mocks in tests and I would love to see that happen. I also subscribe to the idea of separating our tests into two types: unit and integration, but that will mean double work for us... for a big chuck of each unit tested class we will also need an integration test counterpart :P

On CakeTime refactoring, what we need to do is not use the procedural functions but pass around an instance of a DateTime object, which is easy to replace, subclass and mock.

--
You received this message because you are subscribed to the Google Groups "cakephp-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cakephp-core...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[mark_story]

A big +1 to using DateTime objects instead of time() etc. The current CakeTime code is a bit of a disaster as it kept much of the compatibility from 1.x where DateTime didn't exist so all we had was time() and tz offsets.

I don't know if a formal separation between unit and integration tests will provide much value. It certainly opens more channels to bikeshed and have useless discussions. Doubling the work around testing seems like a net negative as well.

Having to setup Router and the current request is a pain, but sometimes important. I would however love to see a mockable router as the static class can be annoying in situations like this.  Could this test have avoided asserting all the tags and only looked for the interesting URL instead?

-Mark

[Sebastiaan van Stijn]

Sounds good.

Regarding the PHP 5.2.8 compatibility, IMO it may be a good opportunity to fine-tune the system requirements for the 2.5 and 3.0 release? Not only regarding the PHP version, but also the supported versions of databases.

I'll add this to the CakePHP 2.5 thread as well