Code Coverage: testing private functions

I was recently discussing whether or not you should directly test the functionality of private methods in a Go project. The other person reasoned for testing public and private functions to ensure 100% test coverage.

Testing private functions in Go is very simple. To do this you put your implementation and tests in the same package. The private functions are available in the test file as they are in the implementation file.

However just because you do not write tests that directly access the private functions, this does not mean you cannot achieve 100% code coverage. In fact if you follow Test Driven Development (TDD) most private functions will only be created during refactoring.

I prefer to only test public functions.

Lets go through a slightly contrived example.

We are going to implement a number package with two functions. One will add one integer to another. The second function will take one integer away from another. Both functions will return the result of the sum as the string representation of the number. i.e. result is 4, the value returned “four”.

We will only return a string for numbers between 0 and 10. If the number is out side of that range we will return “Cannot convert # to a string”.

Add Function

First we implement the functionality of the Add function. Testing and implementing for one simple addition with a result within our range and one addition with a result outside our range.

Tests

package number_test

import (
	"testing"

	"github.com/braddle/blog-testingPrivateFunctions/number"
)

func TestTwoAddTwoReturnsStringFour(t *testing.T) {
	act := number.Add(2, 2)
	exp := "four"

	assertEquals(t, exp, act)
}

func TestSixAndFiveReturnsNumberNotConvertableString(t *testing.T) {
	act := number.Add(6, 5)
	exp := "Cannot convert 11 to a string"

	assertEquals(t, exp, act)
}

func assertEquals(t *testing.T, exp, act string) {
	if act != exp {
		t.Error("Actual value did not match Expected value")
		t.Logf("Expected: %s", exp)
		t.Logf("Actual:   %s", act)
	}

}

Implementation

package number

import "fmt"

// Add function add together the numbers given and returns the result as a 
// string
func Add(a, b int) string {
	num := a + b

	switch num {
	case 4:
		return "four"
	default:
		return fmt.Sprintf("Cannot convert %v to a string", num)
	}
}

Run Tests

$ go test -cover -v
=== RUN   TestTwoAddTwoReturnsStringFour
--- PASS: TestTwoAddTwoReturnsStringFour (0.00s)
=== RUN   TestSixAndFiveReturnsNumberNotConvertableString
--- PASS: TestSixAndFiveReturnsNumberNotConvertableString (0.00s)
PASS
coverage: 50.0% of statements
ok  	github.com/braddle/blog-testingPrivateFunctions/number	0.001s

So far everything looks good. The tests are all passing and we have 100% test coverage.

Adding Minus

Now we implement the functionality of the Minus function. Testing and implementing for one simple subtraction with a result within our range and one subtraction with a result outside our range.

Tests

package number_test

import (
	"testing"

	"github.com/braddle/blog-testingPrivateFunctions/number"
)

// Removed Add Test for Brevity.

func TestSixMinusThreeReturnsStringThree(t *testing.T) {
	act := number.Minus(6, 3)
	exp := "three"

	assertEquals(t, exp, act)
}

func TestThreeMinusSixReturnsNumberNotConvertableString(t *testing.T) {
	act := number.Minus(3, 6)
	exp := "Cannot convert -3 to a string"

	assertEquals(t, exp, act)

}

func assertEquals(t *testing.T, exp, act string) {
	if act != exp {
		t.Error("Actual value did not match Expected value")
		t.Logf("Expected: %s", exp)
		t.Logf("Actual:   %s", act)
	}
}

Implementation

package number

import "fmt"

// Removed Add function for brevity.

// Minus function take the value of b from the value of a and return the 
// result as a string
func Minus(a, b int) string {
	num := a - b

	switch num {
	case 3:
		return "three"
	default:
		return fmt.Sprintf("Cannot convert %v to a string", num)
	}
}

Run Tests

