writing testable code (magetitans mini 2016)

WRITINGTESTABLE

CODEWriting Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

Vinai KoppFreelance Developer & Trainer

Tweet me @VinaiKopp

Writing Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

I want to get into testing


" WANT TO ""I have to GET INTO TESTING"


CHOOSE A TOOL +LEARN THE SYNTAX


▸ PHPUnit▸ PHPSpec▸ Codeception▸ nvm...


That wasn't so bad!


Current status:

A metric ton of untested legacy code


Start with writing tests for bugs


THIS IS HARDWriting Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

This costs lots of time!


This is painful!


There has to bea better way...!


<?php

/** * @todo make more testable *


Things to keep in mindto write TESTABLE CODE


KEEP CLASSES

SMALLWriting Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

A bad example:

Event Observer

(for Magento 1)


<?php

use Varien_Event_Observer as Event;

class Netzarbeiter_CustomerActivation_Model_Observer{ // Check if the customer has been activated, if not, throw login error public function customerLogin(Event $event) {...}

// Flag new accounts as such public function customerSaveBefore(Event $event) {...}

// Send out emails public function customerSaveAfter(Event $event) {...}

// Abort registration during checkout if default activation status is false public function salesConvertQuoteAddressToOrder(Event $event) {...}

// Add customer activation option to the mass action block public function adminhtmlBlockHtmlBefore(Event $event) {...}

// Add the customer_activated attribute to the customer grid collection public function eavCollectionAbstractLoadBefore(Event $event) {...}

// Add customer_activated column to CSV and XML exports public function coreBlockAbstractPrepareLayoutAfter(Event $event) {...}

// Remove the customer id from the customer/session, in effect causing a logout just in case public function controllerActionPostdispatchCustomerAccountResetPasswordPost(Event $event) {...}}


SMALLER?HOW?


First attempt:


WHAT DOES IT DO?


1. Prevent inactive customer logins2. Send notification emails

3. Add column to customer grid


splitNetzarbeiter_CustomerActivation_Model_Observer

into..._Model_Observer_ProhibitInactiveLogins..._Model_Observer_EmailNotifications..._Model_Observer_AdminhtmlCustomerGrid


<?php


class Netzarbeiter_CustomerActivation_Model_Observer_ProhibitInactiveLogin{ // Check if the customer has been activated, if not, throw login error public function customerLogin(Event $event) {...}

// Abort registration during checkout if default activation status is false public function salesConvertQuoteAddressToOrder(Event $event) {...}

// Remove the customer id from the customer/session, in effect causing a logout just in case public function controllerActionPostdispatchCustomerAccountResetPasswordPost(Event $event) {...}}


<?php


class Netzarbeiter_CustomerActivation_Model_Observer_EmailNotifications{ // Flag new accounts as such public function customerSaveBefore(Event $event) {...}

// Send out emails public function customerSaveAfter(Event $event) {...}}


<?php


class Netzarbeiter_CustomerActivation_Model_Observer_AdminhtmlCustomerGrid{ // Add customer activation option to the mass action block public function adminhtmlBlockHtmlBefore(Event $event) {...}

// Add the customer_activated attribute to the customer grid collection public function eavCollectionAbstractLoadBefore(Event $event) {...}

// Add customer_activated column to CSV and XML exports public function coreBlockAbstractPrepareLayoutAfter(Event $event) {...}}


Pretty rough...


Okay first step


MINOR DIFFERENCE INTESTING EFFORT


Second attempt:


Look closer


Dependencies:Netzarbeiter_CustomerActivation_Helper_DataMage_Customer_Model_CustomerMage_Customer_Model_SessionMage_Customer_Model_GroupMage_Customer_Helper_AddressMage_Customer_Model_Resource_Customer_CollectionMage_Core_Controller_Request_HttpMage_Core_Controller_Response_HttpMage_Core_ExceptionMage_Core_Model_SessionMage_Core_Model_StoreMage_Sales_Model_Quote_AddressMage_Sales_Model_QuoteMage_Eav_Model_ConfigMage_Eav_Model_Entity_TypeMage_Adminhtml_Block_Widget_Grid_MassactionMage_Adminhtml_Block_Widget_Grid


Business logic is hidden in Observer or HelperNetzarbeiter_CustomerActivation_Model_ObserverNetzarbeiter_CustomerActivation_Helper_Data


Observers link

Business logic!

Magento


Old Code:


public function customerLogin($observer){ $helper = Mage::helper('customeractivation'); if (!$helper->isModuleActive()) { return; }

if ($this->_isApiRequest()) { return; }

$customer = $observer->getEvent()->getCustomer(); $session = Mage::getSingleton('customer/session');

if (!$customer->getCustomerActivated()) { $session->setCustomer(Mage::getModel('customer/customer')) ->setId(null) ->setCustomerGroupId(Mage_Customer_Model_Group::NOT_LOGGED_IN_ID);

if ($this->_checkRequestRoute('customer', 'account', 'createpost')) { $message = $helper->__('Please wait for your account to be activated');

$session->addSuccess($message); } else { Mage::throwException($helper->__('This account is not activated.')); } }}


New Code:


public function customerLogin(Event $event){ if (! $this->isModuleActive()) { return; }

$this->getCustomerLoginSentry()->abortLoginIfNotActive( $event->getData('customer') );}


▸ Details are hidden▸ Delegation

▸ No magic getters


private static $sentry = 'customeractivation/customerLoginSentry';

/** * @return Netzarbeiter_CustomerActivation_Model_CustomerLoginSentry */private function getCustomerLoginSentry(){ return isset($this->loginSentry) ? $this->loginSentry : Mage::getModel(self::$sentry);}


