DEV Community

Discussion on: When TDD doesn't click, something else should click "harder"!

Collapse
 
matthewbdaly profile image
Matthew Daly • Edited

If you have a program, what makes this program's UI marked as "valid"?

That's not a job for unit tests. This is the domain of high-level acceptance testing. I've used several of the Cucumber derivatives in the past (as I now work primarily with PHP I use Behat), and they're the same. They're useful for testing that the application's behaviour is acceptable to the end user, but they're on a different level to unit tests. For instance, here's an example of a Behat or Cucumber feature for the same use case you mentioned:

Feature: Login

    Scenario: Log in
        Given I am on "https://github.com"
        When I click the button "Sign up"
        I should see the text "Join Github"

I'd argue this is better than the syntax you mentioned because it's more completely decoupled from the actual underlying implementation to the point that it's very easy for even non-technical stakeholders to understand it. Each line in the test maps to a method, and anything in quotes is passed through as an argument, making it easy to reuse the steps. But you get the point - it's testing the application from an end-user's point of view, which is great, but in my experience is not sufficient in isolation. This kind of automated acceptance test tends to be comparatively slow, typically taking a minute or two to run, and that's too slow to enable real TDD, which realistically need a test suite that runs in ten seconds or less.

Also, this makes it very hard to test a wide range of scenarios. If you have two classes where one depends on the other, and both have two paths through the class, a unit test for each one should test both paths. But if you write a higher-level test for them, to have the same level of coverage you need to test four paths through the whole thing, and with more options it quickly gets out of hand to the point that you can't test all the possibilities. Don't get me wrong, this kind of high level acceptance testing is a very useful tool that has a place in my toolkit, but it's too slow to actually practice TDD, and can't cover as many scenarios as low-level unit tests can. High-level acceptance tests were my introduction to testing as a discipline, and they are the easiest way I know of to get started, but they cannot enable the sort of TDD workflow that unit tests can.

Something like a React or Vue component can be unit tested in isolation, and since that component will be used over and over again, it makes sense to do so. For instance, if you build a React component for a date picker, you don't want to have to test it over and over - you want one test file for the component that tests it in isolation. But more than anything else, if a component forms part of your UI, the thing you want to watch out for is unexpected changes in how the component renders. Using Jest snapshots, it's really, incredibly straightforward to check for that. All you have to do is render the component, and set up an expectation that it matches the snapshot, as in this example:

import React from 'react';
import ReactDOM from 'react-dom';
import Alert from './Alert';

it('should match the snapshot', () => {
  const div = document.createElement('div');
  const el = ReactDOM.render(<Alert />, div);
  expect(el).toMatchSnapshot();
});

If I'm working with React, I generally make a point of doing snapshot tests as a bare minimum for my components, since they're really easy to set up and will catch any unexpected changes to how my UI components render more efficiently. After that, it's a judgement call. React and Vue are easy to test, but other UI's may not be so straightforward. Snapshot tests don't test that a UI component is valid, but that it hasn't changed since it was last verified to be valid - as such it doesn't replace human eyes on the UI, but complements them by letting you know when the UI changes.

You are asking whether it's gonna call that mock function and return true, it will for sure, this is guaranteed by PHP!

This is a wrapper function and you're testing if the function client->get would work, but it's tested already in your framework, right?

Of course it is, which is why it's mocked out. We aren't testing our HTTP client, we're testing that we make the right calls to our HTTP client, and that's the critical difference. To use a common metaphor, unit tests are like double-entry bookkeeping in that they should express the inverse logic of the code under test. So for a service class that makes an HTTP request, we don't actually make a request, we just verify that our service calls the right methods on the mock, with the right arguments. A good unit test doesn't test anything other than that class.

In all fairness, controllers are an example of something that should be anaemic enough that testing them in isolation may not be worth the bother - if your controller is complex enough that it's worth unit testing, it may be a sign that it's doing more than is ideal.

