Karianne lytter til testene [Luke 12, 2012]


onsdag 12. desember 2012 Julekalender TDD Java

Karianne Berg (@karianneberg) er et relativt kjent navn i det norske utviklermiljøet. Hun er en engasjert utvikler og en dyktig, morsom og uformell foreleser. I dagens adventsluke gir Karianne oss et innblikk i en av hennes åpenbaringer...

karianne

Hvem er du?
Ikke-bergenser i Bergen som liker å bygge ting.

Hva er jobben din?
Seniorutvikler i Vimond Media Solutions og arrangør av Booster-konferansen

Hva kan du?
Kodekvalitet 

Hva liker du best?
Det faglige engasjementet! Det er ingen tannleger som møtes på kveldstid i grupper over en øl for å diskutere den siste rotfyllingstypen, for eksempel.


Jeg har holdt på med testdrevet utvikling i en del år nå. Det er mulig at jeg er over gjennomsnittlig tjukk i huet, men en av de tingene jeg har hatt aller vanskeligst med å forstå er uttrykket «å lytte til testene». Det var et uttrykk som kom opp i mange bøker, artikler og presentasjoner om TDD og enhetstesting, men jeg var aldri i stand til å fatte hva det faktisk betydde. Ikke før i fjor.

For drøyt et år siden bestemte den fantastiske Kjersti B. Berg og jeg oss for at vi skulle holde en (i to deler) workshop om refaktorering på Smidigkonferansen. Vi ønsket å lage en hands-on workshop med en eksisterende kodebase som hadde full testdekning, men et dårlig design. Slik kunne vi ha en trygg "sandkasse" med kode som folk kunne bryne seg på å gradvis forbedre, med testene som et sikkerhetsnett. Mens vi laget denne kodebasen støtte vi imidlertid på et problem: Dersom vi innførte en feil, feilet testene slik vi ville de skulle gjøre. Imidlertid var det ikke bare en test som knakk, det var opptil halvparten av dem. Dette strider jo imot hva vi ønsket: En test skal teste bare en ting, og den skal knekke kun dersom du ødelegger funksjonaliteten for denne ene tingen.

Vi forsøkte alt vi kunne komme på for å bøte på dette: Vi delte opp tester. Vi slo sammen tester. Vi innførte flere mocks. Vi fjernet all mocking. Vi innførte hjemmesnekrede test doubles. Vi trakk ut initialisering av testdata i egne objekter. Vi reduserte antall assertions. Ingenting hjalp. Vi følte oss som totale enhetstestings-newbies. Men konferanser bytter dessverre ikke dato selv om de som skal holde workshops ikke er fornøyde med innholdet sitt, så vi måtte til slutt gi opp å forbedre testene. Det sluttet imidlertid ikke å surre rundt i huet mitt: Hva i alle dager var det vi hadde gjort feil? Hvorfor klarte vi ikke å fikse dette?

Det var ikke før jeg satt på flyet til Smidig at det endelig gikk opp for meg, og etterpå følte jeg meg så dum som ikke hadde skjønt det før at jeg tror jeg fysisk klaska håndflata i panna (og fikk den dresskledte sidemannen til å se på meg med en viss skepsis). Selvfølgelig! Det var jo ikke testene våre i seg selv det var noe gæærnt med, det var designet av produksjonskoden! Vi hadde jo med vilje laget et design som hadde en god del problemer for at folk skulle få øve se på å forbedre den. Det skulle da bare mangle at ikke testene klaget! «Problemet» vi hadde med testene var egentlig testenes måte å si: «HALLO!!?! Hva for en elendig programmerer er du egentlig? Koden din er pokker meg mer sammenfiltret enn vennekretsen til Trond Giske!»

Det er dette som etter min (nåværende!) forståelse er å lytte til testene. Det er å registrere at noe er vanskelig eller tungvint å teste, og så spørre seg selv «Hva er det med designet mitt som gjør at det er sånn?»

Jeg tror egentlig at mange som lærer seg TDD går gjennom et sett med stadier. Det første er "Testing suger!"-stadiet. Når man starter med TDD føles alt bare vanskelig, og det er lett å konkludere med at det er TDD som er en idiotisk teknikk. Når (hvis?) man kommer videre derfra, havner man på "Testene mine suger!"-stadiet. Her tror man gjerne at TDD er en bra måte å jobbe på, men at man bare ikke kan den godt nok og dermed ikke får den til. Det siste stadiet er "Designet mitt suger!"-stadiet. Her plasserer vi ansvaret der det oftest hører hjemme - hos designet til produksjonskoden. Det kan godt være at det er flere stadier her, men de har ikke jeg kommet til enda.

Dette siste stadiet tar tid å venne seg til. Det handler om å omstille hjernen fra "dette er umulig å teste!" til "hva er det i designet mitt som gjør det umulig å teste?". Fra "pokker, nå brakk alle testene!" til "hvorfor brakk alle testene når jeg gjorde dette?". Det er vanskelig, men for meg har det i alle fall gitt enormt utbytte.

Dette kan kanskje bli litt abstrakt, så la oss se på et eksempel (Red: Bare scroll ned om du synes det blir litt mye kode):

YahtzeeGame.java

public class YahtzeeGame {
    private final ThrowResultStrategy throwResultStrategy;
    private final Setgt; scoredCombinations;

    private Throw currentThrow;
    private int currentScore;
    private int currentRoundNumber;
    private List<Integer> currentlyHeldDice;
    private int currentNumberOfThrowsInThisRound = 0;