$ go test -cover -v
=== RUN   TestTwoAddTwoReturnsStringFour
--- PASS: TestTwoAddTwoReturnsStringFour (0.00s)
=== RUN   TestSixAndFiveReturnsNumberNotConvertableString
--- PASS: TestSixAndFiveReturnsNumberNotConvertableString (0.00s)
=== RUN   TestSixMinusThreeReturnsStringThree
--- PASS: TestSixMinusThreeReturnsStringThree (0.00s)
=== RUN   TestThreeMinusSixReturnsNumberNotConvertableString
--- PASS: TestThreeMinusSixReturnsNumberNotConvertableString (0.00s)
PASS
coverage: 100.0% of statements
ok  	github.com/braddle/blog-testingPrivateFunctions/number	0.002s

We now have two functions and all the tests are passing with 100% test coverage. So far so good. However we have some duplication, this can be removed with a small refactor.

refactor

We already have all the test we need. We just need to move the duplicate code to it own function. There is currently no need for its functionality to be accessed from outside of the number package so we make it a private function.

package number

import "fmt"

// Add function add together the numbers given and returns the result as a
// string
func Add(a, b int) string {
	num := a + b

	return intToString(num)
}

// Minus function take the value of b from the value of a and return the 
// result as a string
func Minus(a, b int) string {
	num := a - b

	return intToString(num)
}

func intToString(num int) string {
	switch num {
	case 3:
		return "three"
	case 4:
		return "four"
	default:
		return fmt.Sprintf("Cannot convert %v to a string", num)
	}

}

Run Tests

$ go test -cover -v
=== RUN   TestTwoAddTwoReturnsStringFour
--- PASS: TestTwoAddTwoReturnsStringFour (0.00s)
=== RUN   TestSixAndFiveReturnsNumberNotConvertableString
--- PASS: TestSixAndFiveReturnsNumberNotConvertableString (0.00s)
=== RUN   TestSixMinusThreeReturnsStringThree
--- PASS: TestSixMinusThreeReturnsStringThree (0.00s)
=== RUN   TestThreeMinusSixReturnsNumberNotConvertableString
--- PASS: TestThreeMinusSixReturnsNumberNotConvertableString (0.00s)
PASS
coverage: 100.0% of statements
ok  	github.com/braddle/blog-testingPrivateFunctions/number	0.002s

Now when we run the tests they all pass, so our refactor is good. We still have 100% test coverage without having to add new tests to cover our new private function intToString. The moved functionality is being indirectly tested by the existing tests on our public functions.

The completed implementation that ensure all some that result in a value between 0 and 10 can be be found in this Github repository.

Using Slack to Be a Better Team Member

I am currently working out how I can be a better team member. I think one of the key parts of a team member is to ensure you are helping all of your team to become better programmers. By helping them to find the answer when they are asking questions and reviewing there code when they submit pull requests to give constructive feedback.

At work we use Slack as one of our communication tools. Other than just talking to each other it is the main way we ask each other questions or announce that a pull request is ready for review. I have started to use the Highlight Words of Slack feature to alert me to when there are members of my team looking for help but not directly asking me a question.

The Highlight Words feature allows you to provide a comma separated list a set of key words and phrases that you want to be alerted to as if someone has used @here or @username in one of the channel.

This is the current list of words and phrases I use to get alerts when someone is looking for help.

not sure why, for some reason, don’t get it, dont get it, make sense, makes no sense, help, question

I also use the pattern below to setup alerts for pull request links being posted on slack. I use this pattern so I only receive alerts about repositories I am interested in.

/[organisation]/[repo]/pull

 

If you are going to use them make sure you setup alerts for highlighting words at the top of the page.

Using this also means that I spend less time reading through slack.

 

This setting can be different for each of the teams you are in. Setup can be done here.

[team subdomain].slack.com/account/notifications

 

Writing A Testable API Client Library

Recently at work I had to create a PHP client library for one of our REST microservices. The client library will be used by many of our projects. I wanted to implement the library using test driven development

