Skip to Content

All Blogs in the series

1. ABAP Unit Test Week 1
2. ABAP Unit Test in Odata
3. ABAP Unit Test TDD Implementation
4. ABAP Unit  Test Meets Legacy Code.

 

Finally done with week 3  of Open SAP course where we saw how to add ABAP unit testing in existing legacy code. We already had a great feedback back about Week 3 in this blog by Paul. I thought of applying it in one of near to actual scenario to see how it looks. I have few observations on pair programming which hopefully I will share via separate blog.

Background of Existing Report

This is a 5+ years old report written in a procedural way with all the global data declarations etc. What it does is as follow

  • Check for a user whose name begins with XXX* and valid to date is greater than current date
  • If valid user check number of days between user creation and valid to date, if greater than 60 it modifies the validity date to creation date + 60 days

Awesome Features of current legacy report:)

  • No selection screen
  • 60 days is hardcoded.
  • Username pattern is hard coded

Sample code to change the validity

*   Difference between from and to days
          CALL FUNCTION 'HR_HK_DIFF_BT_2_DATES'
            EXPORTING
              date1                   = dateto
              date2                   = datefrom
              output_format           = '02' " Out put in days
            IMPORTING
              days                    = dayscount
            EXCEPTIONS
              invalid_dates_specified = 1
              OTHERS                  = 2.
          IF sy-subrc <> 0.
            MESSAGE ID sy-msgid TYPE sy-msgty NUMBER sy-msgno
                    WITH sy-msgv1 sy-msgv2 sy-msgv3 sy-msgv4.
          ENDIF.

          IF  dayscount GT c_limit.
            newdate = gw_logond-gltgv + 60 .
            MOVE-CORRESPONDING gw_logond TO datenew.
            datenew-gltgb = newdate.

*  Change the Valid Through date , in case of days of validity is more than 60
        CALL FUNCTION 'BAPI_USER_CHANGE'
              EXPORTING
                username   = usr02-bname
                logondata  = datenew
                logondatax = change
              TABLES
                return     = return.
            IF sy-subrc IS INITIAL.
              CALL FUNCTION 'BAPI_TRANSACTION_COMMIT'.
            ENDIF.
   ENDIF.
ENDIF.

Enhancement Needed.

This concept now needs to be extended to another set of users whose name begin with YYY* and whose validity shall be for 45 days. Easiest and the worst solution will be to add mine YYY* user id and hardcode another 45 days with IF and ELSE stuff. But I thought of why don’t we implement it via unit test class, it will make every life easier, testable and flexible.

Implementing Unit testing in Legacy code

The most important place in the code is where it decides whether the user needs to be modified or not that is where all action will happen. So started by creating our local test class with method ABC with parameters difference between number of days and username.

So started by creating local unit test class LTC_USERVALIDITY with method test method USER_DAYS for testing. This method will pass Username, Number of days between creation and valid to date and creation date to determine whether a user needs to be updated or not. If update is required Y flag is returned along with calculated date.

CLASS ltc_uservalidity DEFINITION FOR TESTING
RISK LEVEL HARMLESS
DURATION SHORT.
  PRIVATE SECTION.
    DATA: m_cut TYPE REF TO lcl_uservalidity.
    METHODS setup.
    METHODS user_days FOR TESTING RAISING cx_static_check.

ENDCLASS.
CLASS ltc_uservalidity IMPLEMENTATION.
  METHOD setup.
    m_cut = NEW lcl_uservalidity( ).
  ENDMETHOD.
  METHOD user_days.

    " 60 Days test all possible combinations for XXX
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'XXX123'
    days   = 60
    startdate ='20180130'
    IMPORTING r_result = DATA(checkvalue) r_newenddate = DATA(enddate) ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'Y' ).
    m_cut->check_userendate_upd_needed( EXPORTING
     userid = 'XXX123'
     days   = 59
     startdate ='20180131'
     IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'N' ).
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'XXX123'
    days   = 61
    startdate ='20180129'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'Y' ).
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'XTZ123'
    days   = 59
    startdate ='20180131'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'N' ).
    " 45 Days test all possible combinations for YYY
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'YYY123'
    days   = 45
    startdate ='20180130'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'Y' ).
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'YYY123'
    days   = 44
    startdate ='20180130'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'N' ).
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'YYY123'
    days   = 48
    startdate ='20180130'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'Y' ).
    m_cut->check_userendate_upd_needed( EXPORTING
    userid = 'XTZ123'
    days   = 49
    startdate ='20180130'
    IMPORTING r_result = checkvalue r_newenddate = enddate ).
    cl_abap_unit_assert=>assert_equals( act = checkvalue
                                        exp = 'N' ).

  ENDMETHOD.

