Test Driven Development: un esempio pratico (CICLO 1, Task 2.5)

[fek]

Io e Vicius faremo Pair Programming in questo topic, svolgendo il task 2.5 totalmente test driven.

Non abbiamo ne' scritto il codice, ne' preparato i test prima di scrivere questo topic; avverra' tutto "live", e lo impostiamo come un dialogo fra me e lui, proprio come se fossimo davanti allo stesso PC.

Vediamo che cosa ne esce

Questo e' il task:
Riprodurre un suono quando il diamante collide con un bordo

Parto io, Vicius, scrivo il primo test nel file TestGemCollisionSound.java.
Ci serve un qualche modo per comunicare alla gemma che vogliamo riprodurre un suono e il suono da riprodurre.

package it.diamonds.tests;

import it.diamonds.Gem;
import it.diamonds.audio.Sound;
import it.diamonds.audio.SoundException;
import junit.framework.TestCase;

public class TestGemCollisionSound extends TestCase
{
    public void testSetCollisionSound()
    {
        Sound sound = null;
        
        try
        {
            sound = Sound.createForTesting("diamond");
        }
        catch(SoundException e)
        {
            fail();
        }
        
        Gem gem = Gem.createGemForTesting();
        
        gem.setCollisionSound(sound);
        
        assertTrue("Collision sound not set", gem.isCollisionSoundSet());
    }

}

Il test ovviamente fallisce, non compila, a te. Per prima cosa bisogna far compilare questo test e vederlo fallire.

Cercheremo di seguire TDD alla lettera, di volta in volta il minimo cambiamento possibile per far passare il test e poi eliminazione di tutte le duplicazioni nel codice.

[VICIUS]

eccomi.
Allora prima di tutto ho creato delle stubs per far compilare il programma. Questo è il codice:

    public void setCollisionSound(Sound collisionSound)
    {
        return;
    }

    public boolean isCollisionSoundSet()
    {
        return true;
    }

Ora è tutto verde. Quindi sono passato ad implementare un po il codice.
Prima di tutto ho aggiunto una variabile dove meorizzare il "suono".

    private Sound collisionSound;

Ora modifico il metodo che imposta il suono in modo che memorizzi l'istanza passata a setCollisionSound(). Ed ecco il codice:

    public void setCollisionSound(Sound collisionSound)
    {
        this.collisionSound = collisionSound;
    }

Ed ora passo ad implementare il codice per isCollisionSoundSet()

    public boolean isCollisionSoundSet()
    {
        return collisionSound != null;
    }

Per ora è tutto. La palla ha te fek.

ciao

[fek]

Vicius e' bravo, quindi lui puo' permettersi di saltare qualche passaggio, ma se avessimo voluto fare un passettino per volta, per andare piano piano, avremmo prima dovuto scrivere gli stub di modo da far fallire il test.

Quindi:

  public void setCollisionSound(Sound collisionSound)
    {
        return;
    }

    public boolean isCollisionSoundSet()
    {
        return false;
    }

Il test cosi' compila ma non passa. Ora mi serve il piu' piccolo cambiamento al codice che mi faccia passare il test.
Eccolo:

  public void setCollisionSound(Sound collisionSound)
    {
        return;
    }

    public boolean isCollisionSoundSet()
    {
        return true;
    }

Ora il test e' passato, e sappiamo che e' stato proprio il nostro cambiamento a farlo passare, perche' prima non passava. Eureka! Siamo un passettino piu' vicini alla fine del task.

Ora prima di scrivere altro codice, ci serve un altro test che fallisce, eccolo:

    public void testCollisionSoundNotSet()
    {
        Gem gem = Gem.createGemForTesting();
        
        assertFalse("Collision sound not set", gem.isCollisionSoundSet());
    }

Se lancio questo test con l'implementazione semplice, fallisce. Questo vuol dire che non abbiamo abbastanza funzionalita' per far passare entrambi i test.

Quindi?

Ecco un'implementazione semplice che fa passare entrambi i test:

    private boolean collisionSoundSet = false;
    
    public void setCollisionSound(Sound collisionSound)
    {
        collisionSoundSet = true;
    }

    
    public boolean isCollisionSoundSet()
    {
        return collisionSoundSet;
    }

