At the end of the of this series I left you hanging just when I was ready to really start refactoring. Sorry for that and shouldn’t let you no longer. So, here we go.
Refactoring can start
But where to start? Well, let’s list the things that could be cleaned up. In this I use the following rules of thumb:
- If data creation helper functions are referenced multiple times from different tests, these might be good candidates to be moved in to the Initialize function. In this way we promote fresh fixture into shared fixture.
Note that data creator helper functions are created based on the [GIVEN] clauses - If different helper functions contain almost the same code, these are candidates for generalization: from a number of specific methods move the code into one generic method.
In the context of this blog post series we’ll focus on my first rule of thumb only.
So, what data creation helper methods in my test codeunit are referenced multiple times?
Have a look:
Having 5 tests coded and having two of the data creation helper functions referenced 5 times it’s quite obvious that these are called by each test function (and which we can easily check in the code). Sounds like real good candidates for a promotion to the Initialize function. If the concept of the Initialize function does not sound familiar to you read more about it in my book or in the .
// [GIVEN] Location with require shipment
I am first going to focus on the helper function CreateLocationWithRequireShipment for the very reason that it is applied in each of the 5 test functions in exactly the same way:
// [GIVEN] Location with require shipment
LocationCode := CreateLocationWithRequireShipment();With LocationCode being a local variable of type Code[10].
Moving this statement into Initialize means that not each test will create a new location but instead will only be created once. This is not only has a refactoring result – elimination of duplication, or clean-up – but it also makes running all 5 tests more efficient as the location only needs to be created once; as we will see below.
Keep in mind: taking small steps. Let’s not refactor all 5 tests in one go but one by one running the tests after each (small) change.
Before – [SCENARIO #0001]
[Test]
procedure DeleteByUserWithNoAllowanceManuallyCreatedWhseShptLine()
// [FEATURE] Unblock Deletion of Whse. Shpt. Line enabled
var
LocationCode: Code[10];
...
begin
// [SCENARIO #0001] Delete by user with no allowance manually created whse. shpt. line
Initialize();
...
// [GIVEN] Location with require shipment
LocationCode := CreateLocationWithRequireShipment();
...
end;The three dots (…) are used to replace other codes parts that are not in focus right now.
After – [SCENARIO #0001]
[Test]
procedure DeleteByUserWithNoAllowanceManuallyCreatedWhseShptLine()
// [FEATURE] Unblock Deletion of Whse. Shpt. Line enabled
var
...
begin
// [SCENARIO #0001] Delete by user with no allowance manually created whse. shpt. line
// [GIVEN] Location with require shipment
Initialize();
...
end;Before – Initialize
local procedure Initialize()
begin
if IsInitialized then
exit;
IsInitialized := true;
Commit();
end;The make the focus on our work here better I have simplified the setup of Initialize here compared to the one you will find in the GitHub repo.
After – Initialize
var
LocationCode: Code[10];
local procedure Initialize()
begin
if IsInitialized then
exit;
// [GIVEN] Location with require shipment
LocationCode := CreateLocationWithRequireShipment();
IsInitialized := true;
Commit();
end;Now that LocationCode is assigned a value in Initialize and is used in the first test function, it needs to be declared as a global variable as can been seen above.
Exercising the same refactoring step for the other four tests – refactoring and retesting one by one – will give the following final test run result:
Notes
- Refactoring any of the next test functions is with a smaller step than for the first test function as I do not need to update the Initialize function.
- After each rerun the duration of that test diminishes… except for the first test, because with that test Initialize is fully executed.
- Running each test individually will make it duration again just as long as before, because in the case the location needs to be created.
- We have to realize that we can “promote” CreateLocationWithRequireShipment due to the fact that the location is not consumed, being that, it indeed is reusable in all those five tests.
// [GIVEN] Enable “Unblock Deletion of Shpt. Line” on warehouse setup
What next? Well, the other data creator EnableUnblockDeletionOfShptLineOnWarehouseSetup.This is called in exactly the same way in the first four tests. These have all a similar setup/purpose, so, we could most probably move this into the Initialize function.
Before – [SCENARIO #0001]
[Test]
procedure DeleteByUserWithNoAllowanceManuallyCreatedWhseShptLine()
// [FEATURE] Unblock Deletion of Whse. Shpt. Line enabled
var
...
begin
// [SCENARIO #0001] Delete by user with no allowance manually created whse. shpt. line
// [GIVEN] Location with require shipment
Initialize();
...
// [GIVEN] Enable "Unblock Deletion of Shpt. Line" on warehouse setup
EnableUnblockDeletionOfShptLineOnWarehouseSetup();
...
end;The three dots (…) are used to replace other codes parts that are not in focus right now.
After – [SCENARIO #0001]
[Test]
procedure DeleteByUserWithNoAllowanceManuallyCreatedWhseShptLine()
// [FEATURE] Unblock Deletion of Whse. Shpt. Line enabled
var
...
begin
// [SCENARIO #0001] Delete by user with no allowance manually created whse. shpt. line
// [GIVEN] Enable "Unblock Deletion of Shpt. Line" on warehouse setup
// [GIVEN] Location with require shipment
Initialize();
...
end;Before – Initialize
local procedure Initialize()
begin
if IsInitialized then
exit;
// [GIVEN] Location with require shipment
LocationCode := CreateLocationWithRequireShipment();
IsInitialized := true;
Commit();
end;Note that the setup of Initialize here is somewhat simpler than you will find in the GitHub repo.
After – Initialize
local procedure Initialize()
begin
if IsInitialized then
exit;
// [GIVEN] Enable "Unblock Deletion of Shpt. Line" on warehouse setup
EnableUnblockDeletionOfShptLineOnWarehouseSetup();
// [GIVEN] Location with require shipment
LocationCode := CreateLocationWithRequireShipment();
IsInitialized := true;
Commit();
end;But what about the fifth test in which EnableUnblockDeletionOfShptLineOnWarehouseSetup is not used in the [GIVEN] section, rather in the [WHEN]? If we would add this helper function to Initialize will it be triggered uselessly? As matter of fact, not really, as it is already triggered in each preceding test anyway.
Let’s see what the overall effect is on the test duration:
Notes
- Having EnableUnblockDeletionOfShptLineOnWarehouseSetup executed uselessly
- Of course, as we all know each process, like a test run in our case, is not executing in exactly the same duration each time. The above screenshot is just one example of a test run.
- The duration shortening due to the promotion of EnableUnblockDeletionOfShptLineOnWarehouseSetup to Initialize is nothing compared to the promotion of CreateLocationWithRequireShipment .