ENDCLASS.

A custom table for mapping username with number of days was also created.

Then defined the actual method CHECK_USERENDATE_UPD_NEEDED to check from custom table whether a user needs to be updated or not.

CLASS lcl_uservalidity DEFINITION
  FINAL
  CREATE PUBLIC .

  PUBLIC SECTION.

    DATA: username TYPE bname,
          days     TYPE i,
          userdays TYPE STANDARD TABLE OF zuserdays.

    METHODS constructor.
    METHODS check_userendate_upd_needed
      IMPORTING
        userid       TYPE xubname
        days         TYPE pea_scrdd
        startdate    TYPE sy-datum
      EXPORTING
        r_result     TYPE string
        r_newenddate TYPE sy-datum .
  PROTECTED SECTION.
  PRIVATE SECTION.

ENDCLASS.

CLASS lcl_uservalidity IMPLEMENTATION.

  METHOD constructor.
    " Master data for days for user name
    SELECT * FROM zuserdays INTO TABLE userdays.
  ENDMETHOD.


  METHOD check_userendate_upd_needed.
    DATA:ls_userdays TYPE zuserdays,
         uname       TYPE bname.
    uname = userid+0(3).
    READ TABLE userdays INTO ls_userdays WITH KEY username = uname .
    IF sy-subrc EQ 0.
      IF ls_userdays-zdays > days.
        r_result = 'N'.
      ELSE.
        r_result = 'Y'.
        r_newenddate = startdate + ls_userdays-zdays.
      ENDIF.
    ELSE.
      r_result = 'N'.
    ENDIF.
  ENDMETHOD.

ENDCLASS.

Added the selection screen parameter for user type

Finally removed the hardcoded part added our user date extender determiner plugin:)

*          Hardcoded part commented added our plugin which determine if date needs to
*          to be changed or not if yes then calculate also.

*          IF  gv_count GT c_limit.
*            gv_newdate = gw_logond-gltgv + 60 .
       
          uservalidity->check_userendate_upd_needed(  EXPORTING
          userid = gw_bname-bname
          days   = gv_count
          startdate = gv_fromdate
          IMPORTING r_result = DATA(checkvalue) r_newenddate = DATA(enddate) ).
          IF checkvalue = 'Y'.
            MOVE-CORRESPONDING gw_logond TO gw_newdate.
            gw_newdate-gltgb = enddate.
*             gw_newdate-gltgb = gV_newdate.

It worked flawlessly.

Benefits

  • Clearly any change in report done does not matter by whom he/she shall be able to validate that at least this part of the code is not impacted by his/her change.
  • Once the ABAP unit framework is set normally people tend to follow it, so hopefully this program will eventually become better with each iteration.
  • I came to know about the concept of Test Seam introduced in latest ABAP version, for more detail read this blog by Klaus Ziegler

 

Challenges or Open Points

  • One of the important thing which I am still thinking is how do I add flexibility to my code not only in terms of username and days, but also if a new criteria needs to be added later for example user type? Some kind of a similar abstraction concept needs to be bring in, will share as soon as I have got it implemented.
  • Reluctance to write so much code for just another if and else

 

What next from here?

  • Some thing about Test Seam
  • ABAP Test Double framework.

 

Feel free to provide your feedback open to all ears.

To report this post you need to login first.

14 Comments