Vicius, scrivi un test che ci porti da qui alla tua implementazione ora. A te, io lo implemento.

[VICIUS]

Ecco il test.

    public void testGetCollisionSound()
    {
        Sound sound = null;
        Audio audio = new Audio();
        audio.initListener();
        
        try
        {
            sound = Sound.createForTesting("diamond");
        }
        catch(SoundException e)
        {
            fail();
        }
        
        Gem gem = Gem.createGemForTesting();
        
        gem.setCollisionSound(sound);
        assertEquals("i due suoni non sono uguali", gem.getCollisionSound(), sound);
    }

ciao

[fek]

Mi serve per prima cosa far compilare il test:

    public Sound getCollisionSound()
    {
        return null;
    }

Ora il test compila e fallisce. Bene.
Mi serve l'implementazione piu' semplice possibile per farlo passare. Visto che setCollisionSound() mi passa un oggetto e poi il test vuole che getCollisionSound() gli restituisca lo stesso oggetto, questa e' l'implementazione piu' semplice che mi viene in mente:

    private boolean collisionSoundSet = false;
    private Sound collisionSound;
    
    public void setCollisionSound(Sound sound)
    {
        collisionSoundSet = true;
        collisionSound = sound;
    }

    
    public boolean isCollisionSoundSet()
    {
        return collisionSoundSet;
    }

    
    public Sound getCollisionSound()
    {
        return collisionSound;
    }

Il test passa assieme a tutti gl'altri. Barra verde. Eureka!

Ma c'e' una duplicazione che mi annoia. collisionSoundSet e collisionSound stanno esprimendo la stessa informazione in due posti diversi. C'e' una duplicazione e la elimino subito cosi'.

    private Sound collisionSound;
    
    
    public void setCollisionSound(Sound sound)
    {
        collisionSound = sound;
    }

    
    public boolean isCollisionSoundSet()
    {
        return collisionSound != null;
    }

    
    public Sound getCollisionSouind()
    {
        return collisionSound;
    }

Tutti i test continuano a passare! Un altro passettino verso la soluzione. Una duplicazione eliminata, il mondo e' un posto piu' semplice rispetto a due minuti fa'.

Siamo arrivati passettino per passettino alla stessa prima soluzione di Vicius, pero' in piu' abbiamo altri due test che saranno sempre li' a dirci se combiniamo qualche casino.

Vicius ho dubbio, che succede se passo un suono nullo a setCollisionSound()?

Aspetta, te lo scrivo in un test:

    public void testNullCollisionSound()
    {
        Gem gem = Gem.createGemForTesting();
        gem.setCollisionSound(null);
        
        assertFalse("Collision sound still set", gem.isCollisionSoundSet());
    }

Il test passa. E' il comportamento che vogliamo?

[VICIUS]

Originariamente inviato da fek

Il test passa. E' il comportamento che vogliamo?

Io personalmente lo eviterei. Propongo una bella eccezione. Se si vuole un suono null non lo si imposta e basta. QUindi popongo di cambiare il test in:

  public void testNullCollisionSound()
    {
        Gem gem = Gem.createGemForTesting();
        try {
            gem.setCollisionSound(null);
        } catch (ArgumentNullException e) {
            fail("boom!");
        }
    }

ciao

[fek]

Perfetto, a parte quell'orribile indentazione

Abbiamo preso una decisione di design su come trattare il valore null, ma la cosa interessante e' che ci siamo comunicati il design via test, e la decisione stessa via test. In maniera chiara.

Semplifico un po' i test e andiamo avanti, ho visto qualche duplicazione e codice inutile da eliminare.

Ecco i test:

public class TestGemCollisionSound extends TestCase
{
    public void testCollisionSoundNotSet()
    {
        Gem gem = Gem.createGemForTesting();
        
        assertFalse("Collision sound not set", gem.isCollisionSoundSet());
    }
    
    public void testSetCollisionSound() throws SoundException
    {
        Sound sound = Sound.createForTesting("diamond");
       
        
        Gem gem = Gem.createGemForTesting();
        
        gem.setCollisionSound(sound);
        
        assertTrue("Collision sound not set", gem.isCollisionSoundSet());
    }
    
