A simple way to increase maintainability of ABAP unit tests – with BOPF example
I am a fan of unit testing. I have been using it in my code for the past 2 years even though it wasn’t asked for. I thought I was good at it until I attended the ABAP Unit Testing OpenSAP course and read the book xUnit Test Patterns. These two gave me very good perspectives and tools to up my unit testing game.
While the core lessons have been written about several times, I think the most important lessons are
- Unit tests must be easy to write, read and maintain.
- While the first test might take a while to write, the subsequent ones should take very minimal time.
- Most important of all. The unit test must be self contained. So setting up the data (for the dependent object calls), running the code under test and verifying should be in the same unit test.
The last one helped me make my unit tests more readable and maintainable.
In my recent project, we use BOPF. In this blog, I wish to illustrate how to organize the unit tests better with a case where we use BOPF. Its using the above 3 points.
For our case, let us consider the BOPF BO as follows:
Header -> Item. Item contains a field ‘Status’.
Status can be Not started, completed, error
Now for our example, let us say we have to provide a method that returns the number of items that are completed.
For those of you familiar with BOPF, to read data from items, we require the following:
- Get instance of service manager.
- Call retrieve_by_association on header passing header key to get the items.
So here we go. This is our class and method. I know that its a good practice to write the unit tests before. But bear with me for now.
CLASS zcl_unit_test_example_class DEFINITION
PUBLIC
FINAL
CREATE PUBLIC .
PUBLIC SECTION.
METHODS get_completed_items_count
IMPORTING
!iv_key TYPE /bobf/conf_key
RETURNING
VALUE(rv_count) TYPE int4 .
PROTECTED SECTION.
PRIVATE SECTION.
ENDCLASS.
CLASS zcl_unit_test_example_class IMPLEMENTATION.
METHOD get_completed_items_count.
DATA: t_items TYPE bo_items.
rv_count = 0.
TEST-SEAM srv_mngr.
DATA(o_srv_mngr) = /bobf/cl_tra_serv_mgr_factory=>get_service_manager( if_i_status_bo_c=>sc_bo_key ).
END-TEST-SEAM.
o_srv_mngr->retrieve_by_association(
EXPORTING
iv_node_key = if_i_status_bo_c=>sc_node-header
it_key = VALUE #( ( key = iv_key ) )
iv_association = if_i_status_bo_c=>sc_association-header-items
iv_fill_data = abap_true
IMPORTING
et_data = t_items
).
rv_count = REDUCE int4( INIT result = 0
FOR wa IN t_items WHERE ( wa-status = 'Completed' )
( result = result + 1 ) ).
ENDMETHOD.
ENDCLASS.
Now, I have wrapped the service manager instantiation into a seam. This works better for me than other methods of dependency injection because
- It doesn’t affect run time.
- No extra methods or factory is needed for unit testing.
- No need for the unit test class to be friends of any class.
In the unit test, I create a local mock BO class. Setting data for this shall be done in each unit test itself. Code for this is as follows.
CLASS lcl_bo_mock DEFINITION FINAL.
PUBLIC SECTION.
INTERFACES /bobf/if_tra_service_manager.
CLASS-METHODS set_data
IMPORTING
is_header TYPE bo_header
it_items TYPE bo_items.
PRIVATE SECTION.
CLASS-DATA: s_header TYPE bo_header,
t_items TYPE bo_items.
ENDCLASS.
CLASS lcl_bo_mock IMPLEMENTATION.
METHOD set_data.
s_header = is_header.
t_items = it_items.
ENDMETHOD.
METHOD /bobf/if_tra_service_manager~retrieve_by_association.
IF iv_association = if_i_status_bo_c=>sc_association-header-items.
et_data = VALUE bo_items( FOR wa IN t_items WHERE ( key = it_key[ 1 ]-key ) ( wa ) ).
ENDIF.
ENDMETHOD.
ENDCLASS.
In this, I have the nodes as local variables. Based on the key, the corresponding items are fetched.
The set_data method can be used inside each unit test to set the data needed just for that unit test. Lets see how this works with 1 unit test method.
CLASS lcl_unit DEFINITION
FOR TESTING
FINAL
DURATION SHORT
RISK LEVEL HARMLESS.
PUBLIC SECTION.
METHODS setup.
methods t_2_completed FOR TESTING.
PRIVATE SECTION.
DATA m_cut TYPE REF TO zcl_unit_test_example_class.
ENDCLASS.
CLASS lcl_unit IMPLEMENTATION.
METHOD setup.
TEST-INJECTION srv_mngr.
o_srv_mngr = new lo_mock_bo( ).
end-test-injection.
CREATE OBJECT m_cut.
ENDMETHOD.
METHOD t_2_completed.
" Check if 2 items in the list is completed
lcl_bo_mock=>set_data( s_header = VALUE #( key = '1' )
t_items = VALUE #( ( key = '2' parent_key = '1' Status = 'Completed' )
( key = '2' parent_key = '1' Status = 'Completed' ) ) ).
data(result) = m_cut->get_completed_items_count( iv_key = '1' ).
cl_abap_unit_assert=>assert_equals( act = result exp = 2 ).
ENDMETHOD.
ENDCLASS.
Now that is a lot of code to write for 1 unit test. But it is a lot of effort for just the first unit test. But the advantages are
- The data set up code, executing the code under test and result verification is in just 1 place inside the test method. Now I will know the input data and expected result in just 1 place. This improves organizing the unit test much better for me.
- From now on, I can simply copy the unit test method and make changes for further variations. That takes hardly a few seconds for each method.
Here I created 2 more methods quickly.
METHOD t_0_completed.
" Check if 0 items in the list is completed
lcl_bo_mock=>set_data( s_header = VALUE #( key = '1' )
t_items = VALUE #( ( key = '2' parent_key = '1' Status = 'Not started' )
( key = '2' parent_key = '1' Status = 'Not started' ) ) ).
data(result) = m_cut->get_completed_items_count( iv_key = '1' ).
cl_abap_unit_assert=>assert_equals( act = result exp = 0 ).
ENDMETHOD.
METHOD t_1_completed.
" Check if 1 items in the list is completed
lcl_bo_mock=>set_data( s_header = VALUE #( key = '1' )
t_items = VALUE #( ( key = '2' parent_key = '1' Status = 'Not started' )
( key = '2' parent_key = '1' Status = 'Completed' ) ) ).
data(result) = m_cut->get_completed_items_count( iv_key = '1' ).
cl_abap_unit_assert=>assert_equals( act = result exp = 1 ).
ENDMETHOD.
This idea can be extended as follows:
- If the class has more than 1 method which need to be unit tested, move the place where service manager is initialized to a method and call this everywhere. So there will be one test seam and test injection only.
- If more methods such as retrieve or convert alternate key etc are required, enhance the mock BO class with those methods.
- The retrieve by association can be extended to support more nodes and input of more than 1 key.
- Maybe move the mock BO class to a global test class that can be used across all the classes that require unit testing of code that reads from this BO.
I know that there is a BOUnit test tool available. But this is how I like to do it. Thoughts? Comments?
Thanks for sharing your experience. Your opening paragraph made me chuckle! Paul Hardy might have a comment or two about TEST-SEAM.
Nice to see more & more people blogging about their experiences with AUnit. Hopefully this will help make others to jump on the bandwagon.
I would however like to add my 2 cents regarding the usage of TEST-SEAM 🙂
Did you consider a (local) backdoor injection?
Hi Suhas,
Thanks for that. I didnt go this way because
As noted I am against TEST-SEAMS as a general concept, as it is like putting IF UNIT_TEST_RUNNING + ABAP_TRUE THEN DO THIS ELSE DO_THAT in your production code, and thus it is not a proper unit test in the true sense of the word.
If the production code knows if it is running under test or not then you can code a Volkswagen test-beating sort of situtation for your pollution emission results.
I am aware this we probaby be seen as an abstract philosopical argument, but than can also be applied to the averison many people feel to using the "friends' concept.
Many people I have talked to do not like the friendship concept in OO as a general thing as it is "cheating" when it comes to making things private or not. Generally I would agree with that but I am prepared to make an exception for unit tests. I also cannot really see the harm in a class that can only ever be produced by a factory class which is its "friend".
With the injector classes being defined as TEST methods then no injection can be done in production.
Lastly I am very interested with your expereinecs with the BOPF unit test framework. ASs far as I can see it was designed to do pretty much the exact thing you seem to be talkiing about i.e. mocking the various helper classes of which you need instances in BOPF.
Did you find it did not work very well, or at all, or that it worked OK but was just too cumbersome to use?
Cheersy Cheers
Paul
... And that has made my week 🙂
Hi Paul,
You are right. TEST-SEAM vs Friend is a philosophical argument and I tilt towards the former.
I didn't use BUnit because, BUnit is used to test the actions, determinations and validations of a BO. Using BUnit, we can mock the IO_MODIFY, IO_READ and other IO_* methods that these actions, determinations and validations need. In my case, I had to consume the BO's data outside the BO. For that, the BO's service manager had to be mocked. BUnit doesn't do that. Hence, building my own method like this.
Cheers!
Hi Harish,
I’m also NOT a friend of the TEST-SEAM because using this you have something in your production coding that is only for testing, but the production coding should have nothing included which is only for testing.
Someone could argue that adding an optional parameter in the constructor and if it is not filled doing the instantiation of a member variable with /bobf/cl_tra_serv_mgr_factory=>get_service_manager( if_i_status_bo_c=>sc_bo_key ) else using the filled importing parameter is also only for testing, but that is not the whole truth. In this case this could also be usable for other consumer which would like to have another service manager. So, in this case no local friend is needed and no additional coding in the production coding is used, which is only for testing.
Without TEST-SEAM the production code is much more clear and readable.
I addition, to my mind only because something would be easier in tests if something is added to productive coding, but this is not needed in the production coding, is no good idea.
The production code should stay clean.
Sometimes it is needed, to get a test running. For example, to create a local class and implement a function call in this local class method. Also, here I would not use the TEST-SEAM which is only for testing. The local class can also be instantiated in the constructor and filled in a member variable. And if in the future the function module is replaced by a class this class can be instantiated in the constructor instead of the local class and so here nothing needs to be changed in the rest of the productive coding.
But if need for get a test running is not the case the productive coding shall be free of coding which is for testing.
By the way your argument to not instantiate the service manager in the constructor because you want the instantiate only if the service manager is needed I do not understand. In your class you always use the service manager if the method is called. Si it is needed in any way.
Now you could say, that this is only an example and there could be more external dependencies like the service manager which are not used in any method. But in this case, I would recommend rethinking the class purpose and if the class is only doing one thing, based on the clean code thinking.
Here is the coding I would use for the same purpose and testing:
Productive class:
Unit test class:
Regards
Hello Andre,
Thanks for stopping by and providing your way of implementing it. I am up for a good discussion any time.
By clean code, the class should have one reason only to change. By that way, the service provider and all other dependencies could be initialized in the constructor itself as the class would consume it anyway. But think of this scenario:
Now you have introduced a parameter in the constructor. Isn't this introduced just for testing?
Good use of the cl_abap_test_double by the way. In this, the output data is defined in a method separately (configure_item_read_call). Here, if there are more number of test cases which require different kinds of output, then that method becomes complicated. And looking at just a unit test I will not be able to figure out the inputs and outputs to the dependent objects. So maybe, this method has to be probably made a little bit generic to accommodate this.
What are your thoughts?
Hello Harish,
you are right the test method to define the test data is only for this test method. If I would have more test methods I would have the item be the input of the data preparation method and so in the test you see which data are used but the ABAP test double coding is hided by calling the method for the configuration.
For the parameter in the constructor. Here you are also right for this case, here the parameter is in the first time only for the test, but a consumer of the productive class has the choice to call it with its own service manager. So, you open the dependency and give the consumer the choice to use another than the default one.
And for the lazy instantiation:
You could also do this by moving the if statement from the constructor to the productive method or even better create a new method that do this and call it from the productive method.
But also here you could have an optional importing parameter in the constructor.
By the way: what is the problem with have the instance and find out in a validation, that you don’t need it?
The normal case will be I think, that the validation not finish the method before the instance can be used, or?
And if the instance comes from a singleton like here for the BOPF service manager it is almost certain that it is already instantiated.
Regards
André
Hi Andre,
In my productive code (in the actual project) I am doing what you say. I have a method that initializes the service manager and returns it. Only this is used by all the other methods.
The thing with having an optional importing parameter is that, it can be used for productive purposes also. That's what Paul Hardy also mentions above with an example to Volkswagen. But if its wrapped in a TEST-SEAM or the global singleton variable is set by a 'friend' unit test class, then this dependency injection will happen only during the test and never in productive cases.
In my case, if we initialize something that is not necessary then I think its a waste of resources. Hence the lazy initialization. Its not too big a deal here since the service manager is not a resource consumer. But if there is a class that consumes a lot of resources upon initialization itself (for whatever reason), then lazy initialization really helps there.
Thanks!
Hi Harish (and readers)
I am on a similar journey 🙂 BOPF is a compley framework and it is not THAT obvious how to have isolated code that can easily be tested.
What do you guys think of the following: At least for some determinations and validations we could probably identify two steps:
1.) Get all required data (i.e. retrieve items by association)
2.) Apply business logic on that data to get desired output (number of not completed items)
Let us be a bit arrogant: Retrieving data by association isn't that hard. We do not really have to unit test this part as we "only" have to provide right keys.
BUT the second step can be a bit complicated. Maybe it is not only about status values but also duration and some statistics to be applied.
So we could think of creating a class + method just for this second step.
And this class / method does not have to call service manager. It simply gets a header and a list of items and returns a value.
Basically I am talking about extracting the code
into a method of another testable class.
Apart testing I see also some other advantages (reusability).
Just an idea...
Regards
Meinrad