I had some trouble finding any advice or examples of client libraries that had be implemented using Test Driven Development where the tests did not actually rely on making calls the service.

I wanted Guzzle to be a dependency of the client library. Doing this would allow me to pass in a mocked Guzzle instance to my PHPSpec tests .This would mean my unit tests would not rely on communicating with API to pass.

However I did not want users of the client library to have to worry about the Guzzle dependency.

After a discussion with some colleagues I decided the best approach was have the client object require Guzzle as a constructor dependency but provide a static factory method for client library users to use for construction without providing the Guzzle dependency.

class PokeClient
{
    /**
     * @var \GuzzleHttp\ClientInterface
     */
    private $httpClient;

    /**
     * @var \Braddle\PokeApi\Factory\PokemonFactory
     */
    private $pokemonFactory;

    /**
     * PokeClient constructor.
     */
    public function __construct(
        ClientInterface $httpClient, 
        PokemonFactory $pokemonFactory
    ) {
        $this->httpClient = $httpClient;
        $this->pokemonFactory = $pokemonFactory;
    }

    /**
     * @return Client
     */
    public static function create()
    {
        $httpClient = new Client(
            ['base_uri' => 'http://pokeapi.co/api/v2/']
        );
        return new self($httpClient, new PokemonFactory());
    }

Any users of the client library now have a very simple way to instantiate the client library.

In this example the base URI is hard coded as the is only one version of the service. If you have different environments to work with you can pass  URI or a constant for the environment in to the create() function.

Creating an instance of the PokeClient is now as simple at this

$pokeCLient = PokeClient::create();

This implementation allows me to mock Guzzle calls in my PHPSpec tests.

class PokeClientSpec extends ObjectBehavior
{
    function let(Client $httpClient, PokemonFactory $pokemonFactory)
    {
        $this->beConstructedWith($httpClient, $pokemonFactory);
    }

    function it_is_initializable()
    {
        $this->shouldHaveType('Braddle\PokeApi\PokeClient');
    }

    function it_should_be_able_to_create_an_instance_of_itself()
    {
        $this::create()->shouldReturnAnInstanceOf(PokeClient::class);
    }

    function it_should_return_a_pokemon_when_attmepting_to_find_one_by_id(
        Client $httpClient,
        PokemonFactory $pokemonFactory,
        Response $response,
        StreamInterface $stream,
        Pokemon $pokemon
    ) {
        $pokemonId = 975;
        $json = json_encode(
            [
                'id'              => 34,
                'name'            => '',
                'base_experience' => 4,
                'height'          => 15,
                'is_default'      => false,
                'order'           => 1,
                'weight'          => 468,
            ]
        );

        $response->getBody()->willReturn($stream);
        $stream->getContents()->willReturn($json);
        $httpClient->request('GET', 'pokemon/' . $pokemonId)
            ->willReturn($response);
        $pokemonFactory->createPokemon(Argument::any())
            ->willReturn($pokemon);
        
        $this->findPokemonById($pokemonId)
            ->shouldReturnAnInstanceOf(Pokemon::class);
    }
}

This method also allowed my to add addition Guzzle options for my Behat integration tests

Feature: Getting Pokemon

    Scenario: Ensure that is it possible to find a Pokemon by its ID
        Given The client has been instantiated
        When I try to find a Pokemon by ID 1
        Then I should have been returned a Pokemon
class FeatureContext implements Context, SnippetAcceptingContext
{
    /**
     * @var PokeClient
     */
    private $client;
 
    /**
     * @var Pokemon
     */
    private $pokemon;
 
    /**
     * Initializes context.
     *
     * Every scenario gets its own context instance.
     * You can also pass arbitrary arguments to the
     * context constructor through behat.yml.
     */
    public function __construct()
    {
    }
    /**
     * @Given The client has been instantiated
     */
    public function theClientHasBeenInstantiated()
    {
        $this->client = PokeClient::create();
    }
 