I would typically pull other things out and test them with unit tests - for example, my database queries would be pulled out into separate repository classes, and things like API requests into separate service classes. I would then test that those made the right queries to the ORM, or made the right calls to the HTTP client, as appropriate, but if you have higher-level functional tests as well, then your controllers may not need separate unit tests.

It's actually very difficult to start practising TDD on an existing code base because they tend not to be sufficiently amenable to unit testing. I currently maintain a legacy Zend 1 application where most of the functionality is jammed into fat controllers, and I haven't got a hope in hell of being able to write unit tests for that until such time as I've refactored the functionality out of those controllers and into other classes. High level acceptance testing can be done on virtually any code base, but as stated isn't much use for TDD.

Thread Thread
 
yaser profile image
Yaser Al-Najjar

That's not a job for unit tests.

That's why I believe you shouldn't unit test your UI.

I'd argue this is better than the syntax you mentioned

I completely agree, Gherkin language is just better for testing in general.

I will go more in depth with that in the coming post.

and can't cover as many scenarios as low-level unit tests can

I understand what you mean, but really, do the user care about covering all the small details in the background? or do they want an overall working product?

React and Vue are easy to test

Yeah, I really love the part that they took these concerns in mind when building these tools!

Of course it is, which is why it's mocked out. We aren't testing our HTTP client

Can you please check my take with mocks and DI here: dev.to/0xrumple/comment/75bb

controllers are an example of something that should be anaemic enough that testing them in isolation may not be worth the bother

Exactly, I agree with you, controller shouldn't handle domain logic, model should instead.

It's actually very difficult to start practising TDD on an existing code base because they tend not to be sufficiently amenable to unit testing.

Afterall, you know that it comes down to the cost-value matter.

Thread Thread
 
matthewbdaly profile image
Matthew Daly • Edited

That's why I believe you shouldn't unit test your UI.

By definition, you can't unit test your UI, because it's not a single unit of code. A unit test is a test for a single unit of code, be it a function, a class or other logical unit, and the UI can't be described as a single unit of code. You can, and should, write higher-level tests for it, to make sure it fits together as expected, but you can't write a unit test for your UI.

However, you can and should write unit tests for individual components of your UI. In the example I gave of a React-based datepicker, you should write tests to ensure that it renders consistently based on given input, and it reacts in the correct way to various events. Or in the case of a jQuery plugin, you should test that it makes the appropriate changes to the DOM.

I understand what you mean, but really, do the user care about covering all the small details in the background? or do they want an overall working product?

Typically, the end-users aren't the ones you're held accountable by, the stakeholders are. And they're the ones who'll be affected if it turns out there's a fencepost error in a class that means clients have been undercharged. Unit tests tend to catch a different class of error than higher-level tests, because they're for testing that a class or function behaves as expected.

Plus, if there's a component you build and plan to reuse on other projects, such as a shopping cart, by definition you can't really test it alone by any means other than unit tests.

Don't get me wrong, unit tests alone won't catch every issue, but neither will high-level acceptance tests. There's a reason why Google advocate the so-called Testing Pyramid model, with unit tests making up 70% of the tests, functional tests another 20%, and high level acceptance tests the remaining 10%.

In addition, the other main benefits of unit tests are that they drive a better, more decoupled design, and tend to encourage the mental "flow" state that is most productive for developers.

It means you're testing what the mock is supposed to do, and NOT what the what the service as a whole is supposed to do... you're basically cheating yourself with a passing test for that mock code that's guaranteed to work.

No, that is categorically NOT the case. At no point in the example I gave does it make any assertions whatsoever about the response received from the mock. And it's quite patently wrong to state that it's "guaranteed to work". It's guaranteed to receive the specified input - you're simply testing how the class under test reacts to that input. To write good unit tests, you should treat the class under test as a "black box" - your test should pass in the input and verify it receives the correct output in response.

