Skip to Content
Technical Articles
Author's profile photo Mike Pokraka

Writing simple, readable unit tests

One of the mantras for unit tests is that they should be simple and easy to write. The converse is often also true, if testing is difficult, then the tested code is too complex. I’ve found that writing unit tests has had a positive impact on the way I structure my code.

After reading the blog ABAP Unit test patterns – test cases and the short discussion that followed, I would like to offer an alternative approach to creating unit tests in a way that is both easy to write and easy to understand.

Christian makes an important point that tests also serve as documentation of the intended logic. I couldn’t agree more! As a fan of Clean Code principles, I also think that code should be self-documenting and it should be immediately clear what it does.

With this in mind, it is quite easy to write a unit test for testing a complex method with a table input that may look something like:

METHOD detect_duplicate_ids. 
  given_values( VALUE #( ( id = '1' )
                         ( id = '1' ) ).
  status_should_be( 'DUPLICATE_IDS' ). 
ENDMETHOD. 

METHOD detect_non_numeric. 
  given_values( VALUE #( ( id = '1' val = '1' )
                         ( id = '2' val = 'A' ) ).
  status_should_be( 'NON_NUMERIC' ). 
ENDMETHOD. 

METHOD sum_adds_values. 
  given_values( VALUE #( ( id = '1' val = '5' )
                         ( id = '2' val = '2' ) ).
  sum_should_be( 7 ).
ENDMETHOD.

METHOD sum_handles_negatives. 
  given_values( VALUE #( ( id = '1' val = '5'  )
                         ( id = '2' val = '-2' ) ).
  sum_should_be( 3 ).
ENDMETHOD.

So this represents the actual test part, which also doubles as documentation of the expected logic. It reads easily and the technicalities of unit test framework are tucked away in more appropriately named helper methods.

The implementation of the helper methods should also be simple. The test class is something like:

CLASS lcl_test DEFINITION FOR TESTING... 
...
  PRIVATE SECTION. 
    DATA cut TYPE REF TO zcl_myclass.   "Class Under Test
...
ENDCLASS. 

...

METHOD given_values. 
  cut = NEW zcl_myclass( i_values ). 
ENDMETHOD.

METHOD status_should_be. 
  cut->do_whatever( ). 
  cl_abap_unit_assert=>assert_equals( act = cut->get_status( ) 
                                      exp = i_status ).
ENDMETHOD.

METHOD sum_should_be. 
  cl_abap_unit_assert=>assert_equals( act = cut->get_sum( ) 
                                      exp = i_value ).
ENDMETHOD.

A tabular approach such as Frederik presented in his blog is useful in some scenarios, but I find that often my test scenarios used mixed inputs, where a table becomes cumbersome.

For readability, I really like the fact that you don’t need to specify all elements in a VALUE constructor. My example above may be a structure with 30 fields, and I can easily add further tests where different fields are relevant, such as:

