Actual TDD in Real Life

The claim is that nobody ever does Test Driven Development in real ABAP life so I have volunteered to give it a go in my day job.

Unbreakable

I have never ever done TDD outside blogs. It works really well, but experimental programs are not real, and not what I am paid to do. I do these Monster related programs in my spare time and then blog about them to prove some sort of point, whatever I am passionate about at the time.

Jelena raised some very real concerns about in her blog:-

https://blogs.sap.com/2018/03/23/tdd-electrolytes-and-double-boiler/

The pertinent point is that all the examples are trivial in virtually every article you can read in a book or on the internet. What is required to get this Monster to be coming over the ABAP hill?

What are needed are better examples clearly – this is what happens at SAP conferences. The most successful presentations are not the ones where SAP personnel (I am sorry HUMAN RESOURCES) get on stage and say how great the company that employs them is.

The presentations people really want to see are from other companies that have implemented whatever SAP thing they are thinking of implementing and want to hear what went wrong and how difficult it was and did it really work in the end. The answers usually are that loads of things went wrong and it was much more difficult than expected but that it DID work in the end. If it did not the speaker would not be presenting at an SAP event!

So I said in a comment at the end of this blog:-

https://blogs.sap.com/2018/03/24/open-sap-course-unit-testing-week-two-tdd/

• that I would do this for real at work on my very next task at work i.e. putting my (salary) money where my big fat mouth is.

The application I am working on resembles VA01 and so cannot be accused of being unrealistic in the way that Monster examples or SFLIGHT examples or “money machine” examples can be accused of not being relevant to the daily life of an ABAP developer.

I imagine the vast majority of organisations use VA01 and the developers are familiar with the procedural monster which is SAPMV45A.

I together with colleagues am currently rewriting a procedural application that “wraps” the sales order BAPI in an OO way. So the end result will be virtually 100% different code wise, only the business logic will be exactly the same as the legacy application (legacy as in it has no tests).

Culture Shock

NB This is only possible due to corporate culture. In my company top management look to the future, and that filters down. Many companies only look to the next quarter, and if the CEO only cares about the next quarter it is likely the CIO will be focussed on how much IT stuff can be delivered in the next quarter and the future can look after itself. If squirrels thought like that they would die out as a race, but luckily they are smart enough to plan for the long term. So why cannot most CEOs of large companies be as smart as squirrels? Maybe it is because the stock market analysts force them to behave that way, constantly focussing on short term results. The Squirrels are laughing at as humans for being so short sighted.

Analyse This

What I am currently engaged in is to analyse the existing code, one screen at a time i.e. do not try and boil the ocean. My initial approach is one test class per screen. A DYNPRO screen could be described as a “process” in that it has a defined purpose i.e. to capture data and do something with it for whatever purpose.

The purpose of the initial screen in VA01 is to capture the document type and a bunch of organisational information and then forward that to the main screen.

I look through every line of the “legacy” screen and make a list of everything the screen currently does (filtering out anything obviously insane). If you do not think there is anything insane lurking in your ten to twenty year old ABAP applications then you are in for a shock.

Once that code has been analysed you end up with what is called is the IT SHOULD list, a concept invented by Dan North. Dan, Dan the Unit Testing Man. Dan Dares, TDD Pilot of the Future.

What do I mean by that? (The IT SHOULD list, not the silly jokes) “IT” is the equivalent of what the program should do which was presumably was specified in the original specification by the business analyst (if there was such a specification and not just a post-it note). David Bowie invented post-it notes by the way, in the same way “The Monkees” invented TIPEX and that Gerry Rafferty invented the Self Advancing Date Stamp. Another fun fact is that UK “Bullseye” Game Show Host Jim Bowen played the saxophone on Gerry Rafferty’s “Baker Street” as well.

Will you just stop messing around on focus on the problem at hand?

This is the first problem – I am going to concentrate on the first process – the “initial screen” which gets processed before any actual business object is created.

I am not doing this 100% properly as I had created a skeleton framework before I decided to start on the TDD experiment, but the skeleton does not actually do anything as yet, so the tests are designed to force creation of the production code to put the flesh on the bones. As will be seen TDD will make me realise how illogically my skeleton was put together, with the leg bone sticking out of the skull and so forth.

Therefore the initial screen methods are all static, as no instance of the business object has been created as yet, so at first glance seem impossible to test. Let us make the list anyway, and then try to change the design so it is testable.

Your Kiss is on my IT SHOULD list