The whole reason to mock out dependencies in this context is so that we have absolute control over all the input to our class. We're setting up our expectations on our mock so we can make sure that:

  • it makes the calls we are expecting it to make to the mocked dependency, with the expected arguments, and
  • given it receives the response we specify on our mock, we can demonstrate that it reacts in the appropriate way

For instance, say we have the following code for an API wrapper:

$response = $this->client->get($url);
if ($response->getStatusCode() == 402) {
    throw new PaymentRequired;
}
if ($response->getStatusCode() == 401) {
    throw new RequestNotAuthorized;
}
return $response->getContent();

In this case, there are three possible paths through the method, depending on the HTTP status code returned. I'd test these as follows:

  • For the first case, I'd have the mock of the client return a response object with a status code of 402, and assert that my wrapper class throws the PaymentRequired exception.
  • For the second case, I'd have the mocked client return a response object with a status code of 401, and assert that the wrapper class throws the RequestNotAuthorized exception.
  • For the third case, my mock would return a response object whose content reflected a typical response from the API in question, and I would assert that the response received from the wrapper class was as expected. It's perfectly fine, and in fact commonplace, for the wrapper class to transform this response in some fashion, so the response received from the wrapper class may not simply be the exact same response received from the API, and the test should reflect that.

There are no assertions whatsoever made about the response received from the mocked HTTP client - indeed, no assertions should be made about the response from the mock, because we already know those as we set them earlier in the test. Every single assertion is about how the wrapper class handles the response from the mock. As mentioned, it's about having complete control of the input to the class being tested, so that you can be absolutely certain that under a specific set of circumstances, the class will behave in the predicted fashion.

you end up testing the framework

This is absolutely not true, because that part of the framework is mocked. At no time does the class under test interact with the mocked class, only with the mock. And where possible I would mock the interface that the class implements, not the actual concrete class.

I generally work with Laravel (which should feel somewhat familiar as it takes some inspiration from .NET), and in that it's commonplace to create an interface for a dependency and resolve that interface into a concrete class using dependency injection. So, for instance, if I had an application that needed to fetch weather reports for a particular geographical location, I might write the following interface:

<?php

namespace App\Contracts\Services;

use App\Objects\WeatherReport;

interface Weather
{
    public function get(string $name): WeatherReport;
}

Then I might create a class at App\Services\Weather\YahooWeather to implement that interface, and have the container resolve the interface to that class. If I need to migrate to OpenWeather, I simply create a new class implementing that same interface, and change the resolution, but no classes that use that service should need to change. My unit tests for the class that uses that service would mock the interface for the service, rather than any one implementation of that interface, to ensure it would work consistently with all of them.

It has to be said, HTTP clients are a particularly tricky example in this regard. In the PHP world, PSR18 has been recently accepted as a standard interface for HTTP clients, but it's a long way from widespread adoption. Once it is more widely adopted by client libraries, it'll be easy to have API clients specify only that interface and have it resolve to a concrete class through DI, but until that time HTTPlug is the best option in PHP. Technically it's a bad practice to mock a concrete class rather than an interface, but sometimes it's just not practical to do otherwise, and in a context where you can rely on that class being there and remaining consistent, such as when it's part of the framework, sometimes it's just not worth the hassle of wrapping that class, and it makes more sense to just mock that concrete class than to go down the rabbit hole of creating an abstraction on top of it that implements a specific interface and mocking that interface.

Of course you can create mocks for each and every service you face, but aside from the fact "it's not my business"... I don't have the luxury of time to do that, and I'm sure you don't as well.

If you're manually creating a lot of mocks, then yes, that is a chore, but that's no reason not to test, but a reason to look at how you're testing. And if the alternative is testing manually, I sure as hell don't have time to do that when I could just run the test suite in a matter of seconds. Plus, it's really, really scary how often there will be small changes made that are only caught by good unit tests, and if not caught could cause problems.

