akshatpradhan/spoutlets — pull request #69 by priyaaank (2013-03-26 19:25:24+00:00)
Title: Behavior specs

Hi Akshat,

Here is the answer to the second question that you had. (Somewhat). You had requested me to add specs for the behavior in entry model. Unfortunately, I still do not think you have enough behavior. There are couple of things in entry model.
* The width of different emotions. Which I felt weren't your core domain logic. For instance 40 as a value for happiness is great in Entry. However why does entry need to know that 40 should be multiple by 20 to display correctly on UI? Isn't that a UI specific functionality? If so, then it should be in view helper. So I removed the width; and added a width_for_emotion as a function in the entries_helper. I have also added a corresponding test. This reduces the code as it also removes duplication. And your model isn't aware of the changes being made to value for presenting them on UI. I tested this once and it seems to work fine on UI. Take a look

* With that gone, only thing that remain to be tested is validation and association with user. However if you really think about it, it's a piece of code that is owned by rails and is already tested. Do we really need to test if rails is doing associations correctly? Not much value there. However still to demonstrate I added very simplistic tests for entry and user model both. Very straight forward. No edge cases, as there isn't behavior.

* You also had a question around what should be tested in controllers? Well; you have quite a bit of storm brewing up in entries_controller_spec. While most of the bits are tested. There were two things that I could add value to. First you need to consider edge cases; like in my last pull request I mentioned that you have to ensure that stuff being deleted and updated is from same user. I have added a test and code modification for deletion scenario. Take a look. Also a minor improvement is showcasing the fact that you can indeed test your :notice messages easily in controller specs. Hope that will provide that extra bit of an edge to make it more test covered.

In principle, controller tests are only worried about that routing is happening to right paths/url and values are correctly assigned. People freely mock models and related behavior. While it's tolerable to an extent; excessive mocking will ruin the stability of tests. Changes in models keep controller specs unaffected. I worked on a project for a year where I mocked all models. At the end; we were sure if things were breaking at controller or not. In another project that ran even longer we mocked only as a last measure. Specs were overall slow since even controller specs required db interaction; but coverage was pretty solid. After doing both project; key learning was; keep controllers as slim as possible. Only routing logic and nothing else; will keep tests fast and actual model usage (via factories) will keep coverage steady and stable.

Some of these things I have showcased in this commit. Take a look. If you have any questions on this; let me know.