The IT SHOULD LIST then gets arranged by me into several categories:-

Derivations – default certain values based on business logic or the database

Validation – Missing Data – Prompt User to enter mandatory values

Validation – Dubious Data – Prompt User for Confirmation

Validation – Incorrect Data – Raise Error, Inform User

These are based on the BOPF programming model. At this point the only “action” (user command) is pressing the ENTER key so I am not going to bother with that at this time,

So I range the IT SHOULD list for the initial screen in the order above, and then further sort the list by putting the most important ones at the top of each category.

The tests to be written first (according to Dan North) as based on one criteria which is – what is the most important thing your program DOES NOT currently do?

To try and make it real here is my IT SHOULD list. The initial screen is where the user enters the order type and various organisational elements, just like VA01.

IT SHOULD…..

– derive the initial values of the organisational fields based on the PIDs of the user

– derive the text descriptions of the organisational fields

– force the user to enter a sales group for countries where this is mandatory

– prompt the user to confirm if service division is entered

– stop unauthorised users creating sales orders

– stop the user entering a document type which does not relate to a sales order

– stop the user entering an order type which has been blocked

– stop the user entering an invalid combination of organisational elements

– stop the user entering an order type not allowed to be used with the organisational elements (per customising)

Let’s Go Unit Testing Crazy!

So now I have solved the insoluble problem of knowing what to test. It is very tempting to write all the test outlines at once, but that is not the way it is supposed to work – you are supposed to do one at a time. Amazingly the idea is not to think ahead too far e.g. try and cater for all sorts of unusual situations or future requirements, neither of which may ever happen. This is the YAGNI approach (You Ain’t Gonna Need It).

Next problem is the thirty character limit on method names. I get around that my putting a big comment before each method name.

CLASS ltc_initial_screen DEFINITION FOR TESTING
RISK LEVEL HARMLESS
DURATION SHORT
FINAL.

PUBLIC SECTION.

PRIVATE SECTION.
DATA: mo_cut TYPE REF TO zcl_sd_sofe_application.

METHODS: setup,
*——————————————————————–*
* Specifications : IT is the Initial Screen
*——————————————————————–*
“IT SHOULD…………………
“derive the initial values of the organisational fields based on the PIDs of the user
derive_values_from_pids FOR TESTING.

ENDCLASS.                    “ltc_initial_screen DEFINITION

OK for the moment we ignore all the other requirements and do not rest until we get the very first test to pass. In fact it would pass at the moment because it is empty, so we need to flesh out the code a bit with the GIVEN / WHEN / THEN pattern.

This is taking a lot longer than the fifteen minute cycle described in the course, and it should not be this hard as I am supposed to have half an idea of what I am doing!

Look how much code there is already:-

METHOD derive_values_from_pids.

given_user_with_uk_pids( ).
when_screen_is_shown( ).
then_org_values_match_pids( ).

ENDMETHOD.

METHOD given_user_with_uk_pids.

ENDMETHOD.

METHOD when_screen_is_shown.

ENDMETHOD.

METHOD then_org_values_match_pids.

cl_abap_unit_assert=>assert_equals( exp = ‘XXXX’ “Desired Result
act = ms_initial_screen_fields-vkorg
msg = ‘Sales Organisation is Incorrect’ ).

ENDMETHOD.

The amount of test code will only get bigger and the irony is the actual production code is three or four lines all starting with GET PARAMETER ID.  If it was only one line I would test it anyway! That’s madness! When Batman goes to Arkham Asylum does he find the padded cells full of TDD developers?

