Unit testen van een object die een static void object callt

[daanb14]

Mijn vraag
Op dit moment ben ik in groepsverband bezig met het ontwikkelen van een webapplicatie. Hierbij is het noodzakelijk om een gebruiker zijn e-mailadres te laten verifiëren door middel van een willekeurig gegenereerde token die in de link meegestuurd wordt. Deze token staat gehashed opgeslagen in de database. De methode die mailt naar de gebruiker heet mailTheUser. Deze methode roept voor het versturen van een bericht de abstracte class javax.mail.Transport aan. De methode Transport.send(message) is static. Ik wil de mailTheUser methode graag unittesten, omdat het natuurlijk de line coverage omhoog helpt. Ook wil ik graag verifiëren dat er inderdaad naar het catch gedeelte gegaan wordt, wanneer er een fout optreedt bij de static void methode in Transport. De mailTheUser-methode staat in de MailingServiceImpl class. Deze heeft een interface die hij implementeert. Dit is de service-layer. Alle andere services in de applicatie worden aangeroepen door de controller, maar deze service is daar dus een uitzondering op, omdat hij op meerdere plekken binnen de authenticatie gebruik wordt. Er wordt dus gebruik gemaakt van het REST pattern. Dit testen doe ik met PowerMock, omdat Mockito geen static void methodes kan mocken. Met PowerMock lijkt het te werken, maar dan wordt er wel een exception gethrowed, wanneer ik direct vanuit de test de Transport.send() methode aanroep. Roep ik in de test echter de mailTheUser() methode aan die getest moet worden, dan komt de exception er dus niet aan. Zou iemand weten hoe ik wel op een correcte manier Transport.send() een MessagingException kan laten gooien, wanneer ik hem aanroep in de test vanuit de mailTheUser() methode?

Relevante software en hardware die ik gebruik
JetBrains IntelliJ IDEA Ultimate 2018.3.2
Maven 3.6.0
TomEE Plus 7.1.0
Java EE
Java 8