I think you might benefit from taking a look at some of the spec-style testing frameworks. Personally, I find that xUnit-style testing frameworks such as PHPUnit and NUnit don't enable the best workflow for TDD, partly because they require you to manually create your mock objects. I believe NSpec is the most prominent one of these in the .NET world.

A year or two ago I started using PHPSpec for an API client, and I've found that to be the best TDD experience I've ever had. A typical test method in PHPSpec looks like this:

function it_can_get_departures(HttpClient $client, MessageFactory $messageFactory, RequestInterface $request, ResponseInterface $response)                                                
{                                                                                                                                                                                         
    $appId = "Foo";                                                                                                                                                                       
    $key = "Bar";                                                                                                                                                                         
    $this->beConstructedWith($appId, $key, $client, $messageFactory);                                                                                                                     
    $url = "http://transportapi.com/v3/uk/train/station/NRW/live.json?app_id=$appId&app_key=$key&destination=LST&train_status=passenger";                                                 
    $messageFactory->createRequest('GET', $url)->willReturn($request);                                                                                                                    
    $client->sendRequest($request)->willReturn($response);                                                                                                                                
    $this->getDepartures('NRW', 'LST')->shouldReturn($response);                                                                                                                      }                          

This is about as complex as test methods ever get in PHPSpec (and most of that is because setting up requests with HTTPlug can be rather verbose), and most are far simpler. The beConstructedWith() method is only ever required when you want to override the default dependencies passed to the constructor - most of the time, your test will define a single let() method that specifies the default dependencies used to construct the class, as in this example:

function let (HttpClient $client, MessageFactory $messageFactory)                                                                                                                         
{                                                                                                                                                                                         
    $appId = "Foo";                                                                                                                                                                       
    $key = "Bar";                                                                                                                                                                         
    $this->beConstructedWith($appId, $key, $client, $messageFactory);                                                                                                                     
}

The typehinted dependencies are mocked out automatically, so you need only typehint a specific class to get a mock of it, and you can then set any expectations you need on it. This results in more concise tests, and you have to write less boilerplate than for an xUnit-style test.

So, to keep your tests more "honest", and not cheating yourself, the person who wrote that service (say HTTPClient) is supposed to write its mock as well so that you don't WRITE your own mock, but you USE an already built mock.

It's not a matter of writing a mock. I don't know enough about .NET to comment on how easy it is to mock dependencies, but I use Mockery in PHP if I write a PHPUnit test, and it's really trivial to mock a class with that, and for most purposes it wouldn't be useful to provide a mock version of that class, since you'd still need to set the expectations for it. If I mock a class with Mockery, I'm typically looking at one line of code to mock the class, and another line per expectation, which isn't exactly onerous. I have never needed to create any kind of mock server for this kind of use case.

HTTPlug is one of the few libraries I can think of that does have this sort of behaviour, as it has a mock driver that collects requests and returns specified responses, but it wouldn't require much more code to replace that driver with a mock of the driver interface.

Thread Thread
 
yaser profile image
Yaser Al-Najjar

We're exactly on the same page mate.

And all the things you mentioned (Cucumber, NSpec and high level acceptance tests) they are just what the next post is about, to be specific: BDD.

As for this code:

if ($response->getStatusCode() == 402) {
    throw new PaymentRequired;
}
if ($response->getStatusCode() == 401) {
    throw new RequestNotAuthorized;
}

I still can't accept testing this piece of code since it's a mere of if-statements that they will work FOR SURE as long as the other part (the HTTPClient) is gonna respond with 402 and 401 when needed.

Though, I would surely mock it if I have some logic inside the case of 402 to ensure my logic works as expected.

Plus, it's really, really scary how often there will be small changes made that are only caught by good unit tests, and if not caught could cause problems.

I agree with you, but the high level acceptance tests would keep both the stakeholders and the end-users so happy that "everything is just working"... rather than dropping TDD cuz of "passing the deadline".

I really enjoyed and benefited from the discussion with you... I promise, you will like the coming post ;)