Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Lessons learned from testing legacy code

Lessons learned from testing legacy code

Testing a PHP web application is relatively easy when it designed to be "testable", with none of those ugly things such as excessive use of the global scope, lots of procedural code, and plenty of nested objects. Rarely do we step into such a project, making it difficult for developers to start making strides in adding automated testing to legacy code like this. In this talk I'll explore strategies for getting testing going inside your project, drawing upon experiences of making legacy code more testable.

John Mertic

April 16, 2012
Tweet

More Decks by John Mertic

Other Decks in Technology

Transcript

  1. ¡  John  Mertic   §  Contact  Info   ▪  http://

    jmertic.wordpress.com   ▪  Twitter:  @jmertic   §  Community  Manager  for   SugarCRM   ▪  http://www.sugarcrm.com   ▪  http://bit.ly/SugarDevBlog   ▪  Twitter:  @sugarcrmdev   ▪  [email protected]    
  2. THE  GOOD  PART   ¡  Code  is  established,  known  

    working   ¡  Functionality  is  well  vetted   by  the  stakeholders   ¡  End  users  are  well  trained   and  versed  it  using  it   AND  THE  BAD   ¡  Code  can  be  very   “patchwork”  like,  many   different  hands  over  time   ¡  Functionality  may  not   match  today’s  needs   ¡  End  users  work  with  the   system  versus  the  system   working  with  them  
  3. FUNCTIONAL   ¡  Tests  the  code  indirectly     from

     the  user  perspective   ¡  Tests  how  several   components  work  together   ¡  Can  be  easier  to  get  going   on  a  legacy  codebase   UNIT   ¡  Tests  the  code  directly   from  the  developer   perspective   ¡  Tests  individual   components  specifically   ¡  Tends  to  be  more   challenging  to  get  going  on   a  legacy  codebase  
  4. ¡  Procedural  code   ¡  Poorly  designed  objects   § 

    Static  dependencies   §  Busy  constructors   §  Really  looooooooooong  methods   §  A  bizarre  affection  for  the  global  scope  
  5. ¡  Procedural  code;  weigh  the  viability  of  a  unit  

    versus  functional  test   ¡  Build  helper  functions/classes/tools  to  do   repetitive  work  for  you.   ¡  Make  a  practice  of  resetting  the  global  scope   §  PHPUnit  can  do  this  for  you   ¡  Keep  focused  on  the  primary  goal  of  your   tests.  
  6. <?php class SugarTestUserUtilities { private static $_createdUsers = array(); private

    function __construct() {} public function __destruct() { self::removeAllCreatedAnonymousUsers(); } public static function createAnonymousUser($save = true, $is_admin=0) { // CLIPPED - Make the user } public static function removeAllCreatedAnonymousUsers() { // CLIPPED – clean up the users } public static function getCreatedUserIds() { $user_ids = array(); foreach (self::$_createdUsers as $user) if ( is_object($user) && $user instanceOf User ) $user_ids[] = $user->id; return $user_ids; } }
  7. <?php class MyTest extends Sugar_PHPUnit_Framework_TestCase { function setUp() { global

    $current_user; $user = new User(); $current_user = $user->retrieve('1'); } function tearDown() { } ---CLIPPED--- }
  8. <?php class MyTest extends Sugar_PHPUnit_Framework_TestCase { function setUp() { global

    $current_user; $current_user = SugarTestUserUtilities::createAnonymousUser(); } function tearDown() { SugarTestUserUtilities::removeAllCreatedAnonymousUsers(); unset( $GLOBALS['current_user']); } ---CLIPPED--- }
  9. <?php class ListViewDisplay { --clipped code section— /** * Display

    the listview * @return string ListView contents */ public function display() { if (!$this->should_process) { return ''; } $str = ''; if ($this->multiSelect == true && $this->show_mass_update_form) { $str = $this->mass->getDisplayMassUpdateForm(true, $this->multi_select_popup). $this->mass->getMassUpdateFormHeader($this->multi_select_popup); } return $str; } --clipped code section— }
  10. <?php class ListViewDisplayTest extends Sugar_PHPUnit_Framework_TestCase { public function setUp() {

    $this->_lvd = new ListViewDisplay; } public function testDisplayIfShowMassUpdateFormTrueAndMultiSelectTrue() { $this->_lvd->should_process = true; $this->_lvd->show_mass_update_form = true; $this->_lvd->multiSelect = true; $this->_lvd->multi_select_popup = true; $this->_lvd->mass = $this->getMock('MassUpdate'); $this->_lvd->mass->expects($this->any()) ->method('getDisplayMassUpdateForm') ->will($this->returnValue('foo')); $this->_lvd->mass->expects($this->any()) ->method('getMassUpdateFormHeader') ->will($this->returnValue('bar')); $this->assertEquals('foobar',$this->_lvd->display()); } }
  11. ¡  Write  a  unit  test  as  part  of  each  bug

     you  fix.   ¡  Learn  lessons  from  each  test  you  write   §  How  difficult  was  it  to  isolate  the  issue?   §  How  much  scaffolding  had  to  be  done  to  do  the   tests?   §  What  tools/techniques  would  make  testing  easier?  
  12. ¡  Find  ways  to  make  writing  the  tests  easier  

    §  Add  helpers  or  toolkits  to  use   §  Build  testing  templates   §  Build  example  tests  people  can  copy  from.   ¡  Expand  testing  to  full  components   §  Idea:  create  a  policy  that  if  you  add  a  unit  test  for   a  function/method,  you  should  make  sure  you  add   full  coverage  for  the  function/method  and  not  just   for  the  bug  itself  
  13. ¡  It  takes  too  much  time   ¡  It’s  too

     hard  to  test  a   certain  piece  of  code   ¡  I’ll  never  get  100%   code  coverage,  so  why   bother   ¡  I  can’t  justify  the  cost/ value  of  it   ¡  So  does  fixing  the   same  bug  over  and   over   ¡  Develop  a  better   strategy  for  testing   ¡  Nobody  has  100%  code   coverage   ¡  Adding  a  test  now   helps  prevent  the   problem  from  re-­‐ occurring  
  14. <?php class Contracts_Bug18732_test extends SugarTestCase { function testCurrencyLabel(){ $dictionary =

    array(); require('modules/Contracts/vardefs.php'); // make sure the Contract vardef vname reference for currency name is correct $label_name = $dictionary['Contract']['fields'] ['currency_name']['vname']; $this->assertEqual($this- >_mod_strings[$label_name], $this- >_mod_strings['LBL_CURRENCY']); } }
  15. ¡  Are  the  right  type  (  unit  vs  functional  )

      ¡  Isolate  and  cover  the  issue   ¡  Fail  if  the  fix  isn’t  in  place;  pass  if  it  is   ¡  Sensible  to  debug  
  16. ¡  Things  change   ¡  New  shiny  things  make  things

     easier   ¡  Pressure   ¡  Cluelessness  
  17. ¡  Learn  from  mistakes   ¡  Embrace  pragmatism   ¡ 

    Don’t  be  ignorant   ¡  Ask  for  help!