    public void testNullCollisionSound()
    {
        Gem gem = Gem.createGemForTesting();
        try 
        {
            gem.setCollisionSound(null);
        } 
        catch (Exception e) 
        {
            fail("boom!");
        }
    }
    
    public void testGetCollisionSound() throws SoundException
    {
        Sound sound = Sound.createForTesting("diamond");
        
        Gem gem = Gem.createGemForTesting();
        
        gem.setCollisionSound(sound);
        assertEquals("collision sound is wrong", gem.getCollisionSound(), sound);
    }

}

Hmm... c'e' ancora l'oggetto gem e sound creati ad ogni test. Duplicazione da eliminare.

public class TestGemCollisionSound extends TestCase
{
    private Sound sound;    
    private Gem gem;
    
    
    public void setUp() throws SoundException
    {
        sound = Sound.createForTesting("diamond");        
        gem = Gem.createGemForTesting();
    }
    
    
    public void testCollisionSoundNotSet()
    {
        assertFalse("Collision sound not set", gem.isCollisionSoundSet());
    }
    
    public void testSetCollisionSound() throws SoundException
    {
        gem.setCollisionSound(sound);
        
        assertTrue("Collision sound not set", gem.isCollisionSoundSet());
    }
    
    public void testNullCollisionSound()
    {
        try 
        {
            gem.setCollisionSound(null);
        } 
        catch (Exception e) 
        {
            fail("boom!");
        }
    }
    
    public void testGetCollisionSound() throws SoundException
    {
        gem.setCollisionSound(sound);
        assertEquals("collision sound is wrong", gem.getCollisionSound(), sound);
    }

}

E' tutto cosi' semplice ora

Prossimo test ora, Vicius, che suggerisci?

[fek]

Non fosse che ho sbagliato un test.

 public void testNullCollisionSound()
    {
        try 
        {
            gem.setCollisionSound(null);
        } 
        catch (Exception e) 
        {
            fail("boom!");
        }
    }

Questo sta testando che non venga generata nessuna eccezione che e' il contrario di cio' che vogliamo.
Ecco il test giusto:

    public void testNullCollisionSound()
    {
        try 
        {
            gem.setCollisionSound(null);
        } 
        catch (Exception e) 
        {
            return;
        }
        
        fail("Exception not thrown");
    }

Che giustamente fallisce.

Vicius, a te. Il codice piu' semplice che fa passare il test.

[VICIUS]

Come prossimo test possiamo a controlalre che il suonon non venga riprodotto prima della collisione. Questo è il codice del test:

    public void testSoundBeforeCollision()
    {
        Sound sound = null;
        
        try
        {
            sound = Sound.createForTesting("diamond");
        }
        catch(SoundException e)
        {
            fail();
        }
        
        Gem gem = Gem.createGemForTesting();
        
        gem.setCollisionSound(sound);
        
        assertFalse("Sound must not be played before a collision",
            sound.wasPlayed());
    }

ciao

[fek]

Vai troppo veloce per me!
Fai prima passare il mio test.

[VICIUS]

Originariamente inviato da fek

Vai troppo veloce per me!
Fai prima passare il mio test.

OK. OK.

Allora in setCollisionSound non ho dovuto far altro che inseire un if per lanciare la nuova eccezione. abbiamo quindi questo codice.

    public void setCollisionSound(Sound sound)
        throws NullPointerException
    {
        if (sound == null) 
        {
            throw new NullPointerException();
        }
        
        collisionSound = sound;
    }

ora il test passa.

ciao

[fek]

Ora il tuo test, ecco la versione semplificata:

    public void testSoundBeforeCollision()
    {
        gem.setCollisionSound(sound);
        
        assertFalse("Sound must not be played before a collision",
            sound.wasPlayed());
    }

... che passa. Ecco, questo test va a rinforzare le maglie di test pero' non e' quello che ci serve per proseguire nel nostro task.

Una regola del TDD e' che non si aggiunge nessuna funzionalita' se non si ha un test che fallisce.

Quindi ora ci serve un test che fallisca per proseguire:
Eccolo qui:

    public void testSoundAfterCollision()
    {
        gem.setCollisionSound(sound);
        
        Input input = Input.createInputForTesting();
        input.generateKey(KeyCode.vk_Left, true);

        gem.setSpeed(1000.0f, 1000.0f);
        gem.reactToInput(input);
               
        assertTrue("Sound must be played after a collision",
            sound.wasPlayed());
    }