    public static final int NUMBER_OF_ROUNDS = Combination.values().length;
    private static final int MAX_NUMBER_OF_THROWS_PER_ROUND = 3;

    public YahtzeeGame(ThrowResultStrategy throwResultStrategy) {
        this.throwResultStrategy = throwResultStrategy;
        this.currentlyHeldDice = new ArrayList<Integer>();
        this.currentRoundNumber = 1;
        this.scoredCombinations = new HashSet<Combination>();
    }

    public void throwDice() {
        if (currentNumberOfThrowsInThisRound >= MAX_NUMBER_OF_THROWS_PER_ROUND) {
            throw new YahtzeeException("You cannot throw the dice more than "
                    + MAX_NUMBER_OF_THROWS_PER_ROUND + " times per round!");
        }

        Throw newThrow = throwResultStrategy.throwDice();
        currentThrow = currentlyHeldDice.isEmpty()
                           ? newThrow
                           : currentThrow.mergeWith(newThrow, currentlyHeldDice);
        currentNumberOfThrowsInThisRound++;
    }

    public int scoreFor(Combination combo) {
        if (noThrowsYetInThisRound()) {
            throw new YahtzeeException("You have to throw at " +
                    "least once before you score");
        }

        if (scoredCombinations.contains(combo)) {
            throw new YahtzeeException("This combination has " +
                    "already been taken!");
        }

        scoredCombinations.add(combo);

        int score = 0;

        switch (combo) {
            case ONES:
                score = scoreForNumberOfDiceWithValue(1);
                break;
            case TWOS:
                score = scoreForNumberOfDiceWithValue(2);
                break;
            case THREES:
                score = scoreForNumberOfDiceWithValue(3);
                break;

             // More cases here for other combinations
        }

        currentScore += score;
        currentRoundNumber++;
        currentNumberOfThrowsInThisRound = 0;
        currentlyHeldDice = new ArrayList<Integer>();
        currentThrow = null;

        return score;
    }

    private int scoreForNumberOfDiceWithValue(int value) {
        int num = currentThrow.findNumberOfDiceWithValue(value);
        return num * value;
    }

    public int finalScore() {
        return currentScore;
    }

    // More functionality here...
}

YahtzeeGameTest.java

public class YahtzeeGameTest {
    private ThrowResultStrategy resultStrategy;
    private YahtzeeGame game;

    @Before
    public void setup() {
        resultStrategy = mock(ThrowResultStrategy.class);
        game = new YahtzeeGame(resultStrategy);
    }

    @Test
    public void allOnesGivesFivePointsForOnes() throws Exception {
        when(resultStrategy.throwDice()).thenReturn(new Throw(1, 1, 1, 1, 1));

        game.throwDice();
        int score = game.scoreFor(Combination.ONES);

        assertThat(score).isEqualTo(5);
    }

    @Test
    public void allTwosGivesTenPointsForTwos() throws Exception {
        when(resultStrategy.throwDice()).thenReturn(new Throw(2, 2, 2, 2, 2));

        game.throwDice();
        int score = game.scoreFor(Combination.TWOS);

        assertThat(score).isEqualTo(10);
    }

    @Test
    public void allThreesGivesFifteenPointsForThrees() throws Exception {
        when(resultStrategy.throwDice()).thenReturn(new Throw(3, 3, 3, 3, 3));

        game.throwDice();
        int score = game.scoreFor(Combination.THREES);

        assertThat(score).isEqualTo(15);
    }

    // More similar tests here...
}

Klassen under test representerer et yatzyspill. Koden er forkortet litt av hensyn til lesbarheten, men om du leser testene, så bør det være noe som skurrer. Det de tester er at et gitt kast faktisk gir et gitt antall poeng for en gitt kombinasjon (for eksempel enere). For å kunne teste dette, må jeg imidlertid gjøre tre ting:

  1. Opprette et objekt av klassen YahtzeeGame
  2. Opprette en mock av interfacet ThrowResultStrategy, som gir oss et kast
  3. Kalle YahtzeeGame.throwDice()

Dette synes jeg føles fryktelig tungvint å teste. Jeg vil egentlig bare teste fjorten linjer kode (markert med gult) inni en metode i YahtzeeGame-klassen, men ender opp med å måtte mocke ut noe jeg føler jeg ikke trenger (ThrowResultStrategy) og kalle metoder som egentlig ikke er relatert til det jeg ønsker å teste. Med andre ord, testene har vist meg et problem. Dessverre er tester ofte litt som hylende babyer - det er ikke alltid så lett å skjønne hvorfor de klager, selv om du hører veldig godt at de gjør det.

Når en test krever veldig mye oppsett, slik som denne gjør, er det ofte et tegn på at klassen under test gjør mer enn den burde. Dersom vi ser nærmere på scoreFor()-metoden i YahtzeeGame, ser vi at den egentlig gjør alt. Den regner ut poengsummer (som er det vi forsøker å teste), men den holder også styr på hvilken runde vi er i, legger til poengsummen for akkurat denne kombinasjonen til den endelige poengsummen, resetter antall kast så langt i runden og gjør validering. Det er fryktelig mange ansvarsområder for en metode å ha, og YahtzeeGame-klassen bryter dermed med Single Responsibility Principle. Det er med andre ord ikke et testproblem, men et designproblem. Testene fortalte meg bare at noe var galt.

Alt jeg trengte å gjøre, var å høre etter.


comments powered by Disqus