    /**
     * @When I try to find a Pokemon by ID :id
     */
    public function iTryToFindAPokemonById($id)
    {
        $this->pokemon = $this->client->findPokemonById($id);
    }
 
    /**
     * @Then I should have been returned a Pokemon
     */
    public function iShouldHaveBeenReturnedAPokemon()
    {
        if (!$this->pokemon instanceof Pokemon) {
            throw new \Exception('No Pokemon found');
        }
    }
}

See the full example client library for the Pokemon API in this GitHub repository.

Recruitment Test GitHub Repository

Recently I have been giving my talk on Imposter Syndrome at a couple of meetups.

During the talk I mention that I used recruitment tests as a way to prove to myself that I can write good code and also as an extended code kata to improve my use of proper test driven development.

A number of people have expressed an interest in having a go at these tests themselves. I thought rather than emailing them out to people that were interested, I would make a little open source project out of it.

So I have created a GitHub repository containing a collection of the recruitment test that I have enjoyed implementing. I hope you enjoy the challenges as well.

If you have completed any fun or interesting recruitment test or you company have a good recruitment test it would be great if you could add to the project with a Pull Request. See the contribution if you do want to submit a test yourself.

Imposter Syndrome Lightning Talk (Video)

I recently gave a ten minute talk about Imposter Syndrome at a Lightning Talk session organised at work. Here is the video of that talk.

I decided to give the talk because I had been suffering with Imposter Syndrome for quite some time and for a lot of that time I did not realise that other developers suffered with the same fears that I did.

I wanted to share my experiences with Imposter Syndrome.
Why it affected me?
How it affected me?
How I am attempting to get over it.
To let others that are suffering with Imposter Syndrome that it is not just them that feel like that and It is possible to get past it.

This is also the same talk I gave as a lightning talk PHP North West Conference.

I hope to extend this 10 minute lightning talk into a slightly longer talk. I am not sure it could stretch to a full hour conference slot, but I think I could get it to 30 minutes meetup talk. If anyone has a meetup they would like me to present this talk at please get in contact I would love to hear from you.

Doctrine Entity Event

In my first post on Single Table Inheritance I had some example code that made use of a $tax variable to demonstrate where tax is added or removed. This simplification hides some complexity of the actual implementation.

In the actual implementation I needed to have access to a TaxCalculator object in all Price entities. The issue I had was how to ensure this happens without having to remember to manually inject the TaxCalculator everywhere Price entities are retrieved or persisted.

To do this I used a Doctrine feature called Entity Listeners. The Listener functionality allows you to access entities during a number of Doctrine operations.  Once configured you are able to hook into a number of different events before or after various CRUD events, they are:

  • preFlush
  • postLoad
  • PrePersist
  • PostPersist
  • PreUpdate
  • PostUpdate
  • PostRemove
  • PreRemove

The listening function is passed the entity that triggering the event and an instance of Doctrine\Common\EventArgs. In my example i do not use the EventArgs object. It can be used to give you another way to access the entity triggering the event or an instance of Doctrine/ORM/EntityManager.

The following Doctrine YAML configuration specifies the Entity Event Listener class (Braddle\EntityListener) for the Price entities.

Braddle\Entity\Price:
  type: entity
  table: price
  inheritanceType: SINGLE_TABLE
  discriminatorColumn:
    name: type
    type: string
  discriminatorMap:
    including_tax: PriceIncludingTax
    excluding_tax: PriceExcludingTax
  id:
    id:
      type: integer
      generator:
        strategy: AUTO
  fields:
    price:
      type: float
  entityListeners:
    Braddle\EntityListener:

Here is the implementation of my Entity Event Listener (Braddle\EntityListener) this listener injects a TaxCalculator into price entities. I have only used the postLoad and postPersist functions so that i can ensure that new Prices have a TaxCalculator injected when they are saved and all existing Prices when they are retrieved from the database.

namespace Braddle;

use Braddle\Entity\Price;

class EntityListener
{

