I would like to draw a line in the sand on our project wrt testing.
From this point forward (this does not include any patches already on
list), I will reply with a NACK and not review any patch that does not include a cucumber test (if it is user-facing) or a spec (everything else).
Preferably, this will take the form of an extra patch in the series containing just the test. This patch should clearly show the functionality being added/fixed, and fail when applied in isolation. The reviewer should apply this patch first, run tests, and then apply the 2nd patch containing the functionality. At this point all tests should pass, and reviewer can proceed to give any feedback on structure or design of the actual code that is being proposed. Exceptions to this are items that are strictly css-related, or minor view markup that does not cause existing tests to fail.
Note that this is not directed at anyone in particular (we have all been guilty of this), but we have been talking about this for a very long time now, and I think everyone is now fluent enough in our testing frameworks that there is no excuse not to write tests for all new functionality and/or bugfixes.
If anyone has any issues with this please let me know, otherwise, I would encourage everyone to adopt this approach effective immediately.
-j