Tagged: solid Toggle Comment Threads | Keyboard Shortcuts

  • danielsaidi 9:14 am on October 18, 2011 Permalink | Reply
    Tags: decorator pattern, , solid,   

    Am I writing bad tests? 

    To grow as a developer, there is nothing as good as opening up your guard and invite others to criticize your potential flaws…as well as reading a book every now and then.

    In real life, you have to be a pragmatic programmer (provided that you are, in fact, a programmer) and do what is “best” for the project, even if that means getting a project released instead of developing it to the point of perfection.

    In hobby projects, however, I more than often find myself reaching for this very perfection.

    (Note: My earlier attempts to reach perfection involved having separate, standardized regions for variables, properties, methods, constructors etc. as well as writing comments for all public members of every class. I was ruthlessly beaten out of this bad behavior by http://twitter.com/#!/nahojd – who has my eternal gratitude)

    Now, I suspect that I have become trapped in another bad behavior – the unit test everything trap. At its worst, I may not even be writing unit tests, so I invite all readers to comment on if I am out on a bad streak here.

    The standard setup

    In the standard setup, I have:

    • IGroupInviteService – the interface for the service has several methods, e.g. AcceptInvite and CreateInvite.
    • GroupInviteService – a standard implementation of IGroupInviteService that handles the core processes, with no extra addons.
    • GroupInviteServiceBehavior – a test class that tests every little part of the standard implementation.

    This setup works great. It is the extended e-mail setup below that leaves me a bit suspicious.

    The extended e-mail setup

    In the extended e-mail sending setup, I have:

    • EmailSendingGroupInviteService – facades any IGroupInviteService and sends out an e-mail when an invite is created.
    • EmailSendingGroupInviteServiceBehavior – a test class that…well, that is the problem.

    The EmailSendingGroupInviteService class

    Before moving on, let’s take a look at how the EmailSendingGroupInviteService class is implemented.

    Code for parts of the e-mail sending implementation.

    As you can see, the e-mail sending part is not yet developed 😉

    As you also can see, the methods only call the base instance. Now, let’s look at some tests.

    The EmailSendingGroupInviteServiceBehavior class

    Let’s take a look at some of the EmailSendingGroupInviteServiceBehavior tests.

    Image showin parts of the test class

    As you can see, all that I can test is that the base instance is called properly, and that the base instance result is returned.

    Conclusion

    Testing the decorator class like this is really time-consuming, and for each new method I add, I have to write more of these tests for each decorator class. That could become a lot of useless tests.

    Well, the tests are not useless…they are just…well…I just hate having to write them 🙂

    So, this raises my final question:

    • Would it not be better to only test the stuff that differ? In this case, only keep CreateInvite_ShouldSendEmail
    Let me know what you think
     
    • Daniel Lee 10:52 am on October 18, 2011 Permalink | Reply

      I am going through a similar thought process. I would say that those tests that only test that the base instance was called are not worth it. There is no new logic being tested at all. It would be different if you changed the parameters and then called the base instance. If you already have tests on your base instance then that is good enough IMO.

      But I can totally understand where you are coming from. When practicing TDD it feels wrong to write a bunch of methods without writing tests for them. Maybe more coarse-grained tests in the layer above the email service class would solve this?

      This is really a .NET thing, if this was Ruby code then you’d just do this as a mixin. It’s only ceremony and not really providing any value. But I don’t know of a way to avoid this in .NET unfortunately.

      • danielsaidi 11:03 am on October 18, 2011 Permalink | Reply

        I totally agree with you…and also think it depends on the situation. In this case, where I work alone and my classes are rather clean, or when you work with people that share coding principles, then I agree that only testing the altered behavior is sufficient. However, if so is not the case, then perhaps thorough tests like these are valuable…

        …which maybe signals that you have greater problems than worrying over code coverage 🙂

        Thanks for your thoughts…and for a fast reply!

    • Daniel Lee 11:41 am on October 18, 2011 Permalink | Reply

      It’s a judgement thing (the classic ‘it depends’ answer). If you think that those forwarding methods are probably never going to change then those tests are mostly just extra baggage. But if you are pretty sure this is only step 1 then they could be valuable in the future.

      But if you have more coarse-grained tests (tests that test more than one layer) above these tests then they should break if someone changes the code here. For code this simple you don’t have to take such small steps with the tests. What do you think?

    • Daniel Persson 12:47 pm on October 18, 2011 Permalink | Reply

      If you really would like to test it, i would say only do one test – which is the setup and assert (one assert) on the expected result. No need for verify since it is verified in the setup to assert test.

      And whether you should do the tests or not, I agree with Daniel Lee. It’s probably not worth it, unless good reason. The behavior that changes from the base is the one that should be primary tested. If you overspecify, the tests will be harder to maintain and the solution it self be harder/more time consuming to change.

      • danielsaidi 1:00 pm on October 18, 2011 Permalink | Reply

        …which is exactly the situation I faced this Friday, when I ended up spending the entire afternoon adjusting one test class…which on the other hand probably had to do more with me writing bad tests 😉

    • Petter Wigle 8:11 pm on October 19, 2011 Permalink | Reply

      I think the tests tell you that the design could be improved.
      In this case I think it would have been better if the EmailSendingGroupInviteService would inherit from GroupInviteService. Then you would get rid of all the tedious delegations.
      Or you can rewrite the code in Ruby 🙂

      Otherwise I don’t think it is worth the effort to write test for code like this that is very unlikely to be broken. But if you do TDD then you’re using the tests to drive the design. The process is valuable even if the tests that come out of it is quite pointless.

  • danielsaidi 11:54 am on March 17, 2011 Permalink | Reply
    Tags: , infragistics, ioc, solid, ,   

    Infragistics – not that TDD friendly 

    I am currently working with cleaning up a WPF application that is tightly connected to Infragistics UI components. I am SOLIDifying, Dependency Injecting, IoC:ing and all those high fashion stuff. Does that turn me into a state-of-the-art developer? Time will tell 🙂

    However, since no unit tests existed when I took over responsibility for the solution, I decided to take the opportunity to wrap all code that I am rewriting in unit tests. Sadly, Infragistics does not seem to want you to test functionality that is based on their classes, since almost every class that I’ve come across so far has been internal. No public constructors, no interfaces…nothing.

    To work around the problem, I am currently wrapping everything within facade classes as well. It feels cheap and gives me a looot of additional work, but when I am done, I will at least have a shot at becoming the Joel Abrahamsson of the Infragistics universe. Not bloody likely, I know, but at least, I will probably put the resulting facade classes up for download.

     
    • Ivar 7:55 am on March 25, 2011 Permalink | Reply

      en annan lösning är ifall du kan gå in och pilla på assemblyt och lägga in [InternaldVisibleTo] och peka ut ditt testbinliotek.

      Ps. Trevligt i Barcelona!

      • danielsaidi 10:27 pm on March 25, 2011 Permalink | Reply

        Aaah, okej 🙂 Jag tror inte att jag kommer att göra det, eftersom projektet är rätt kort, men tack för tipset!

c
Compose new post
j
Next post/Next comment
k
Previous post/Previous comment
r
Reply
e
Edit
o
Show/Hide comments
t
Go to top
l
Go to login
h
Show/Hide help
shift + esc
Cancel