    /**
     * Fired before an object is about to be saved by doctrine
     *
     * @param PricingConfigAware $object The model to inject the price config into
     *
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     *
     * @return void
     */
    public function prePersist(Price $object)
    {
        $object->setTaxCalculator($this->getTaxCalculator());
    }

    /**
     * Fired before an object is about to be saved by doctrine
     *
     * @param PricingConfigAware $object The model to inject the price config into
     *
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     *
     * @return void
     */
    public function postLoad(Price $object)
    {
        $object->setTaxCalculator($this->getTaxCalculator());
    }

    /**
     * @return \Braddle\TaxCalculator
     */
    private function getTaxCalculator()
    {
        return new TaxCalculator(20);
    }
}

You can find the full example application using the Doctrine Entity Event in this GitHub Repository.

Class Table Inheritance

The Class Table Inheritance feature of Doctrine allows you to have a number of different entities similar to Single Table Inheritance. The difference with Class Table Inheritance is it allows you to split the data specific to an entity in to a separate table. This allows each entity to have extra data specific to its needs, without having a table with a large number of null-able columns.

When creating a new entity you simply instantiate the class for the specific type required. When you persist the entity Doctrine creates an entry in the base table with the discriminator of your chosen class, Doctrine will also create an entry in the class specific table with a foreign key linking back to the base table.

When retrieving a specific entity from the database Doctrine uses the discriminator to decide which class to construct the data with. Doctrine also loads any class specific data from the class specific table.

If I were implement an Order Item system using Class Table Inheritance, I would create an abstract class and base table that could handle all of the generic Order Item functionality and data. Creating a concrete implementations for each specific Order Items requirements.

In the example configuration I have only created two concrete implementations one for products and one for vouchers.

# Model.Order.OrderItem.yml
Model\Order\OrderItem:
  type: entity
  inheritanceType: JOINED
  discriminatorColumn:
    name: type
    type: string
  discriminatorMap:
    product_order_item: ProductOrderItem
    voucher_order_item: VoucherOrderItem
  table: order_item
  id:
    id:
      type: integer
      generator:
        strategy: TODO
  fields:
    createdAt:
        type: datetime
  manyToOne:
    order:
      targetEntity: Model\Order\Order
      inversedBy:   orderItems
      joinColumn:
        name:                 order_id
        referencedColumnName: id
        
# Model.Order.ProductOrderItem.yml
Model\Order\ProductOrderItem:
  type: entity
  fields:
    sku:
      type:     string
      nullable: false
    quantity:
      type:     integer
      nullable: false
      
# Model.Order.VoucherOrderItem.yml
Model\Order\VoucherOrderItem:
  type: entity
  fields:
    voucherCode:
      type:     string
      nullable: false
    discountAmount:
      type:     float
      nullable: false

With the above configuration I would then create the classes below.

An abstract OrderItem that handles the link between OrderItems and their Orders and also maintains the time that an OrderItem was created.

class OrderItem
{

    /**
     * @var integer
     */
    private $id;

    /**
     * @var Order
     */
    private $order;

    /**
     * @var \DateTime
     */
    private $createdAt;

    public function __construct(Order $order)
    {
        $this->order     = $order;
        $this->createdAt = new \DateTime();
    }
}

The concrete ProductOrderItem extends the OrderItem with functionality specific to Products within an Order. Storing and validating the products Sku and required quantity.

class ProductOrderItem extends OrderItem
{

    /**
     * @var string
     */
    private $sku;

    /**
     * @var integer
     */
    private $quantity;

    public function setSku($sku) { ... }
    public function getSku() { ... }

    public function setQuantity($quantity) { ... }
    public function getQunatity() { ... }
}

The concrete VoucherOrderItem extends the OrderItem with functionality specific to Vouchers within an Order. storing and validating the voucher code and discount amount.

class VoucherOrderItem extends OrderItem
{