Wat ik al gevonden of geprobeerd heb
Ik heb geprobeerd om het probleem op te lossen door gebruik te maken van PowerMock. Hierin heb ik aangegeven dat doThrow(new MessagingException("ExampleExceptionMessage") moet gebeuren wanneer de Transport.class aangeroepen wordt. Ook heb ik geprobeerd de MailTheUser() methode de exception niet te laten loggen, maar er voor tijdelijk een RuntimeException op te gooien.

Hieronder staat een voorbeeld van de MailTheUser() methode:

Java: MailingServiceImpl.class

public class MailingServiceImpl implements MailingService {

    private static final Logger LOGGER = Logger.getLogger(MailingService.class.getName());

    @Override
        void mailTheUser(String messageString, Account user, String link, String messageString2) {
        final String from = "[e-mail]";
        final String username = "exampleUsername";
        final String password = "examplePassword";
        String to = user.getEmail();
        Properties properties = this.getProperties();
        Session session = Session.getInstance(properties,
                new javax.mail.Authenticator() {
                    protected PasswordAuthentication getPasswordAuthentication() {
                        return new PasswordAuthentication(username, password);
                    }
                });
        try {
            Message message = new MimeMessage(session);
            message.setFrom(new InternetAddress(from));
            message.setRecipients(Message.RecipientType.TO,
                    InternetAddress.parse(to));
            message.setSubject("Verifieer uw account");
            message.setText(messageString + link + messageString2);
            Transport.send(message);
        } catch (MessagingException e) {
            LOGGER.log(Level.SEVERE, e.toString(), e);
        }
    }
    
    private Properties getProperties() {
        String fromHostServer = "smtp.gmail.com";
        String smtpPort = "587";
        Properties properties = new Properties();
        properties.put("mail.smtp.auth", "true");
        properties.put("mail.smtp.starttls.enable", "true");
        properties.put("mail.smtp.host", fromHostServer);
        properties.put("mail.smtp.port", smtpPort);
        return properties;
    }
}

Hieronder bevindt zich de unittest:

Java: MailingServiceImpl.class

@RunWith(PowerMockRunner.class)
@PrepareForTest({MailingServiceImpl.class, Transport.class})
public class MailingServiceImplTest {

    private MailingServiceImpl sut;

    @Before
    public void setUp() {
        sut = new MailingServiceImpl();
        PowerMockito.mockStatic(Transport.class);
    }

    @Test
    public void unitTestOfTheMailingMethodSuccessful() {
        String messageString = "testString";
        String testEmailAddress = "[e-mail]";
        Account userExample = new Account(testEmailAddress);
        String link = "dummyLink";
        PowerMockito.doNothing().when(Transport.class);
        sut.mailTheUser(messageString, userExample, link, messageString);
    }

    @Test
    public void unitTestOfTheMailingMethodFailed() {
        String link = "dummyLink";
        String messageString = "testString";
        String testEmailAddress = "[e-mail]";
        PowerMockito.doThrow(new MessagingException("ExceptionExampleText")).when(Transport.class);
        Account userExample = new Account(testEmailAddress);
        sut.mailTheUser(messageString, userExample, link, messageString);
    }
}

En de hierbij behorende Maven dependencies in de pom.xml:

XML: pom.xml

        <dependency>
            <groupId>org.mockito</groupId>
            <artifactId>mockito-all</artifactId>
            <version>1.10.19</version>
            <scope>test</scope>
        </dependency>

        <dependency>
            <groupId>org.powermock</groupId>
            <artifactId>powermock-module-junit4</artifactId>
            <version>1.7.4</version>
            <scope>test</scope>
        </dependency>

        <dependency>
            <groupId>org.powermock</groupId>
            <artifactId>powermock-api-mockito</artifactId>
            <version>1.7.4</version>
            <scope>test</scope>
        </dependency>

[ Voor 2% gewijzigd door daanb14 op 09-01-2019 11:34 . Reden: Gemiste tab gerepareerd. ]

[_360_]

Als je een class hebt die je onder test wil brengen waar static dependencies in zitten is er een goede strategie om je class testbaar te maken.

Wat je kan doen is deze statische dependency extracten naar een eigen class. Deze class heeft 1 method waar je de statische call maakt.

Van deze class extract je een interface. Die interface inject je vervolgens met dependencyInjection in je MailingServiceImpl class.

Vervolgens kan je met je favo Java mocking framework zeggen dat je interface method send() een MessagingException moet gooien als ie aangeroepen wordt.

[Haan]

Het is inderdaad vaak een goede aanpak om afhankelijkheden op externe (lees niet je eigen code) componenten weg te abstraheren.
Merk wel op dat je er qua code coverage niets mee opschiet aangezien je het probleem alleen ergens anders naartoe verplaatst

[_360_]

Haan schreef op woensdag 9 januari 2019 @ 19:10:
...
Merk wel op dat je er qua code coverage niets mee opschiet aangezien je het probleem alleen ergens anders naartoe verplaatst

Als ik het zou doen, zou ik het doen om mijn code die een dependency heeft op code die ik niet controleer, testbaar te maken. In dit geval het senden van een message. Maar het kan ook voor DateTime.Now zijn, of een service met een directe refererentie naar het fileSystem.

Persoonlijk geef ik niet zoveel om code-coverage als metric.
Code coverage zegt heel weinig over code coverage. Je weet als dotnetter dat je een test zo kan schrijven dat een bepaalde methode geraakt wordt zodat je coverage hoog lijkt. Maar het kan heel goed zijn dat die unit- of integratie test slechts een fractie van de gecoverde code test.

Het is daarom een misleidende metric. Goed om managers tevreden te houden. Maar het zegt niks over de kwaliteit van de tests en hoeveel van de business logic in je code daadwerkelijk getest wordt.

Maar goed, dit is een andere off topic discussie

P.s. even wat context: Ik heb op plaatsen gewerkt waar dit wel als een belangrijke deliverable werd gezien door managers. Daar zag je dat dit soort manieren werden toegepast om targets te halen.

Wat dan wel?
Public endpoints die goed worden gecovered door acceptance- en integration tests. Unittests die behavior testen en geen implementatie, zodat je kan refactoren zonder dat bij je bij het minste of geringste ook unittests moet gaan aanpassen.

[daanb14]

_360_ schreef op woensdag 9 januari 2019 @ 18:17:
Als je een class hebt die je onder test wil brengen waar static dependencies in zitten is er een goede strategie om je class testbaar te maken.

Wat je kan doen is deze statische dependency extracten naar een eigen class. Deze class heeft 1 method waar je de statische call maakt.

Van deze class extract je een interface. Die interface inject je vervolgens met dependencyInjection in je MailingServiceImpl class.

Vervolgens kan je met je favo Java mocking framework zeggen dat je interface method send() een MessagingException moet gooien als ie aangeroepen wordt.

Wat je dus bedoelt denk ik, is dat je een adapter maakt die de statische call maakt. Deze adapter zet je vervolgens een interface tussen en die roep je aan vanuit de MailingServiceImpl. Hopelijk klopt die beschrijving?

[_360_]

Een adapter maak je om een niet passende interface te laten passen op een andere interface. Ja met een beetje fantasie zou je dat hier ook onder kunnen scharen.

Je zou het ook een wrapper kunnen noemen. Die twee liggen dicht bij elkaar. Maar je beschrijving klopt. Die adapter class zal dus 1 regel echte code hebben. Nl. het maken van die statische call.

[daanb14]

_360_ schreef op woensdag 9 januari 2019 @ 22:19:
Een adapter maak je om een niet passende interface te laten passen op een andere interface. Ja met een beetje fantasie zou je dat hier ook onder kunnen scharen.

Je zou het ook een wrapper kunnen noemen. Die twee liggen dicht bij elkaar. Maar je beschrijving klopt. Die adapter class zal dus 1 regel echte code hebben. Nl. het maken van die statische call.

Heel erg bedankt. Ik merk dat ik nog veel te leren heb, maar dat is alleen maar leuk. Nu kan ik mooi lekker doorgaan met stubben. En er wordt inderdaad een metriek gesteld van minimaal 85%.

[ Voor 4% gewijzigd door daanb14 op 09-01-2019 22:27 ]

[Haan]

_360_ schreef op woensdag 9 januari 2019 @ 21:37:
[...]


Als ik het zou doen, zou ik het doen om mijn code die een dependency heeft op code die ik niet controleer, testbaar te maken. In dit geval het senden van een message. Maar het kan ook voor DateTime.Now zijn, of een service met een directe refererentie naar het fileSystem.

Persoonlijk geef ik niet zoveel om code-coverage als metric.
Code coverage zegt heel weinig over code coverage. Je weet als dotnetter dat je een test zo kan schrijven dat een bepaalde methode geraakt wordt zodat je coverage hoog lijkt. Maar het kan heel goed zijn dat die unit- of integratie test slechts een fractie van de gecoverde code test.

Het is daarom een misleidende metric. Goed om managers tevreden te houden. Maar het zegt niks over de kwaliteit van de tests en hoeveel van de business logic in je code daadwerkelijk getest wordt.

Maar goed, dit is een andere off topic discussie

P.s. even wat context: Ik heb op plaatsen gewerkt waar dit wel als een belangrijke deliverable werd gezien door managers. Daar zag je dat dit soort manieren werden toegepast om targets te halen.

Wat dan wel?
Public endpoints die goed worden gecovered door acceptance- en integration tests. Unittests die behavior testen en geen implementatie, zodat je kan refactoren zonder dat bij je bij het minste of geringste ook unittests moet gaan aanpassen.

Helemaal eens, maar het was de TS die over coverage begon Een bepaalde percentage unit test coverage vereisen is inderdaad weinig zinnig, het geeft hooguit een indicatie. Zo ben ik zelfs eens een test project tegen gekomen waarin tests zaten die de default constructor gingen aanroepen met als commentaar "To increase coverage"

[daanb14]

Nog een vraagje hierbij. Ik heb nu een klasse met een public void sendMessage(Message message) methode. Deze roept Transport.send() aan. Nu ik dit uit de MailingServiceImpl class heb gehaald, rest mij nog de vraag of iemand de naam zou weten van dit patroon en wat een goede naam zou zijn van de klasse die nu Transport met MailingServiceImpl verbindt?

[Haan]

Volgens mij is dat het Facade pattern, maar correct me if I'm wrong, het al eerder aangehaalde Adapter pattern lijkt er ook een beetje op, en ook het Decorator pattern, maar die vind ik beiden niet helemaal passen bij dit specifieke geval. Wel allemaal interessante materie om van op de hoogte te zijn voor in het vervolg
Een wrapper class die je om een externe dependency zet, noem je vaak gewoon XXWrapper , dus in jouw geval TransportWrapper.

[daanb14]

Haan schreef op donderdag 10 januari 2019 @ 15:10:
Volgens mij is dat het Facade pattern, maar correct me if I'm wrong, het al eerder aangehaalde Adapter pattern lijkt er ook een beetje op, en ook het Decorator pattern, maar die vind ik beiden niet helemaal passen bij dit specifieke geval. Wel allemaal interessante materie om van op de hoogte te zijn voor in het vervolg
Een wrapper class die je om een externe dependency zet, noem je vaak gewoon XXWrapper , dus in jouw geval TransportWrapper.

Heel erg bedankt voor het duidelijke en snelle antwoord. Ik zie dat jij dotnetter bent. Volgend jaar ga ik in het verdiepend semester met .NET core aan de slag. Ik ben ook van plan om na mijn opleiding mijzelf te gaan specialiseren in .NET C# development i.p.v. Java.

[_360_]

Haan schreef op donderdag 10 januari 2019 @ 09:17:
[...]

Helemaal eens, maar het was de TS die over coverage begon Een bepaalde percentage unit test coverage vereisen is inderdaad weinig zinnig, het geeft hooguit een indicatie. Zo ben ik zelfs eens een test project tegen gekomen waarin tests zaten die de default constructor gingen aanroepen met als commentaar "To increase coverage"

Daar heb je helemaal gelijk in. Dat zag ik pas nadat ik mijn rant over code-coverage had geplaatst.

[daanb14]

_360_ schreef op donderdag 10 januari 2019 @ 15:23:
[...]


Daar heb je helemaal gelijk in. Dat zag ik pas nadat ik mijn rant over code-coverage had geplaatst.

Wel een mooie onderbouwde rant, dat wel