Slide 1

Slide 1 text

SURVIVING A LEGACY CODEBASE IT’S (PROBABLY) NOT AS BAD AS YOU THINK Jeremy Thurgood PyconZA 2017

Slide 2

Slide 2 text

WHAT IS LEGACY CODE? PRELUDE

Slide 3

Slide 3 text

“CODE FOR AN OBSOLETE COMPUTER” THE VERY OLD DEFINITION

Slide 4

Slide 4 text

“CODE INHERITED FROM SOMEONE ELSE” THE MOST COMMON CURRENT DEFINITION

Slide 5

Slide 5 text

“CODE YOU’RE AFRAID TO MODIFY” A MORE VISCERAL DEFINITION

Slide 6

Slide 6 text

“CODE WITHOUT TESTS” —MICHAEL FEATHERS A PRACTICAL DEFINITION THAT POINTS TO A SOLUTION

Slide 7

Slide 7 text

GET IT TESTED ACT 1

Slide 8

Slide 8 text

WRITE SOME TESTS All done, let’s go home. IF ONLY IT WERE THAT EASY.

Slide 9

Slide 9 text

WHY IS IT NOT THAT EASY? It just wasn’t written to be testable. Spaghetti Giant functions Global mutable state Tightly coupled dependencies

Slide 10

Slide 10 text

IT GETS WORSE OVER TIME Changing legacy code is scary. It doesn’t get refactored. Things get hacked in.

Slide 11

Slide 11 text

ALL IS NOT LOST! We can make legacy code testable. Very carefully. With limited, controlled changes.

Slide 12

Slide 12 text

SMALL EXAMPLE: CREATE_VM I want to modify this celery task… …but it creates a remote XenServer API session. @app.task(time_limit=120) def create_vm(vm, xenserver, template, name, **others): session = getSession( xenserver.hostname, xenserver.username, xenserver.password) storage = session.xenapi.SR.get_all() # ... Another 180 lines of VM creation using the session ... def getSession(hostname, username, password): session = xenapi.Session('https://%s:443/' % (hostname)) session.xenapi.login_with_password(username, password) return session

Slide 13

Slide 13 text

SMALL EXAMPLE: CREATE_VM Factor out everything that uses the session… …build a test double for the session object… …and then write some tests. @app.task(time_limit=120) def create_vm(vm, xenserver, template, name, **others): session = getSession( xenserver.hostname, xenserver.username, xenserver.password) return _create_vm(session, vm, template, name, **others) def _create_vm(session, vm, template, name, **others): storage = session.xenapi.SR.get_all() # ... Another 180 lines of VM creation using the session ... class FakeXenServer(object): """Fake XenServer to use in tests.""" # ... 300+ lines of implementation ...

Slide 14

Slide 14 text

THINGS TO NOTE We made one small change to the legacy code. We did not change the behaviour of the legacy code. We did not change the signature of create_vm. We built a test double that will be useful elsewhere. We can now start making safe, tested changes.

Slide 15

Slide 15 text

TESTING LEGACY CODE FOCUS ON THE TASK AT HAND Only test the relevant code Hacks are okay WRITE INVESTIGATIVE TESTS “What does it do if I give it this input?” Copy “expected” values from actual output

Slide 16

Slide 16 text

TOOLS AND TECHNIQUES ACT 2

Slide 17

Slide 17 text

FIRST, SOME TOOLS STATIC ANALYSIS ake8 pylint AUTOMATED REFACTORING PyCharm Rope THE HUMAN BRAIN

Slide 18

Slide 18 text

BREAKING DEPENDENCIES Test doubles! …but how do we get them in there? Break up long functions Add parameters Encapsulate global references Use seams

Slide 19

Slide 19 text

ENCAPSULATING A GLOBAL DEPENDENCY Instead of using reactor directly, put it in an attribute… …then override that attribute in the test. class SmppTransceiverTransport(Transport): clock = reactor # ... Quite a lot of transport implementation code ... def check_stop_throttling(self, delay=None): # ... A few lines of throttle-stop-checking code ... self._unthrottle_delayedCall = self.clock.callLater( delay, self._check_stop_throttling) def test_smpp_transport(): clock = Clock() transport = SmppTransceiverTransport() transport.clock = clock # ... The rest of the test ...

Slide 20

Slide 20 text

No content

Slide 21

Slide 21 text

SEAMS “A seam is a place where you can alter behavior in your program without editing in that place.” —Michael Feathers module imports function/method calls attribute lookups

Slide 22

Slide 22 text

EXPLOITING SEAMS Install a fake library Monkey-patch Subclass and override

Slide 23

Slide 23 text

BREAKING A DEPENDENCY WITH A SEAM We could extract getSession() as we did before… …by making nontrivial changes to untested code. :-( @app.task(time_limit=60) def updateServer(xenserver): session = getSession( xenserver.hostname, xenserver.username, xenserver.password) # ... Dozens of lines of code to update a server ... for vmref, vmobj in allvms.items(): updateVm.delay(xenserver, vmref, vmobj) # ... Dozens more lines of server-related code ... @app.task(time_limit=60) def updateVm(xenserver, vmref, vmobj): # ... A few lines of check and setup code ... session = getSession( xenserver.hostname, xenserver.username, xenserver.password) # ... Dozens of lines of code to update the VM ...

Slide 24

Slide 24 text

No content

Slide 25

Slide 25 text

BREAKING A DEPENDENCY WITH A SEAM Instead, we monkey-patch in our own getSession(). It’s ugly, but we know we didn’t break anything. :-) def test_updateServer(monkeypatch): from xenserver import tasks from xenserver.tests.helpers import XenServerHelper xshelper = XenServerHelper() monkeypatch.setattr(tasks, 'getSession', xshelper.get_session) # ... Way too many lines of code to test updateServer() ...

Slide 26

Slide 26 text

CHANGING UNTESTED CODE Separate new code from old. PROS: New code can be tested in isolation Changes to old code are restricted CONS: Spaghetti with meatballs Old code doesn’t improve Use with care, clean up later.

Slide 27

Slide 27 text

THE LEGACY CODE ALGORITHM 1. Identify code that needs to change 2. Break dependencies 3. Write tests 4. Make changes 5. Refactor (where possible)

Slide 28

Slide 28 text

READ THIS BOOK

Slide 29

Slide 29 text

THE END OF THE SLIDES Now you get to ask me hard questions. (Or easy questions. I prefer those.) Image credits: Unicorn cat from (CC By 4.0) Book cover from product page animalsclipart.com amazon.ca