In any event I have now got the stage where my test is complete enough to fail! I press CTRL + SHIFT + F10 (or take the menu option (Local Test Classes -> Unit Test) and I get a red light and a message telling me what assertion has failed. Just as an aside as I am doing this for real, I have to work on a system where ABAP in Eclipse is not yet available, though it will be in a month or so when it gets upgraded from 7.02 to 750.

The THEN part was easy enough, but I immediately get into trouble with the WHEN part. There is literally nothing I can call in my existing code which does just what I want, which means I have my methods doing too many disparate things. In this case the initial design was a series of calls to methods of helper objects. This was clearly mixing up the WHAT was going on with the HOW i.e. the implementation details i.e. what helper object was doing what.

This is an unexpected side effect to the TDD process – you are forced into an OO design. Actually I imagine that is what some people are going to hate about it, along the “not everything can be test doubled” lines.

In any event I have come to the conclusion that the changes I was forced to make to my class to make it testable improved the design. It helps it is a brand new class, but nonetheless it would help with existing classes or even forcing classes to be used in existing procedural code (which is not that difficult).

Test Double or Nothing

One thing you realise in a hurry is that CREATE OBJECT commands are poison to the unit test process. You always have to have the actual instances injected into the code where they will be used, or created by some sort of factory, thus enabling them to be test doubled.

In this case I had the actual instance of a view created a few lines before the DISPLAY method was called, and thus there was no way to test double the UI. I need a test double view instance because I do not actually want a screen to appear, and I want to fake the user input. I will need a fair few other test double classes as well, but the rule is to write the least amount of code that will make the test pass, so we do one thing at a time, and right now the one thing is creating a test double UI subclass, inheriting from the real one.

Then I attempt to inject the test double class as a helper object into the real class. That took a lot of debugging and re-organisation to get that to work properly without dumping.

Still, even though I only have one test so far, during the “red” cycle when I am changing things to get the one test to pass, I soon found a totally unrelated problem in my logic, a problem I thought would never occur, but it occurred for the test, and it would have occurred in real life sooner or later. To be brief because a field was mandatory on the SAP GUI I had presumed you could not return from the view without that field being filled. However what if we had a different UI technology, or no UI technology at all and were getting called via a BAPI or something? Then the field might possibly be blank and the program would have dumped.

In essence it took me while a while to get from the test being red because I had not written anything, to the test being red because of problems with the test double UI object being injected, and unrelated problems with the existing code, which as I said, I had considered trivial. This proves what I said earlier, as soon as you have more than a few lines of code, maybe even one line of code, you have the potential for bugs. This process shines a spotlight upon them.

Anyway the first stage was to have a test double object for the UI where all the display method does is have the user press ENTER. I now need a way to fake the PID values being read and updating the data structure.

The eagle eyed amongst you will notice I am faking everything that is happening, so what exactly is it I am testing? Simply that a routine is called that reads the PIDs during processing of the screen. That is something the program should be doing (according to the requirements I drew up earlier, the IT SHOULD list) so I am making sure it actually does it.

Once again I find I had put the code to get the parameter IDs directly in the model, this needs to be abstracted to a persistency layer helper object, so it can be faked. It was not too difficult extracting the methods (with eclipse it would be even easier) though with everything on the initial screen being static, the injection process is really painful.

I set the test double derivation to return a hard coded value of VKORG equivalent to what the test is looking for. In the setup I inject the test double persistency layer, and magically the test goes green.

My Blue Heaven

The next stage is the blue “refactor” stage where once things are working you can make the code better and be sure you have not broken anything, because if you have then the test(s) will turn red again.

Making things “better” is of course highly subjective. I tend to equate “better” with “simpler” as in can I make the code clearer either by renaming something to make it more obvious, or reducing the lines of code – note if that comes at the cost of clarity then I might even favour increasing the lines rather than reducing them. Some of the new ABAP construct do not always make code clearer, sometimes quite the reverse.

Consider this example from the course instructors:-

m_cash_provider = COND #(

WHEN i_cash_provider IS BOUND

THEN i_cash_provider

ELSE NEW cl_cash_provider( ) ).

Is that clearer or more obscure than the following:-

IF i_cash_provide IS BOUND.

m_cash_provider = i_cash_provider.

ELSE.

CREATE OBJECT m_cash_provider TYPE cl_cash_provider.

ENDIF.

Or are both equally as clear and the new construct is being used just to prove there is a new construct?

Going back to the “refactor” stage, in this case since the test double is sending back a hard coded value, and I do not like hard coded values, and moreover I have a “give” method with no code inside it, I think I will use the “given” method to instruct the test double what values to return. After my forthcoming upgrade I could use CL_ABAP_TESTDOUBLE to do this, at the moment on 7.02 I have to do this manually.

I will give my test double persistency layer an internal table of users and PID values based on USR05 which is where they live at session start up. Then because my test double can have extra methods in the GIVEN method I will call methods to set the current user and add the PID values to the internal table.

METHOD given_user_with_uk_pids.
* Local Variables
DATA: ls_pid_values TYPE usr05.

mo_test double_pers_layer->set_current_user( ‘BLOGGSJ’ ).

ls_pid_values-bname = ‘BLOGGSJ’.
ls_pid_values-parid = ‘VKO’.
ls_pid_values-parva = ‘GB30’.
mo_test double_pers_layer->add_pid_line( ls_pid_values ).

ENDMETHOD.                    “given_user_with_uk_pids