    /**
     * @var string
     */
    private $voucherCode;

    /**
     * @var float
     */
    private $discountAmount

    public function setVoucherCode($voucherCode) { ... }
    public function getVOcuherCode() { ... }


    public function setDiscountAmount($discountAmount) { ... }
    public function getDiscountAmount() { ... }
}

This implementation of the OrderItems maintains a clean database without any unecessary null-able columns. The implementation also conforms to the Single Responsibility Principle as a concrete OrderItem class only needs to be edited if the requirements of that specific type of OrderItem change. If I needed to create a new OrderItem for employee discount, I would create and configure a new EmployeeDiscountOrderItem entity in Doctrine without having to touch any existing OrderItem classes.

Single Table Inheritance

The Doctrine single table inheritance feature maps a number of different entity classes to a single table in your database.

When creating an entity you simple construct the desired class. When you persist the new entity to your data store, Doctrine stores the discriminator (class type identifier) with the rest of the entity’s data.

When retrieving a specific entity from the database Doctrine uses a discriminator to decide which class to construct with the data.

I have recently used the single table inheritance feature to refactor a Price class that used to use a boolean field to determine if the price contained within the entity was inclusive or exclusive of tax.

The previous code that made use of the Price entity looked something like this

$price = $priceRepository->findById(1);

$priceWithTax = null;

if ($price->isTaxIncluded()) {
    $priceWithTax = $price->getPrice();
} else {
    $priceWithTax = $price->getPrice() + $tax;
}

This code is difficult to init test this calling code given the number of routes through it.

However if we use single table inheritance we can refactor out the conditionals for Polymorphism.

Before I started to refactor the functionality the entity’s configuration YAML looked something like this

Slapi\Model\Product\Price\Price:
  type: entity
  table: price
  id:
    id:
      type: integer
      generator:
        strategy: AUTO
  fields:
    price:
      type: float
    taxIncluded:
      type: boolean

I then changed the YAML set up to something similar to this

# Model.Price.Price.yml
Model\Price\Price:
  type: entity
  table: price
  inheritanceType: SINGLE_TABLE
  discriminatorColumn:
    name: type
    type: integer
  discriminatorMap:
    including_tax: IncludingTax
    excluding_tax: ExcludingTax
  id:
    id:
      type: integer
      generator:
        strategy: AUTO
  fields:
    price:
      type: float

# Model.Price.IncludingTax.yml
Model\Price\IncludingTax:
  type: entity

# Model.Price.ExcludingTax.yml
Model\Price\ExcludingTax:
  type: entity

The new configuration gives us the following classes.

class price
{
    /**
     * @var float
     */
    private $price;

    /**
     * @return float
     */
    abstract public function getPrice();
}
class ExcludingTax extends Price
{
    /**
     * @return float
     */
    public function getPrice()
    {
        return $this->price + $tax;
    }
}
class IncludingTax extends Price
{
    /**
     * @return float
     */
    public function getPrice()
    {
         return $this->price;
    }
}

After the refactoring the entity to use single table inheritance to create a PriceIncludingTax and PriceExcludingTax class the code makes use of the price entities now looks something like this

$price = $priceRepository->findById(1);

$priceWithTax =  $price->getPrice();

The code in the second example is much easier to read and has no knowledge of the fact that a price entity return from the price repository could be inclusive or exclusive of tax. The calling code is also a lot easier to unit test as there is now only a single route through it.

Inheritance in Doctrine

I was discussing a feature of Doctrine that allows you to map object inheritance in entities. I have used this feature a number of times for a few different scenarios. I was very surprised that none of the other developers had used the feature before and a few had not even heard of it.

I thought it would be useful to discuss how I have used this feature to implement a clean design recently.

The feature has three different modes of working Single Table Inheritance, Class Table Inheritance and Mapped Superclass (which i have not used yet).