METHOD future_dated_is_open. 
  given_values( VALUE #( ( keydate = sy-datum + 1 ) ).
  status_should_be( 'OPEN' ). 
ENDMETHOD. 

In other words, I’m not just testing a fixed series of values, but different input permutations, all with a reasonably simple syntax and without the noise of irrelevant data.

Astute readers may have noticed I skipped the ID in the last example. Another trick we can use is to code mandatory or default values into the ‘given_…’ method: Check for empty values and supply defaults. Keep the test method only about the test.

This is obviously just a simple example, the same technique can be applied to database abstractions where the ‘given’ methods prime the mock data accessed by the CRUD methods. I wanted to keep this blog simple, but I could do a more fully-fledged example if there is sufficient interest.

Assigned Tags

      18 Comments
      You must be Logged on to comment or reply to a post.
      Author's profile photo Harry M Flemming
      Harry M Flemming

      You have shared an informative blog here. I could understand more about writing unit tests by reading this post. It was difficult early. This blog would be helpful to many people to learn more about it. So glad to read this post. Keep it up your work. Keep on sharing informative posts more. Really looking forward to it.

       

      Author's profile photo Bärbel Winkler
      Bärbel Winkler

      Thanks for publishing this, Mike!

      I’m however mystified or even confused by some of the code-snippets you show. For example, what does the “do_whatever( )” in method STATUS_SHOULD_BE actually refer to as it doesn’t show up anywhere else? Is this just a placeholder which needs to be modified in any actual code? And why is a comparable statement missing in method SUM_SHOULD_BE?

      cut->do_whatever( ).

      I’m also not really sure where the code you show first (the block starting with “METHOD detect_duplicate_ids.” ) would need to be placed in the overall class the testing is intended for.

      I think what I’m missing is the overall picture – i.e. the complete code – for what your unit tests are meant for to really understand how this all hangs together and could be adapted in real scenarios.

      Speaking of real scenarios, I actually tried to define a unit test the other day for something really simple but was brought up short when I got the error message that unit tests can only be created for public methods and I wanted to have one for a private method. This kind of restriction makes unit testing somewhat useless, doesn’t it, considering the many not public methods a global class will usually have? I don’t really get, why unit testing shouldn’t be feasible to do for protected and/or private methods – especially if unit testing is supposed to be easy to do and implement. Defining methods as public just to make unit testing possible doesn’t quite seem to be the right way around this restriction.

      Cheers

      Bärbel

       

      Author's profile photo Christian Guenter
      Christian Guenter

      Hi Bärbel,

      you can also test private methods. All you need is to define the test class as local friend to the class under test.

      https://help.sap.com/doc/abapdocu_750_index_htm/7.50/en-US/abapclass_local_friends.htm

      ”Declaring local classes of a class pool as friends of the global class is necessary, in particular, for local test classes that test the private components of the global class.”

      Here's an example from the abapGit project.

      https://github.com/larshp/abapGit/blob/604c68200ed6a038229756f179fb86b9ccc9cc39/src/zcl_abapgit_file_status.clas.testclasses.abap#L14

       

      Best regards

      Christian

       

      Author's profile photo Bärbel Winkler
      Bärbel Winkler

      Thanks, Christian!

      I vaguely remembered something or other about "friends" in this context but not the details (and stll getting easily confused with ABAP OO doesn't really help with remembering the details!).

      So, I gave implementing a simple unit test another try and it finally worked once I had figured out the somewhat weird definition format needed in the local test class for the friends-definition.

      *"* use this source file for your ABAP unit test classes
      CLASS ltcl_unit_tests DEFINITION DEFERRED.
      CLASS zcl_check_wb_action DEFINITION LOCAL FRIENDS ltcl_unit_tests.
      CLASS ltcl_unit_tests DEFINITION FOR TESTING
                            RISK LEVEL HARMLESS
                            DURATION SHORT
                            FINAL.

      I'd not call this "intuitive" by any stretch of the imagination!

      Would it make sense and as a rule of thumb to always just have this sequence of statements for any test class? Or, put differently, wouldn't it make sense to have this definition automatically implied by ABAP to not needlessly run into these public vs. private restrictions for local test classes? I may be wrong given my limited experience with ABAP OO but based on the assumption that unit testing should be fairly granular I'd expect this to apply to private methods more than to public ones.

      Cheers

      Bärbel

      Author's profile photo Christian Guenter
      Christian Guenter

      My prefferred way is to put the local friends definition after the definition of the test class. So you save the "definition deferred" line.

      On your second question, well it depends. There's a school of thought heavily promoting the idea only testing public methods. I think they have some valid arguments, but I like to be pragramtic and test private methods if needed.

      Therefore I'd vote against to add the local friends definition automatically. To be sure that it was done intentionally.

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      Hi Bärbel,

      I must confess, I posted this while waiting for a flight, so it’s a made up pseudocode example. I used two variations on purpose, one where I’m just evaluating a function (get_sum) and another where my object is doing something and I’m evaluating the end result. Sorry if that confused instead of clarified.

      A more real-life variant might be:

      METHOD posting_in_future_should_fail. 
      
        given_values( VALUE #( postingdate = sy-datum + 1 ) ). 
      
        posting_should_fail_with(
          VALUE #( msgty = 'E'
                   msgid = 'ZFOO_POST' 
                   msgno = '104'  ) ). "Date % cannot be in the future
      ENDMETHOD. 
      
      METHOD given_values. 
        "provide default values for anything that's initial
        "instantiate object
      ENDMETHOD.
      
      METHOD posting_should_fail_with. 
      
        TRY. 
            cut->post( ).
      
          CATCH zcx_foo into data(error).
            data(message) = error->get_generic_message( ).  "i.e. without msgv1 etc.
      
        ENDTRY. 
        
        cl_abap_unit_assert=>assert_equals( act = message
                                            exp = i_message ). 
      
      ENDMETHOD.

      Regarding private methods, there’s some debate around whether private methods should even be tested. If you want to test them, the test class must be declared a FRIEND, Christian has already provided the details.

      But… if private methods are only a means to an end, and the “end” is always a public attribute or method, it can be argued that that is all that needs to be tested. An incorrect result of a private method will always manifest in a ‘public’ result somewhere. Otherwise there’s no point in the method!

      Alternatively, if a private method is really that complex that it needs individual testing, it should probably be it’s own (local) class. Then the private method(s) become public methods (though they can still stay local), and we can write a separate test class for it. This helps to keep classes simple and manageable, and is exactly what I meant by testing has the automatic benefit of helping structure your code better.

      Author's profile photo Bärbel Winkler
      Bärbel Winkler

      Thanks, Mike! Writing a blog post for  SAP Community while waiting for a flight sounds like a very good usage of available time to me!

      I really don't get why there's a debate about whether or not unit testing should even be done for private methods. If I write code from scratch which includes private methods, I'd like to ensure that they provide the expected results to whatever is calling them. Something I'd call a "bottom up approach" with ensuring that the internal small stuff works before building code which relies on it.

      If I only test the overall public result what does that really tell me if internally multiple private methods are called and I don't really know which one is the culprit?

      But then, it's entirely possible that I'm missing the point!

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      That’s why it’s a hot topic ?

      But think about it: a private method is just a helper to a public method. Either for encapsulation, or for removing redundancy, or for readability.

      So you should in theory be able to write a test that targets the private method functionality through the public method. Consider a crude example:

      METHOD times_two. 
        if is_number( i_num ). 
          result = i_num * 2. 
        else. 
          raise exception type zcx_no_number.
        endif.
      ENDMETHOD.
      
      METHOD is_number.  "Private method
        result = xsdbool( i_num CO '0123456789' ).
      ENDMETHOD.

      I will test is_number by:

      METHOD alpha_should_fail.
      
        TRY. 
            data(n) = times_two( 'A' ).
            cl_abap_unit_assert=>fail( 'No exception raised for alpha input' ).
      
          CATCH zcx_no_number.
            "As expected
      
        ENDTRY.
      
      ENDMETHOD.

      The point is to keep things simple. Lots of small tests that test one thing. 

      The advantage here is that if you have 5 private methods and start refactoring and moving code around, you don't need to rewrite your tests, because nothing should change outwardly.

      In fact this is where unit tests are at their most useful, because you can then safely move stuff around, incrementally, testing after each change. Move lines of code, ctrl-F3, ctrl-shift-F10, extract a new local method, ctrl-F3, ctrl-shift-F10, rename stuff, ctrl-F3, ctrl-shift-F10, ... you get the picture.

      Author's profile photo Bärbel Winkler
      Bärbel Winkler

      Well, I know that this is just - in your own words! - a crude example but I don't think that I'd write a separate method for a simple check as IS_NUMBER!

      For me, this is way too piecemeal coding to really help with making code understandable and it leads back to my alltime gripe with ABAP OO that the overall code gets much too fragmented to be easy to grasp due to the many - at least at first sight - disjointed pieces of the puzzle lying around in one big unstructured heap!

      Don't get me wrong, I really like the idea of providing unit tests in the code, but I don't much like the additional layer of complexity it introduces to already complex code! Not to mention that I'm still getting lost just in deciding how to best slice and dice code to make it "OO compliant" in the first place ?.

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      I don't think that has so much to do with OO. "big unstructured heap" sounds like bad OO, and I see no difference to a function module with 50 FORM routines (yes, real life case). I find the second variety a lot harder to work with, but that's just my opinion 😉

      A classic mistake I've seen a few times is 'spaghetti classes': Large classes with lots of public methods that barely interact with the class and in many cases might as well be static - they take input, do stuff with it, return.

      In fact, the whole point about this blog is that OO should be written easy to understand from ground up, then it needn't be so difficult. Yes, the syntax lends itself to smaller methods, and that should be a good thing. The idea is that you only read as deep as you need to.

      IS_NUMBER is obvious, I don't need drill into the code to see what it does. That's what the blog was about. Instead of wading through a 300 line module, it can be condensed into a few descriptively named methods and I can drill down into the one I'm interested in. As a reader I find that more efficient.

      Author's profile photo Paul Hardy
      Paul Hardy

      In regard to testing private methods. I always give myself the option using the FRIENDS thing above.

      It seems logical and obvious at first glance that you would want to test private methods, as you would want to test everything, surely...

      However as it transpires, when I started doing this for real I hardly every wrote a test for a private method unless it had stuffed up in production.

      My unit tests describe what a program should do as in "IT SHOULD default the default ship-to if a customer is entered which has a default ship-to" i.e. the tests represent the specification.

      So I have to fake the user entering a certain customer, and fake the configuration of the customer and the call a public method which processed the user input i.e. quite a high level method.

      This means you can change the HOW something is done and you will know at once if you have broken WHAT the program is doing.

      I also tend to write small methods, as it makes your code read more like English as in

      IF user_input is_a_number( ).

      sing_a_song( ).

      ENDIF.

      Robert Martin wrote a blog about atomising your code down to the lowest level possible and it got a lot of very emotional reactions both ways.

      Badly written OO code is a million times more difficult to understand than any sort of procedural code. If you do it correctly however the reverse is true.

      To quote Robert Martin once again your code should be a "screamer" as in that it should be screamingly obvious what it does at first sight. If a causal reader (e.g. business analyst) cannot understand it then it as a Bad Thing.

      Cheersy Cheers

      Paul

       

       

       

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      "Badly written OO code is a million times more difficult to understand than any sort of procedural code. If you do it correctly however the reverse is true."

      Very well said!

      When I started doing unit tests, I also tested private methods via FRIENDS. But by their very definition they MUST be executable - and therefore testable - via the public methods, otherwise they'd just be dead code or the design is poor.

      The trick is to split the tests to cover individual pieces of logic. If method A calls private method B then some tests will test A and others test B by calling A with the relevant inputs.

      While tests should be independent, they are cumulative. Thus we can write a test for every assumption until everything is covered. Sounds complicated, but is actually fairly easy.

      Author's profile photo Jelena Perfiljeva
      Jelena Perfiljeva

      Badly written OO code is a million times more difficult to understand than any sort of procedural code. If you do it correctly however the reverse is true.

      Couldn't agree more. I suspect this is one of the reasons that some procedurally-trained ABAP developers don't take a leap to OOP. The first efforts tend to not look good. Unless we make peace with it and let go, the self-preservation reflex would cause us to run back into the familiar territory.

      This is similar to the plateau phase that occurs in any weight loss effort. After some time, you reach a point when no progress is made or even you gain some weight back and you just give up and go back to eating pizza. But if you get past it, that's when you can really achieve the lasting effect.

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      Superb analogy 🙂

      Except that legacy code isn't nearly as nice as pizza. I'm currently fighting with a FM containing over 200 global variables and it does not equate to extra pepperoni.

      Author's profile photo Graham Robinson
      Graham Robinson

      I really like this post Mike – thanks,

      I am also very much enjoying the discussion about whether or not you should explicitly test private methods too.

      But taking your examples a little further – and by naming the “code under test” when – you can get a test case that looks like this.

        METHOD check_add_value.
          given_value( 6 ).
      
          when->add( 4 ).
      
          then_value_should_be( 10 ).
        ENDMETHOD.

      I think this adds an extra layer of cool. ?

       

      Cheers

      Graham Robbo

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      Hi Graham, 

      Thanks. Yes, I think private method testing is another blog in itself, also local classes (watch this space…).

      That is a classic setup-do-validate pattern that goes by a few names, sometimes even with four phases. I do follow this pattern, but I don’t like given-when-then for two reasons:

      Firstly, I don’t see the need for always splitting it into all three parts. I sometimes do, but just as often combine two of the phases, particularly if the “do” part is a functional method, such as  contains_order_number( cut->get_email_subject( ) ). 
      Or if the test name or method name already says clearly what we’re doing: “posting_should_fail” indicates that we are doing something and expecting an outcome. I think it’s clear, obvious and succinct. No need to split it.

      The second reason is more personal opinion. I never felt comfortable with the “given…when…then” terms. It may be grammatically correct, but it doesn't feel right; we don’t usually talk like that.

      I particularly don’t like “when”, it’s too wooly. In natural language it is more frequently used in a temporal sense. But here we’re using it as a condition (still OK), but to indicate the action phase. Yuk.

      But it gets worse: if we’re using the conditional meaning, “when” actually makes more sense for the setup (“given”) phase: “when the value is negative then I expect posting to fail” (note we still have a setup-do-validate sequence in there). Or “when the value is negative and I try to post a document then I expect it to fail”.

      But as I said this is my personal view and I won’t hold it against anyone who wants to use given-when-then.

      Author's profile photo Graham Robinson
      Graham Robinson

      Don't disagree with anything you said there - but I still think it add an extra layer of cool. LOL

      Author's profile photo Mike Pokraka
      Mike Pokraka
      Blog Post Author

      I don't disagree with that 🙂