You must be Logged on to comment or reply to a post.

  1. Justin Loranger

    Just curious, why did you put all the tests into one method?  If one fails, you wouldn’ necessarily know which one.

    Also, your new class is a local class to the report?  Where does the test class go?  A separate include?

    Thanks for the example.

    Justin

     

    (2) 
    1. Nabheet Madan Post author

      Hello

      Thanks for the feedback.

      Test class goes as local include. Logically created one method as they are related tests,  we can create different methods for logically different criterias. The ouput when you run shows the line number where it failed.

      Thanks

      Nabheet

      (1) 
  2. Mark Wagener

    Hi,

    firstly nice that you took the effort to make your own example and to write about it!

    Regarding the test method, I thought the same like Justin. Why didn’t you create separate methods? Don’t get me wrong, I really appreciate that you went the extra mile, therefore I want to provide helpful feedback (even though it might seem picky…).

    It would be nice to see your tests go wrong (red) once and what you changed about it, also a screenshot of all tests being green would be nice…just to put some more attention on the ATC part and to show your experiences with it.

    Just for the bigger picture, would it we possible to share the complete report with test class?

     

    Cheers,

    Mark

    (1) 
    1. Nabheet Madan Post author

      Thanks Mark for the feedback. Since kind of tests to be run is more or less same that is why. OOPs let me add screen shots sorry missed it…

      (1) 
  3. Alexander Geppart

    Hello Nabheet,

    thanks for sharing.

    Actually I have some old test methods that looks similar to yours user_days method, but

    as told in open sap wtc course, it should be easier to write a new test (a new test variant) compared to existing ones. With given test method it would be hard to do that as this test method would grow with each new test variant.

    From my point of view, there are two methods to improve your/our test code.

    First one would be to follow advices from Justin Loranger and Mark Wagener and create different test methods (and even different test classes).
    In your example that could be like this:

    ltc_user_xxx_upd_needed_cause (test class)
    – days_eq_60 (test method)
    – days_eq_61 (test method)

    ltc_user_xxx_no_upd_needed_cause (test class)
    – days_eq_59 (test method)
    – user_name_eq_XTZ123 (test method)

    ltc_user_yyy_upd_needed_cause (test class)
    – days_eq_45 (test method)
    -…

    The other option would be to group similar test methods.
    In your example that could be like:

    ltc_user_validity_check_if (test class)
    – update_is_needed (test method)
    – no_update_is_needed (test method)

     

    best regards,

    Alex

    (4) 
    1. Nabheet Madan Post author

      Thanks Alexander for the great feedback. Grouping similar test methods logically is something i can relate to.  This is the reason for sharing the experience to get 360 degree view.

      Thanks once again

      (0) 
  4. Joachim Rees

    Hey Nabheet,

    thanks for sharing!

    On
    > I am still thinking is how do I add flexibility to my code not only in terms of username and days, but also if a new criteria needs to be added later for example user type?

    I would say: don’t worry about possible future use-cases now – they might never come! If you get a new requirement, e.g. with the new criteria, then is the time to think about how to implement it!

    best
    Joachim

    (1) 
    1. Uwe Fetzer

      I’m with you here -> don’t change the code (yet), but thinking is never a bad thing. Put your thoughts for you colleges (or for your future self) as comment in the code, for example “for more flexible checks -> use BRF+”.

      Cheers, Uwe

       

      (3) 
  5. Shai Sinai

    Thanks for sharing.

    Just two comments:
    Since your business logic/decision is based on DB table:
    1. Isn’t using all these hard-coded checks w/o a test seam (or mockup data) quite useless?
    (Although it seems you are going to talk about test seams on next part).
    2. Is there any point in checking both XXX and YYY users values? (Currently there are only two records, but I guess you aren’t going to cover any data record which would be added in the future?)

    (1) 
    1. Nabheet Madan Post author

      Thanks Shai for the feedback. Please find below my response.

      1. Yes Improvements based on test seam is next on card
      2. Yes since as of now the scope is limited to these test cases.

       

      Thanks

      Nabheet

      (0) 

Leave a Reply