▸ Dependencies can be injected▸ Business logic moved into model


Model with specific responsibilityclass Netzarbeiter_CustomerActivation_Model_CustomerLoginSentry


public function abortLoginIfNotActive(Mage_Customer_Model_Customer $customer){ if (! $customer->getData('customer_activated') { $this->getSession()->logout(); $this->getDisplay()->showLoginAbortedMessage(); }}


▸ Business logic independent of entry point▸ More type safety


private function getSession(){ return isset($this->session) ? $this->session : Mage::getModel('customeractivation/session');}

private function getDisplay(){ return isset($this->display) ? $this->display : Mage::getModel('customeractivation/display');}


▸ Injectable dependencies▸ Business logic in specific models


Need something done?

Need some information?


"I don't care how it's done!"


Delegate!Writing Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

IDCDDWriting Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

I Don't Care Driven Development


I Don't Care Driven Design


The result?


CLASS EXPLOSION!


SMALL CLASSES


TrivialTO TEST


How far can delegation go?


Delegate until the nextdelegator == delegatee


In the endour class

is wrappinga Magento class


In the endour class

is wrappinga Framework class


DECOUPLING FROMTHE FRAMEWORK


This is a Good Thing


ONE

FAQWHEN STARTING WITH TESTING


WHAT TO TEST?


Better question:


WHY DOESTHE CLASS EXIST?


WHY DOESTHE METHOD EXIST?


RETURN A VALUE

ORSIDE EFFECT


If a method

RETURNS A VALUEonly test for that


If a method

CAUSES A SIDE EFFECTonly test for that


SIDE EFFECTS?


Side Effect #1:Global State


▸ Filesystem▸ Databases

▸ Global Functions + Variables▸ Static Method Calls + Properties

▸ Forking


Side Effect #2:A method call on another object


if (! $customer->getData('customer_activated') { $this->getSession()->logout(); $this->getDisplay()->showLoginAbortedMessage();}


TESTINGRETURN VALUES

IS EASIER THANSIDE EFFECTS


Avoid creating methods that do both:side effect && return value


ALSO TEST FOREXCEPTIONS


Key PointsWriting Testable Code - MageTitans Mini, 5th May 2016 - ! - [email protected] - twitter://@VinaiKopp

▸ Separation betweenHooks or Entry Points

andBusiness Logic


▸ Split Business Logic into specific classes


▸ Use IDCDD to find where to separate


▸ Also separate code thatreturns a valuefrom code that

causes a side effect


▸ Test if a class fulfills it's purpose


▸ Don't use magic methods

(No more calls to __call())


▸ Avoid method call chaining

(Don't return $this);


▸ Mainly for Magento 1.x:Make dependencies injectable for testing


...and have fun testing!


(tell you me comment)(ask? you me question)

(thank you)


writing testable code (magetitans mini 2016)

Software