I needed to fiddle with some definitions in the test method to get that to compile without a syntax error.  Afterward, the test still shows a green light, so I know I have not broken anything.

So one unit test done which does not really prove anything except that a certain method gets called in the production code at the correct point. Also that took a lot longer than fifteen minutes, more like one complete working day all up.

However that is probably what you might expect the very first time you do anything properly, as opposed to playing around with half-baked experiments, or doing SFLIGHT examples, or testing that one and one added together really does return two (the most common example you find on the internet).

That was HORRIBLE – let’s do it again!

The acid test (if you forgive the pun) comes next. Will adding the next test be any easier than the first? It is testing a similar sort of thing (that a method is called) so maybe things will go a lot faster this time.

Creating the new test method and the GIVEN / WHEN / THEN methods was really fast, as they are all blank to start off with. I add my assertion so that the test fails.

So I get to the red stage within a few minutes. As an aside someone at SAP loves having things appear in alphabetical order rather than in a logical order. The test methods appear in alphabetical order run ABAP Unit shows the result, and of course in SE80 in the object list the methods appear in alphabetical order. You may find this difficult to believe what in the initial version of the “persistent object” framework you had to put the fields in your structures in alphabetical order!

Anyway I would prefer to see the test in logical order rather than alphabetical order, but there you go, what cannot be cured must be endured.

Since the WHEN is the same (when the initial screen is shown) it is a question of filling in the GIVEN method. I add two similar methods to set data, methods which do not yet exist. With Eclipse I could then generate the methods automatically, without it I manually create the definitions and implementations.

I also have to write almost twice as many lines of code due to not being able to use inline declarations yet.

Nonetheless total elapsed time for adding the second text and getting it to the green state was about twenty minutes, not far over what the coures instructors wanted. In this case no refactoring was needed.

Nether Regions

As an aside, in the ABAP editor you have the “Region / End Region” comments you can make around blocks of code, which can then be expanded or compressed, as in the below screenshot:-

Expanded

Compressed

RAISE EXCEPTION – It’s Breaking up the Nation

My next test will be more complicated as it involves making sure an exception is raised when a field is not filled in under certain circumstances. The exception class in question does not even exist yet.

I can still write the test though enough so it fails:-

METHOD then_missing_field_error.

cl_abap_unit_assert=>assert_equals( exp = abap_true
act = mf_missing_field_error
msg = ‘Mandatory VKGRP Not Entered’ ).

ENDMETHOD.

I need to create the exception, then CATCH it in my test method that processes the screen, and setting a flag if the exception is caught. The test will still fail because the production code does not throw the exception. The last stage will be changing the production code.

That leads me on to the fact that the production code I supposed to handle the exception itself – it should catch the exception and then tell the view what field is missing and get the view to force the user to either enter the missing data or CANCEL which terminates processing,

That is yet another problem which forces a redesign – in this case the error handling is inside the CATCH statement in the public method being tested. In order to propagate the exception I have to put the error handling in its own method, so it can be test doubled. In real life, it just continues so the screen pops up again, the test double just propagates the exception so it can be caught by the test framework. Oddly enough that is the opposite of the usual situation where the real class does something and the test double one does not.

Does that sound crazy? Instead of having all the code to handle the exception just below the CATCH statement instead to just have a one line call to a method that does the exception handling logic? Maybe it is, but it is just what good old “Uncle Bob” advocates – he says methods should do just one thing, and error handling is one thing, and so the purpose of the method and the error handling for the method should be separated. That suits me just fine in this situation.

I could continue with the next few tests, but by this point I have pointed out the general idea.

Conclusion

The aim of the game was to do TDD for real instead of silly examples that have no relevance to real ABAP developers.

So I did it at work, on an application that virtually all ABAP developers can relate to, except maybe those who work for organisations that do not sell anything, or even provide services for free like some charities.

My conclusion is that this is a lot of hard work at the start, but gets easier once you are over the initial learning curve. The positive thing is that you can feel your program getting better and more rock solid hour by hour, just as advertised.

Due to the fact I am allowed to do this sort of thing I can honestly say I foresee a future where changes to one part of a huge program do not break functionality in another part. That has never been the case before.

Furthermore Robert Martin’s quote “QA should find NOTHING” by which he means no bugs once the code goes to the test system, is also possible. Which is not to say they should not test it anyway.

As might be imagined, I am biased from the start, and even though this process was a lot harder in real life than in silly Monster blogs, I still recommend it wholeheartedly.

Cheersy Cheers

Paul