Non e' un test semplice, e c'e' quel metodo setSpeed che non abbiamo. E ci serve un test per scriverlo.

Mettiamo da parte questo test e ne scriviamo uno per setSpeed prima, che dici? Facciamo passare quello e poi torniamo qui.

Ecco il test:

    public void testSetSpeed() throws TextureNotFoundException
    {
        Input input = Input.createInputForTesting();
        input.generateKey(KeyCode.vk_Left, true);

        gem().setSpeed(10.0, 0.0);
        gem().reactToInput(input);
        
        assertEquals(90.0f, gem().getX());        
    }

Questo test e' in TestGemMove.java.

A te.

[VICIUS]

Perfetto procediamo con questo setSpeed. Questa volta cerco di procedere piu lentamente. Dunque prima lo stub.

    public void setSpeed(float vx, float vy)
    {
        return;
    }

Compila ma da semaforo rosso.
Ok. Facciamo passare questo benedetto test. Dunque visto che Gem è una classe figlia di Sprite che non devo far altro che chiamare setSpeed di sprite dal mio codice in questo modo.

    public void setSpeed(float vx, float vy)
    {
        super.setSpeed(vx, vy);
    }

Ancora rosso... Uhm... che cosa è successo ?

ciao

[fek]

Originariamente inviato da VICIUS

Ancora rosso... Uhm... che cosa è successo ?

E' successo che qualcuno ha implementato setSpeed in Sprite senza che fosse richiesto da nessun task o test (grrrr), e senza scrivere un test che lo testasse (grrrr#2) ed infatti non funziona.

Direi di togliere il metodo da Sprite e spostarlo a livello di Gem. Abbiamo un bel test che fallisce, dovrebbe essere piuttosto immediato farlo passare. Possiamo andare passo passo.

[VICIUS]

Come mi hai fatto notare il baco è banale. I valori vx e vy non vengono usati come velocita ma come variabili temporane mentre la velocita è sempre 1f. Per correggere è bastato cambiare nome alle due variabili temporane ed usare le due variabili vy e vx al posto del valore fisso di velocità.
qindi il codice ora è:

    public void reactToInput(Input input)

    {

        float dx = 0f;

        float dy = 0f;



        if(input.isKeyUp())

        {

            dy += vy;

        }

        if(input.isKeyDown())

        {

            dy -= vy;

        }

        if(input.isKeyLeft())

        {

            dx -= vx;

        }

        if(input.isKeyRight())

        {

            dx += vx;

        }



        move(dx, dy);

    }

Tutti i test ci danno semaforo verde. possiamo procedere.

ciao

[fek]

Ora possiamo tornare al test di prima che compila... e fallisce.

A te

[VICIUS]

Non ci resta che far partire il suono. Quindi vado nel metodo move della classe Gem e aggiungo il seguente codice subito dopo stopPulse();

collisionSound.play();

Faccio partire junit ed ecco che mi da luce verde. Test Passato.

ciao

[fek]

Mamma butta la pasta?

Ora, ci sono condizioni particolari da testare secondo te?

[VICIUS]

Originariamente inviato da fek

Mamma butta la pasta?

Ora, ci sono condizioni particolari da testare secondo te?

Quanto sei crudele

Dunque cosa succede se fate partire ad esempio Game o in uno sprite non impostate il suono di collisione? Semplice il sistema salta perchè mi sono, volutamente ,scordato di testa se collisionSound è diverso da null prima di riprodurlo. Quindi scrivo prima un bel test che faccia scattare il baco.

    public void testPlayNullCollisionSound()

    {      

        Input input = Input.createInputForTesting();

        input.generateKey(KeyCode.vk_Left, true);



        gem.setSpeed(1000.0f, 1000.0f);

        try 

        {

            gem.reactToInput(input);

        }

        catch (Exception e)

        {

            fail("collision sound è null");

        }

    }

Ora che ho un test posso passare a correggere l'errore inserendo il mio bel if nel codice per far passare il test precedente. Abbiamo quindi:

if (collisionSound != null)
                {
                    collisionSound.play();
                }

ciao

[fek]

Che dici si va a dormire ora o c'e' altro?