After I published my last post, some folks rightly pointed out that the way I implemented the add_popular_sith procedure was not well-testable and that I should rather change my code than trying to control the underlying sequence.
And I totally agree. For several reasons:
- Controlling the sequence adds a lot of complexity to my testing code
- It invalidates depending packages (and yes, there are ways around that problem)
- It also forces me to touch internals. That means when I change the sequence (even if it’s the sequence name) my test will fail even though the procedure still works. Same happens if I replace the sequence with an identity column.
The whole test-setup around controlling the sequence is a “smell” that shows me one thing: I have functionality that is hard to test (and therefore probably also hard to change and maintain).
Here’s our current implementation:
create table popular_sith (
id integer primary key not null,
name varchar2(200)
);
create sequence popular_sith_seq start with 1;
create or replace package sith_manager as
procedure add_popular_sith(
i_name varchar2 );
end;
/
create or replace package body sith_manager as
procedure add_popular_sith(
i_name varchar2 )
as
begin
insert into popular_sith (id, name )
values (
popular_sith_seq.nextval,
nvl(i_name, 'Darth Ora')
);
end;
end;
/
Highlighted line 21 is what makes this code hard to test, because the sequence’s nextval is indirect input that we cannot control easily.
Let’s change that in 3 small iterations:
Iteration 1: Add a possibility for direct input
The test that required us to reset the sequence was described like that:
create or replace package ut_sith_manager as
-- %suite(Sith Manager)
-- %suitepath(darkSide.management)
-- %test(ADD_POPULAR_SITH uses default name on NULL param)
procedure add_sith_default_on_null_param;
end;
/
Notice that the thing we want to test here is the behavior of the add_popular_sith method when we pass NULL into the name param.
create or replace package body ut_sith_manager as
procedure add_sith_default_on_null_param
as
l_actual_row popular_sith%rowtype;
begin
-- Act
sith_manager.add_popular_sith(
i_name => null,
i_id => -1);
-- Assert
select * into l_actual_row
from popular_sith
where id = -1;
ut.expect(l_actual_row.name).to_equal('Darth Ora');
end;
end;
/
This test currently fails, because we don’t have a second parameter id in our procedure. But if we could control the ID that way, it would be a pretty easy and straight forward test, right? So let’s change our implementation a tiny little bit:
create or replace package sith_manager as
procedure add_popular_sith(
i_name varchar2,
i_id integer default null);
end;
/
create or replace package body sith_manager as
procedure add_popular_sith(
i_name varchar2,
i_id integer default null)
as
begin
insert into popular_sith (id, name )
values (
nvl(i_id, popular_sith_seq.nextval),
nvl(i_name, 'Darth Ora')
);
end;
end;
/
We have a successful test now that checks the expected behavior of our default name mechanic.
Iteration 2: Safeguard the ID
“But wait!” you might say. “How can we be sure now that our sequence even works?”
I completely agree: We can’t at the moment. But I don’t consider that a bad thing. The way we test the default-name-behavior now highlights something that was hidden before:
We have two essential behaviors in this method: Default-name behavior and automatic provisioning of a primary key.
The need to have a primary key set automatically always existed, but it was hidden – and implicitly safeguarded by our old “control-the-sequence”-test.
We can safeguard this behavior in a much more precise and explicit way now:
create or replace package ut_sith_manager as
-- %suite(Sith Manager)
-- %suitepath(darkSide.management)
-- %test(ADD_POPULAR_SITH uses default name on NULL param)
procedure add_sith_default_on_null_param;
-- %test(ADD_POPULAR_SITH generates ID when not provided)
procedure add_sith_generate_id_on_null;
end;
/
create or replace package body ut_sith_manager as
[...]
procedure add_sith_generate_id_on_null
as
l_actual_row popular_sith%rowtype;
l_name varchar2(100) := 'My special test Sith';
begin
-- Act
sith_manager.add_popular_sith(l_name);
-- Assert
select * into l_actual_row
from popular_sith
where name = l_name;
ut.expect(l_actual_row.id).not_to_be_null();
end;
end;
/
Two tests for two behaviors – if one of them breaks, we will know exactly which one and will not waste unnecessary time.
We also don’t touch internals: Our new test will work no matter how the underlying sequence is named or if we replace it with an identity column.
Iteration 3: But my public API!
“But I don’t want that anyone can provide their own ID! This will create a huge mess!”
Although this might not be a problem in a lot of circumstances I can absolutely understand that argument. We opened-up our public API for testing purposes – something we might not want.
Therefore, we add another tiny change (if we are on Oracle 12.2 or above):
create or replace package sith_manager as
procedure add_popular_sith(
i_name varchar2);
procedure add_popular_sith(
i_name varchar2,
i_id integer)
accessible by (package ut_sith_manager);
end;
/
create or replace package body sith_manager as
procedure add_popular_sith(
i_name varchar2)
as
begin
add_popular_sith(i_name, null);
end;
procedure add_popular_sith(
i_name varchar2,
i_id integer)
accessible by (package ut_sith_manager)
as
begin
insert into popular_sith (id, name )
values (
nvl(i_id, popular_sith_seq.nextval),
nvl(i_name, 'Darth Ora')
);
end;
end;
/
Instead of a second id parameter with default null, we now use method overloading and have one method with just the name and another method with name and id parameter.
This is basically the same as before, but now we can restrict access to the second procedure – where we have an id parameter – to only our test package.
If we try to call sith_manager.add_popular_sith('Darth Vader', 42) from anywhere else but our ut_sith_manager package, we’ll get
ORA-06553: PLS-904: insufficient privilege to access object ADD_POPULAR_SITH
For those who wonder: The accessible by clause doesn’t require the mentioned objects to exist, so we didn’t introduce coupling to our test code (which we don’t want to ship to production).
Philipp Salvisberg taught me about this powerful feature that comes in very handy in unit-testing situations.
Conclusion
There are a bunch of things that we achieved by only three small changes:
- We eliminated a cause for package invalidation
- We significantly reduced complexity of our test code
- We made the behavior of
add_popular_sithmore visible - We improved the stability and value of our tests by only testing the behavior and not the internals
- We did all of this without ever changing the public API of our existent
add_popular_sithmethod.
As always, you can find the full example on my github repository.
The next time you run into code that is hard to test, hold for a second and think about whether this could be a “smell” that you can improve by changing your implementation.
Bonus Iteration: Avoid Unnecessary Sequence Increment
Grandmaster of PL/SQL Steven Feuerstein pointed out that by using nvl I was increasing the sequence unnecessarily, even when providing an ID. This is because nvl always evaluates both sides of the comparison and does not stop when the first argument is already not null.
He is right, but his suggestion to replace it with case didn’t work either.
No matter what I did inside the insert statement – the sequence got increased every time until I separated it from the insert statement:
create or replace package body sith_manager as
procedure add_popular_sith(
i_name varchar2)
as
begin
add_popular_sith(i_name, null);
end;
procedure add_popular_sith(
i_name varchar2,
i_id integer)
accessible by (package ut_sith_manager)
as
l_id integer := coalesce(i_id, popular_sith_seq.nextval);
begin
insert into popular_sith (id, name )
values (
l_id,
nvl(i_name, 'Darth Ora')
);
end;
end;
/
We can prove that the sequence doesn’t increase anymore now:
select popular_sith_seq.nextval from dual;
select * from table(
ut.run('ut_sith_manager.add_sith_default_on_null_param')
);
select popular_sith_seq.currval from dual;
You can expect a new quiz to appear in the Oracle DevGym by Steven soon, I guess.
We both learned something new. This was a wonderful example of “Curious mammals at work”!
Did you like that post? If so, please let me know – I consider writing more about “well-testable” PL/SQL code and that extra bit of motivation can only help 😉
Update 2020-04-23: Added Bonus Iteration
0 Comments