Wednesday, 13 March 2019

Enforcing Test Driven Development into an Existing Project


In an earlier post I wrote about my observations of a book I have recently read called Test Driven Development: By Examples by Kent Beck. The book talks about Test-Driven Development (TDD) and how to apply it practically. From this I wanted to get going with the concept as quickly as possible but without having to make an entirely new project. 

I started to think about how I could apply the practices taught into an existing node module I have written - and blog about too often - hosted on npm

In the chapter “How do you switch to TDD midstream” Kent describes how you might transition into using TDD and the issues that might rise from doing so. I have taken the liberty of summarising from my own perspective and will discuss how I implemented it into my latest football simulation engine code (blog)

After reading TDD and considering its application, there were 4 main factors (tips) for enforcing TDD in existing projects:
·     Test the requirements, not the code
·     Start with new features/bugs
·     Refactor code with testing in mind
·     Ignore working code

As I opened the code and assessed my kingdom the extent of the problem became apparent. There were no tests, but the project was thousands of lines of code, had 3 main exported functions and around 50 functions throughout with no test data or examples to be used by the end user.

Kent says in his book ‘The biggest problem is that code that isn't written with tests in mind typically isn't very testable’. Which happened to be my first thought as I dug into the large functions I had made. Where do I start? I began to write a list of what of the different parts and split them into a simple table with the headers Test, Ignore, Refactor - keeping in mind “What you don't do is go write tests for the whole thing and refactor the whole thing”. I wanted to decide what could be tested, what should be tested and what shouldn’t be tested. 

Test: A new test should be written to test the functionality I expected from the requirements.
Ignore: Large bulky functions which called other functions and needed a lot of intense work to make it test ready.
Refactor: Code that could be refactored to make it testable.

Test
Ignore
Refactor
Input Validation
Move Ball
Set Freekick
Closest Player to Ball
Find Possible Actions
Set Corner
Closest Player to Position
Resolve Ball Movement
Set Penalty
Check Offside
Decide Movement
Formation Check
Split Number into N

Shot Made

The first and easiest on the list was Input Validation, so I began to write the first test for when a games initiation input JSON was validated. The first test was simply that we got an object back. Success! This led to a quick imitation for the other two main functions of the project; start second half and play iteration.

 I then wrote some negative test cases by purposely inserting input data that shouldn’t be valid. Importantly, the question I asked myself was, what tests cases were valid based on the requirements and not the code that had already been written. This caught a number of validations I was not already catching in the validation functions.

An example was when I removed the ball from the play iteration function input and the test failed - by not failing. How can a football simulation work without the ball? This is why Tip 1 is: Test the requirements, not the code

mocha.it('playIteration no ball with team',async() =>{
      letprovidedItJson'./test/input/badInput/noBallWithTeam.json'
      try{
        letoutputIterationawaitvalidation.playIteration(providedItJson)
        expect(outputIteration).to.be.an('Error')
      } catch(err) {
        expect(err).to.be.an('Error')
        letexpectedOutput'Provide: position,withPlayer,Player,withTeam,direction,ballOverIterations'
        expect(err.toString()).to.have.string(expectedOutput)
      }
    })

Having fleshed out all the required checks on the input validation for the three main functions (using mainly negatives) I felt assured that the code would be able to tell the user when their input was wrong, and more specifically - why it was wrong. 

At the same time, I was doing some house cleaning of the code and some general refactoring. Additionally, a few bugs were raised in the GitHub repository by users of the module. 

Some related to queries on parameter names, others were refactoring suggestions, but some referenced specific issues being seen in the code. Players were getting stuck, the closest player to a position was incorrectly assigned and the ball was moving too quickly. Investigations also caused me to raise my own issues which I could also test. 

Bugs and new features for existing code mean that the requirements are set out before any changes or new code is written. The code should behave in a certain way and currently doesn’t. From this you can essentially work on a blank canvas in terms of writing tests (using TDD). Tip 2: Start with new features/bugs

For the bug of not calculating closestPlayerToBall, there was no way to validate the issue because there were no tests. I started to write a test case by placing players on the ‘pitch’ where I knew which player was closest to a position or the ball. The test then is simple - does the function, when given the input data, correctly identify which player is closest to the ball? 

Once we run the first test - and potentially get a FAIL, we can start TDD from the beginning. What is the simplest thing we can do to the existing code to make it PASS? By then following the TDD method the code was altered until it passed the conditions successfully. At which point we had a passing test for the closest player using TDD as the method. 

Writing tests for the whole project in this way and then ensuring they PASS would conform the project to TDD and I believe would reduce the number of small bugs that end up being raised in the Github repository. However, it is a long process and would take up a lot of productive time, when there are other big fixes and new features to address.

Another major issue was the way that shots are made. A player makes a shot, a bunch of logic plays out and then either the goalkeeper has the ball or it’s a goal kick. All without so much as a little ball or player movement. This leads to the next step in the introduction of TDD - Tip 3 - Refactor code with testing in mind.

The shot made function had been several hundred lines long and included a variety of methods within. This doesn’t test easily. As code was edited for v2.1.2 I started to make changes to make it as simple as possible, abstracting out functionality into smaller functions and thus making each component easier to test using TDD. 

It’s important to remember that this isn’t the time for TDD yet. TDD is the process and steps for creating code that passes the test. Following the rule of only writing tests for new features or bugs this extraction of logic into smaller forms sets us up for TDD without investing unproductive time into its making.

In the list I created at the start there appears to be no real difference between some of the functions in the ignore and the refactor slots. There are a number of reasons to ignore specific functions. For example;

-      The function is very large and/or complex
-      The function cannot easily be split into smaller functions
-      The tests would take a long time to write
-      The function isn’t broken

For example, in FootballSimulationEngine - Decide Movement cannot easily be split into smaller functions and is very large, Find Possible Actions would both take a long time to write tests for (because of its complicated logic) but also the function simply isn’t broken. Resolve Ball Movement requires a lot of random number generation and mathematics which makes it difficult to test and Move Ball isn’t broken either. 

This analysis focuses heavily on the code I have written and not the requirement which goes against tip 1, but this is mainly because of Tip 4 - Ignore working code. If it isn’t broken, don’t fix it.

Put in another way, it has taken two years (more or less) to get the code to where it is today, with time etched away from weekends and evenings. To use TDD in the whole project would mean to essentially start from scratch. In his book, Kent states “Spending money without making it is generally speaking not a sustainable process.” And as a subscriber to the statement ‘time is money’ it seems more responsible and a better use of time to slowly introduce TDD as bugs appear and are fixed, or new features are included. 

By v2.1.2 the list looked a big better. In future editions I can see some of the ‘Ignore’ items moving into the ‘refactor’ bracket, the ‘refactor’ tasks moving into the ‘test’ section and removing the ‘test’ tasks as tests are completed for them. See below for the table as of the commit of v2.1.2.

Test
Ignore
Refactor
Input Validation
Move Ball
Set Freekick
Closest Player to Ball
Find Possible Actions
Set Corner
Closest Player to Position
Resolve Ball Movement
Set Penalty
Check Offside
Decide Movement
Formation Check
Split Number into N

Shot Made
Shot Made



 If you want to see a wider use of TDD in the FootballSimulationEngine code I’d love to see more enhancements raised in the Github repo. Thanks for reading. 


No comments:

Post a Comment